Revert #14452 and make compile-time operations on stable arguments stable

[mbovel]

Revert #14452

#14452 made compile-time operations covariant. The use-case was mainly to allow allow the compile to remove skolems around operation arguments. That for example enables the following:

(?1 : x.type * x.type) * x.type <: (x.type * x.type) * x.type

Unfortunately, we realized afterwards that this adds significant constraints on the normalization of these compile-time operations. To preserve transitivity of algorithmic subtyping, we need to make sure that ∀ T, U. T <: U → Norm[T] <: Norm[U], which is non-trivial to achieve when compile-time operations are covariant.

For instance, consider the two following situations (an arrow from A to B means A <: B):

In both cases, the problem is the red arrow:

• On the left: if we normalize 1 + 1 to 2, then we should make sure that it is still a subtype of 1 + (2 | 3). This could be achieved by distributing additions and multiplications over unions, but we might not want to do it because this would make the runtime/space of normalization exponential.
• On the right, we see a more general case: if 1 + (a : A) is normalized to (a: A) + 1, how do we ensure that the later is also a subtype of 1 + A? We thought about a few possible ways to do that—for example by approximating non-singleton types by their known atoms—, but it seems that if we want to handle the general case, we can't avoid brutally trying different orders of operands to compare operations, which is also exponential in the worst case.

Conclusion: making compile-time operations covariant complexifies the implementation of normalization because it creates new subtype edges that we would need to preserve when normalizing. We do not want to handle all these cases now, we revert #14452 to make them invariant again (reverted in 12a10f2).

Make compile-time operations on stable arguments stable

So we don't want compile-time operations to be covariant, but we still need:

(?1 : x.type * x.type) * x.type <: (x.type * x.type) * x.type

In order to achieve this:

1. 16fa957 extends Type.isStable so that compile-time operations with stable arguments are considered stable,
2. 7c8667e extends the fixForEvaluation method inside AppliedType.tryCompiletimeConstantFold so that arguments of compile-time operations are dereferenced to their deepest underlying stable type.

See tests/neg/singleton-ops-int.scala for the new tests it enables to pass.

Add micro benchmarks

To check the performance of common operations on types, 823d710 adds a new JMH suite that benchmarks the runtime of the common type operations isStable, normalized, simplified, dealias, widen, atoms, isProvisional on some simple types (AppliedType, TermRef, etc.).

The suite can be run with:

sbt "scala3-bench-micro / jmh:run"

The use-case of these benchmarks for this PR was mainly to test different caching strategies for Type#isStable and to make sure that they indeed result in a performance gain. See results in comments for more details.

[odersky]

Very nervous about performance implications here. isStable is a hotspot and what you do here seems to be about an order of magnitude more expensive than the rest. If we want to go down that route we probably should cache the value in AppliedType.

[mbovel]

b6704d7 caches isStable in AppliedType. As isStable does not depend on type variables, can we just cache it once and for all? Or should it still depend on the period? All current tests pass with the first alternative.

In term of performance, I couldn't see any difference with our current benchmarks, so I added a micro benchmark measuring specifically the performance of isStable: 9ca8891.

Before any change (main):

Benchmark                         (valName)   Mode  Cnt         Score        Error  Units
IsStable.isStableBenchmark              int  thrpt   30  31573088.895 ± 446326.483  ops/s
IsStable.isStableBenchmark  deepSumUnstable  thrpt   30  31905715.432 ± 403879.002  ops/s
IsStable.isStableBenchmark    deepSumStable  thrpt   30  31632042.261 ± 329863.487  ops/s

With applied-types-aware isStable but without cache (9ca8891.):

Benchmark                         (valName)   Mode  Cnt         Score        Error  Units
IsStable.isStableBenchmark              int  thrpt   30  27618898.774 ± 351504.087  ops/s
IsStable.isStableBenchmark  deepSumUnstable  thrpt   30   2702127.175 ±  99159.508  ops/s
IsStable.isStableBenchmark    deepSumStable  thrpt   30    815868.091 ±  27206.935  ops/s

With applied-types-aware isStable cached (b6704d7):

Benchmark                         (valName)   Mode  Cnt         Score        Error  Units
IsStable.isStableBenchmark              int  thrpt   30  31601394.759 ± 228033.535  ops/s
IsStable.isStableBenchmark  deepSumUnstable  thrpt   30  31393180.896 ± 336974.793  ops/s
IsStable.isStableBenchmark    deepSumStable  thrpt   30  31814767.854 ± 531967.869  ops/s

This confirms that the performance of isStable on a deep type applications (balanced tree of depth 4) was indeed 1 or 2 order of magnitude slower without cache. By the way, don't we have exactly the same problems with intersections and unions? Why is it particularly a problem with applied types?

[dottybot]

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/15268/ to see the changes.

Benchmarks is based on merging with main (6783853)

[soronpo]

LGTM

[dottybot]

performance test scheduled: 6 job(s) in queue, 1 running.

[dottybot]

performance test scheduled: 6 job(s) in queue, 1 running.

[mbovel]

Note that an important consequence of making compile-time operations on stable arguments stable is that it enables to use operation types where singletons are expected as demonstrated by this test:

summon[t85.type + t86.type <:< Singleton]

For now, this however does not work:

summon[t85.type + t86.type <:< Singleton & Int]

I plan to fix it in a separate PR.

[dottybot]

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/15268/ to see the changes.

Benchmarks is based on merging with main (78dbc59)

[mbovel]

test performance please

[dottybot]

performance test scheduled: 1 job(s) in queue, 1 running.

[dottybot]

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/15268/ to see the changes.

Benchmarks is based on merging with main (76a0b29)

[mbovel]

The previous run of the compilation benchmarks shows no significant differences.

Following are some more detailed micro benchmarks results.

isStable

• In green, we can see as expected that extending isStable to consider compile-time operations on stable arguments stable without caching makes its runtime complexity linear: we get a ~3x slowdown for singletonsSum and intsSum, and 50x and 14x slowdowns for their deep counterparts.
• In purple, we see that caching in AppliedType solves the problem.
• In yellow, we see that caching in Type not only solves the problem, but drastically improves the runtime for all tested types. The speedup is 27x to 109x.

Each point is the throughput for a JMH iteration, relative to the median of the corresponding "Baseline" iterations. Note the logarithmic scale for the y-axis. Higher is better.

normalized

• In red, green and purple, we see that checking isStable in tryCompiletimeConstantFold significantly slows it down. Caching AppliedType#isStable only does not solve the problem.
• However, in yellow, we see that caching isStable for all types solves the problem.

See legends in first graph. Linear scales. Higher is better.

simplified

Similar observations as for normalized.

See legends in first graph. Linear scales. Higher is better.

[mbovel]

Reasoning here: no need to check that tp is stable because it will be if its underlying type is stable. Could someone confirm if this is correct?

Otherwise, we might want:

case tp: TypeProxy if tp.isStable && tp.underlying.isStable => tp.underlying.fixForEvaluation

or

case tp: SingletonType if tp.isStable && tp.underlying.isStable => tp.underlying.fixForEvaluation

[odersky]

I think at least morally this is correct.

[mbovel]

After caching TermRef#isStable and noticing that it doesn't impact the benchmark results, I digged further and noticed that the time difference is spent in pattern matching in isStable. Here are some results showing the difference between pattern patching vs methods overriding for 3 different JVMs:

The y-axis unit is operations per second. Note the logarithmic scale. Higher is better.

I am not sure if these results are generally relevant or are only artifacts of these micro benchmarks.

[mbovel]

test performance with #compiletime please

[dottybot]

performance test scheduled: 3 job(s) in queue, 1 running.

[dottybot]

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/15268/ to see the changes.

Benchmarks is based on merging with main (836ed97)

[smarter]

I think you mean overriding not overloading, also it's not clear from the graph if bigger numbers are better or worse, but usually overriding is faster

[mbovel]

I think you mean overriding not overloading

Indeed, fixed.

also it's not clear from the graph if bigger numbers are better or worse, but usually overriding is faster

Legend added. The unit of the y-axis is ops/sec; higher is better.

[mbovel]

As discussed during today's dotty meeting:

• Let's keep commits up to 9205cc6 in this PR
• Let's investigate pattern matching performance separately. There are existing benchmarks: Add a benchmark on pattern matching vs alternatives scala#9632.

[odersky]

LGTM

[odersky]

I think at least morally this is correct.

[mbovel]

For now, this however does not work:

summon[t85.type + t86.type <:< Singleton & Int]

It actually does work; I just didn't realize that & had a higher operation priority than <:< 😅

summon[t85.type + t86.type <:< (Singleton & Int)] // compiles