Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use BasicObject#equal? for Symbol#==. #3449

Merged
merged 1 commit into from Jun 25, 2015

Conversation

eregon
Copy link
Contributor

@eregon eregon commented Jun 25, 2015

Significantly faster for the case of different symbols:

require 'benchmark/ips'
Benchmark.ips do |b|
  b.report("Symbol#==") do
    :a == :b
  end
end

Before:
Symbol#== 2.712M (± 3.4%) i/s - 13.589M
After
Symbol#== 6.862M (± 2.6%) i/s - 34.413M

Significantly faster for the case of different symbols:

require 'benchmark/ips'
Benchmark.ips do |b|
  b.report("Symbol#==") do
    :a == :b
  end
end

Before:
Symbol#==      2.712M (± 3.4%) i/s -     13.589M
After
Symbol#==      6.862M (± 2.6%) i/s -     34.413M
@eregon
Copy link
Contributor Author

eregon commented Jun 25, 2015

Semantics are not changed, because in Symbol#<=> only subclasses of Symbol are allowed and it is anyway prohibited to make an instance of a subclass, making identity comparison have the same behavior.

@yorickpeterse
Copy link
Contributor

Yeah, I think I or @jemc bumped into this a while ago which ultimately led to @jemc creating #3391. Since there's no downside to this I don't see why we shouldn't merge this, thanks!

yorickpeterse pushed a commit that referenced this pull request Jun 25, 2015
Use BasicObject#equal? for Symbol#==.
@yorickpeterse yorickpeterse merged commit d63ccf5 into rubinius:master Jun 25, 2015
@jemc
Copy link
Member

jemc commented Jun 25, 2015

Thanks, @eregon - I self-assigned #3391 so that I would get to it when I had time at the end of that week, then completely forgot about it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants