Retry constraint.replace after constraint.updateEntry
#20399
Conversation
Checking `isSub` on the resulting bounds can have introduced new constraints, which might allow us to replace the type parameter entirely.
EugeneFlesselle
force-pushed
the
constraint-replace-2
branch
from
3a1c2c5
to
49940ee
Compare
|
@jchyb Can you also take a look and see how it performs? Thank you. |
|
I can't see any difference in how the reproduction performs, unfortunately. I tried printing out what's happening here and it looks like in all of the problematic cases in the reproduction the upper bound starts out as |
|
Are the two |
In scala#20120, we reach constraints with equal bounds that are intersection types, they are formed from multiple successive calls to `addOneBound`. We miss the `replace` optimization in this case because the bounds only become equal progressively, and we are only checking for equality with the constraint being added. The second tryReplace after updateEntry and isSub does not address this specific issue but scala#19955.
They are, but |
|
@jchyb could you measure it once more ? Sorry about the numerous attempts, I do hope this'll be the last one. Thanks again! |
|
Yes! It looks completely fixed (only 52 ms on T1!): |
Great, glad to hear it !
Your minimisations were very helpful too |
|
@odersky still good to merge ? |
|
Wonderfull news about fixing the issue! Great job! I'll try it tomorrow on whole project to check how it applies on the big scale and how it now compares to Scala 2.13 |
|
Nice fix, @EugeneFlesselle ! |
Checking
isSubon the resulting bounds can have introduced new constraints, which might allow us to replace the type parameter entirely.Fix #20120
Close #20208 the original implementation