-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Fix TrieMap#{filterInPlace,mapValuesInPlace} #9727
Conversation
currently an inefficient but simpler implementation. i'm working on a faster one, but... I think we should merge the reliable one first, and think about optimisation later |
oops I'm going to overengineer a test again 😬 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (let's merge #9726 first though)
0882c14
to
c643485
Compare
@lrytz I think this has changed enough to need a re-review |
c643485
to
51a65b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
Rebased and added MiMa filters.
test/junit/scala/collection/concurrent/ConcurrentMapTester.scala
Outdated
Show resolved
Hide resolved
51a65b8
to
d0fcd15
Compare
Fix the behaviour of `concurrent.Map#filterInPlace` and `concurrent.Map#mapValuesInPlace` to respect atomic entry changes.
d0fcd15
to
b13ebd7
Compare
@NthPortal the Scala community build (plus the problem is reproducible by cloning https://github.com/twitter/util and running do you have time to dig into it...? |
that's really bad. but also how??? yeah, I'll look into it |
oh noooooooooooo |
followup: #9737 |
Fix the behaviour of
TrieMap#filterInPlace
andTrieMap#mapValuesInPlace
to respect atomic entry changes.Built on top of #9726
Fixes scala/bug#12443