smarr / are-we-fast-yet

Are We Fast Yet? Comparing Language Implementations with Objects, Closures, and Arrays
Other
318 stars 36 forks source link

Double literals and Comparisons in Ruby's CD and other fixes #57

Closed smarr closed 3 years ago

smarr commented 3 years ago

This turns literals into doubles where it seems to match other benchmarks. Turn the == in the red-black tree to .equal? since this seems to match better the semantics in the other benchmarks.

@eregon could you sanity check this?

eregon commented 3 years ago

This turns literals into doubles where it seems to match other benchmarks.

Looks good.

Turn the == in the red-black tree to .equal? since this seems to match better the semantics in the other benchmarks.

What's the rationale for this one? Semantically it's the same unless the nodes define == (they don't). I think equal? is rarely used explicitly in Ruby, so == seems more idiomatic.

smarr commented 3 years ago

What's the rationale for this one? Semantically it's the same unless the nodes define == (they don't). I think equal? is rarely used explicitly in Ruby, so == seems more idiomatic.

There were some other equal? already, and the code looked inconsistent in the usage of == vs. .equal? https://github.com/smarr/are-we-fast-yet/blob/master/benchmarks/Ruby/cd.rb#L175

The Java version uses ==, and the Python version now uses is to match that. So, my intention was mostly to make the Ruby code consistent. Looking at the difference between ==, .eq?, and .equal?, it seemed to me that .equal? would be the best match for pointer equality.

I am fine with using == if you think it's more idiomatic, and tell me that semantics-wise it's not an issue :)

eregon commented 3 years ago

It's probably good to match other languages then and use equal? explicitly for this case. I think in Ruby people would almost only use equal? if they know == is redefined (e.g. comparing by value). It doesn't matter much either way, in fact both methods end up calling the target: method(:equal?) == method(:==) # => true.