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

when Hash#compare_by_identity is called, it should not copy the keys #1664

Closed
godfat opened this Issue Apr 26, 2014 · 1 comment

Comments

Projects
None yet
2 participants
@godfat
Copy link
Contributor

godfat commented Apr 26, 2014

It seems like we're all copying keys when inserting a value with a string key. I guess this is to avoid people mutating the string after inserting into a hash, and forgot to rehash the hash.

ruby -ve 'h={};s="";h[s]=s;p s.object_id, h.keys.first.object_id'
ruby 2.1.1p76 (2014-02-24 revision 45161) [x86_64-darwin12.0]
70146287427600
70146287428080
jruby -ve 'h={};s="";h[s]=s;p s.object_id, h.keys.first.object_id'
jruby 1.7.11 (1.9.3p392) 2014-02-24 86339bb on Java HotSpot(TM) 64-Bit Server VM 1.7.0_09-b05 [darwin-x86_64]
2000
2002
rbx -ve 'h={};s="";h[s]=s;p s.object_id, h.keys.first.object_id'
rubinius 2.2.5 (2.1.0 e543ba32 2014-02-08 JI) [x86_64-darwin12.5.0]
360
364

However, MRI would not copy the strings after calling compare_by_identity, or it's defeating the purpose:

ruby -ve 'h={}.compare_by_identity;s="";h[s]=s;h[s]=s;p s.object_id, h.keys.first.object_id, h.size'
ruby 2.1.1p76 (2014-02-24 revision 45161) [x86_64-darwin12.0]
70333940591460
70333940591460
1

Note that MRI is not copying the data, therefore even if we insert twice, it still maps to the same object. Both JRuby and Rubinius are still copying here, defeating the purpose of compare_by_identity.

jruby -ve 'h={}.compare_by_identity;s="";h[s]=s;h[s]=s;p s.object_id, h.keys.first.object_id, h.size'
jruby 1.7.11 (1.9.3p392) 2014-02-24 86339bb on Java HotSpot(TM) 64-Bit Server VM 1.7.0_09-b05 [darwin-x86_64]
2000
2002
2
rbx -ve 'h={}.compare_by_identity;s="";h[s]=s;h[s]=s;p s.object_id, h.keys.first.object_id, h.size'
rubinius 2.2.5 (2.1.0 e543ba32 2014-02-08 JI) [x86_64-darwin12.5.0]
360
364
2

Would also file this for Rubinius.
Done: rubinius/rubinius#3017

@godfat

This comment has been minimized.

Copy link
Contributor Author

godfat commented Apr 26, 2014

Also added a test case on rubyspec:
https://github.com/rubyspec/rubyspec/pull/274

enebo added a commit that referenced this issue May 1, 2014

Merge pull request #1666 from godfat/jruby-1_7-no-copy-for-compare_by…
…_identity

Fix #1664 for Hash#compare_by_identity and Hash#store

@jrubyci jrubyci closed this in e608e4b May 1, 2014

@enebo enebo added this to the JRuby 1.7.13 milestone Jun 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.