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

rehashing regression with compare_by_identity #5304

Closed
ahorek opened this Issue Aug 31, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@ahorek
Copy link
Contributor

ahorek commented Aug 31, 2018

Environment

jruby 9.2.1.0-SNAPSHOT (2.5.0) 2018-08-29 dad052a Java HotSpot(TM) 64-Bit Server VM 10.0.1+10 on 10.0.1+10 +jit [mswin32-x86_64]

probably related to the new hash implementation #5215 , @ChrisBr ?

it "rehashes internally so that old keys can be looked up" do

1000.times do
  h = {}
  (1..10).each { |k| h[k] = k }
  o = Object.new
  def o.hash; 123; end
  h[o] = 1
  h.compare_by_identity
  puts h[o] == 1
end

another example (fails randomly)
https://travis-ci.org/rails/sprockets/jobs/422225343

Expected Behavior

should always return true

Actual Behavior

true
true
true
false
true
true
true
true
true
true
true
false
true
true
true
true
true
true
true
true
true
true
true
true
true
true
true
false
true
true
true
true
true
true
false
false
true
true
true
true
true
true
...
@ChrisBr

This comment has been minimized.

Copy link
Contributor

ChrisBr commented Sep 1, 2018

@ahorek thanks for the bug report.

Confirmed, this does not happen with 9.2.2.0. I will look into it!

ChrisBr added a commit to ChrisBr/jruby that referenced this issue Sep 1, 2018

Fix Hash#rehash for open addressing
We used the old bins array to get the index instead of the new one.
This caused that the bins array was wrongly calculated if a
collisions occours and eventually did not found keys in the hash
anymore.

Fix jruby#5304

ChrisBr added a commit to ChrisBr/jruby that referenced this issue Sep 1, 2018

Fix Hash#rehash for open addressing
We used the old bins array to get the index instead of the new one.
This caused that the bins array was wrongly calculated if a
collisions occours and eventually did not found keys in the hash
anymore.

Fix jruby#5304
@ChrisBr

This comment has been minimized.

Copy link
Contributor

ChrisBr commented Sep 1, 2018

@ahorek found the issue and submitted a PR, stay tuned 👍

@kares kares closed this in #5305 Sep 3, 2018

kares added a commit that referenced this issue Sep 3, 2018

Fix Hash#rehash for open addressing (#5305)
We used the old bins array to get the index instead of the new one.
This caused that the bins array was wrongly calculated if a
collisions occours and eventually did not found keys in the hash
anymore.

Fix #5304

@enebo enebo added this to the JRuby 9.2.1.0 milestone Nov 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment