Remove AndOrType

[OlivierBlanvillain]

This PR remove AndOrType to help the JVM's inliner (+2 other small cleanups based on the same data).

The pattern implemented by AndOrType is very problematic for the inliner as the type profiles would be around 50/50 between AndType and OrType, meaning that there is no way to inline further. IIUC, if the ratio goes above 95/5, there the JVM will inline for the common 95% cases and deopt in the other 5% cases.

To get that info from the VM I bootstrapped Dotty with -XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining, then grepped for not inlineable. Here is the full log. Note that each of these are on the hot path, so there should be some performance to be gained in each of them. All the _1 & _2 on types look like other low hanging fruits.

[smarter]

Why not use inline def?

[OlivierBlanvillain]

It breaks the pickling tests :/
I will try to minimize into an issue

[allanrenucci]

class Foo {
  inline def foo: Int = try { 1 } finally println("Hello")
  foo
}

$ dotc tests/allan/Test.scala -Ytest-pickler 
exception occurred while compiling tests/allan/Test.scala
Exception in thread "main" class dotty.tools.dotc.reporting.diagnostic.messages$Error at ?: pickling difference for class Foo in tests/allan/Test.scala, for details:
                   |
                   |  diff before-pickling.txt after-pickling.txt
	at dotty.tools.dotc.reporting.Reporting.error(Reporter.scala:88)
	at dotty.tools.dotc.transform.Pickler.testSame(Pickler.scala:107)
	at dotty.tools.dotc.transform.Pickler.testUnpickler$$anonfun$4(Pickler.scala:99)
	at scala.compat.java8.JProcedure1.apply(JProcedure1.java:18)
	at scala.compat.java8.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.TraversableLike$WithFilter.$anonfun$foreach$1(TraversableLike.scala:789)
	at scala.collection.mutable.HashMap.$anonfun$foreach$1(HashMap.scala:138)
	at scala.collection.mutable.HashTable.foreachEntry(HashTable.scala:236)
	at scala.collection.mutable.HashTable.foreachEntry$(HashTable.scala:229)
	at scala.collection.mutable.HashMap.foreachEntry(HashMap.scala:40)
	at scala.collection.mutable.HashMap.foreach(HashMap.scala:138)
	at scala.collection.TraversableLike$WithFilter.foreach(TraversableLike.scala:788)
	at dotty.tools.dotc.transform.Pickler.testUnpickler(Pickler.scala:100)
	at dotty.tools.dotc.transform.Pickler.runOn(Pickler.scala:83)
	at dotty.tools.dotc.Run.runPhases$4$$anonfun$4(Run.scala:123)
	at scala.compat.java8.JProcedure1.apply(JProcedure1.java:18)
	at scala.compat.java8.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.IndexedSeqOptimized.foreach(IndexedSeqOptimized.scala:32)
	at scala.collection.IndexedSeqOptimized.foreach$(IndexedSeqOptimized.scala:29)
	at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:191)
	at dotty.tools.dotc.Run.runPhases$5(Run.scala:136)
	at dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:141)
	at scala.compat.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:12)
	at dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:88)
	at dotty.tools.dotc.Run.compileUnits(Run.scala:143)
	at dotty.tools.dotc.Run.compileSources(Run.scala:94)
	at dotty.tools.dotc.Run.compile(Run.scala:78)
	at dotty.tools.dotc.Driver.doCompile(Driver.scala:29)
	at dotty.tools.dotc.Driver.process(Driver.scala:127)
	at dotty.tools.dotc.Driver.process(Driver.scala:96)
	at dotty.tools.dotc.Driver.process(Driver.scala:108)
	at dotty.tools.dotc.Driver.main(Driver.scala:135)
	at dotty.tools.dotc.Main.main(Main.scala

[allanrenucci]

This has been fixed. Should revert this change?

[smarter]

I doubt this kind of "outlining" is useful since I expect that the JIT will just inline it right back.

[OlivierBlanvillain]

I pushed the last commit (doing the outlining stuff) by mistake, I've started some benchmarks locally to see how this helps...

[OlivierBlanvillain]

test performance please

[dottybot]

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

[dottybot]

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3986/ to see the changes.

Benchmarks is based on merging with master (36cc65a)

[odersky]

I don't see why we want to put a tag as a parameter in the constant class. It complicates the interface and makes it much harder to create constants. What was the reason for changing this?

[OlivierBlanvillain]

@odersky on master, this large pattern matching is executed in the constructor of every single Constant (the JIT also complains that it's also too large to be inlined). With this refactoring, if the type of the constant is known on the call site we avoid doing that work by directly putting the correct tag:

object Constant {
    def apply(x: Unit) = new Constant(x, UnitTag)
}

Note that the "dynamic" version is still there as a def apply(x: Any): Constant, IIRC it's only used in one of two places.