You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I'm in favour of explicitly documenting the "don't mutate in the callback" restriction. Maybe this could be documented as a general note for mutable collections that results are undefined if they are mutated during queries or iteration.
But I'd be happy to revert to the current behaviour in the 2.12.x series to be conservative.
@Ichoran said:
Ugh. I wonder if there's a way to cheaply detect that there is mutation during the closure and fall back to the slow way if not? I'd imagine it's just a couple of eqs, which should be pretty fast. I'm wary of not keeping this behavior consistent with 2.12 since it is a semi-obvious way to do things (i.e. if you don't find something you expect to, run an initialization routine that covers more than just the single one you missed).
Documenting the new behavior as the default in 2.13 (and reverting in 2.12) would be a natural choice. OTOH, getOrElseUpdate is much more convenient than doing the same thing manually, so it's understandable why people would want to use it even though they do not care about the improved performance (with the limitation of not being allowed to modify the map in the callback). The only obvious places in the Scala repo where the map is mutated while computing the default value are in ProdConsAnalyzerImpl.scala and SetMapConsistencyTest.scala, in both cases using an AnyRefMap.
The latter is a regression test for #8213 where the new behavior that we now have in HashMap was classified as a bug and consequently fixed for AnyRefMap. For the sake of consistency I suggest that we revert the change and allow mutation. Since the motivation for #10049 was performance of groupBy, we could explore a more directed fix, e.g. overriding groupBy in HashMap and using a private copy of getOrElseUpdate with the new semantics.
These used to both be true in 2.12.0 and earlier. The change to optimize
HashMap#getOrElseUpdate
in 2.12.1 uses stale values in theupdate
part.Note that Java 9 has explicitly specced
Map#computeIfAbsent
to outlaw mutation of the map in the callback.The text was updated successfully, but these errors were encountered: