Fix #19907: Skip soft unions in widenSingle of widenInferred #19995
Conversation
noti0na1
changed the title
noti0na1
changed the title
|
test performance please |
|
test performance please |
|
performance test scheduled: 149 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/19995/ to see the changes. Benchmarks is based on merging with main (a36849f) |
| val widenedFromUnion = widenOr(widenedFromSingle) | ||
| val widenedFromUnion = | ||
| if widenUnions && typeSize(inst) > 64 then | ||
| // If the inferred type `inst` is too large, the subtype check for `bound` in `widenSingle` |
There was a problem hiding this comment.
Very useful diagnosis! I think we can even go further: If we have widenUnions then we never need to widen singletons in soft unions, since we will do an OrDominator afterwards, and this is the same for original or widen types. We don't even need a size limit for this.
I'll push a commit that tries this.
There was a problem hiding this comment.
Correction: It does not work since we sometimes leave original orType if it is transparent, and then we should ideally widen singleton types.
There was a problem hiding this comment.
I could fix it and account for transparent unions. See latest commit.
There was a problem hiding this comment.
Thanks, I updated the title.
noti0na1
changed the title
|
test performance please |
|
performance test scheduled: 139 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/19995/ to see the changes. Benchmarks is based on merging with main (a36849f) |
|
Passing on to @smarter for final comments or approval. |
Replace mergeIfSuper by a different algorithm that is more efficient. We drop or-summands in both arguments of a lub that are subsumed by the other. This avoids expensive recursive calls to lub or expensive comparisons with union types on the right. I tested the previous performance regression #19907 with the new algorithm, and without the changes in #19995 that avoid a slow lub. Where previously it took minutes it now compiles fast. Specifically, we get for i19907_slow_1000_3.scala: 2.9s with the optimizations in #19995, 3.3s with just this PR. And for i19907_slow_1000_4.scala: 3.9s with the optimizations in #19995, 4.5s with just this PR. So the optimizations in #19995 are much less critical now since lubs are much faster. Still, it's probably worthwhile to leave them in in case there is a humongous program that stresses lubs even more.
Fix #19907.
If the inferred type
instis too large, the subtype check forboundinwidenSinglecan be expensivedue to comparisons between large union types, so we avoid it by skipping soft unions in the first step.
Singletons of soft unions are widened when we
widenUnionnow.