make Visitor not satisfy WideningTransformer

[jvasileff]

For consideration/discussion.

When working on #107, I noticed that WideningTransformer<Null>s used as visitors perform better than Visitors. Prior to #107, timings where 63.31ms for the transformer, and 79.72ms for the visitor. I assume this is due to the extra object created by the Java backend for interfaces with default methods and the extra layer of indirection.

With this patch, the visitor takes only 28.33ms vs. 41.49ms for the visitor after #107. So there is a significant performance boost that would be meaningful to code that runs several passes of a visitor.

Now, this still isn't as fast as the typechecker's visitor, which I can only guess attains its speed from 1) being an abstract class rather than a delegate object, and/or 2) having void methods rather than Object-returning methods. These aren't things we can address.

The raw numbers:

-------------------
Initial (1.2.0 ast)
-------------------
countNodesTransformerNull:         63.31ms (control)
countNodesVisitor:                 79.72ms
countNodesTcVisitor:               14.76ms (control)

------------------------------
With visitor optimization #107
------------------------------
countNodesTransformerNull:         56.96ms (control)
countNodesVisitor:                 41.49ms
countNodesTcVisitor:               14.91ms (control)

-----------------
With fast-visitor 
-----------------
countNodesTransformerNull:         59.25ms (control)
countNodesVisitor:                 28.33ms
countNodesTcVisitor:               14.94ms (control)

[lucaswerkmeister]

I want to have this change in 1.2.1, but I can’t merge this PR because it touches virtually every file in source/ceylon/ast/core, so it doesn’t apply right now, and even if I merged a fixed version right now, it could break when I made more changes in independent branches.

I think I’ll make this change right before the release, when everything else is already done. And, importantly, source-gen will have to be updated as well, to produce the extra visit method.

@jvasileff how did you produce this patch? Not manually, I hope…

[jvasileff]

how did you produce this patch? Not manually, I hope…

see the commit message for the precise steps 😄

[lucaswerkmeister]

Whoa. Nice! Thanks!

I had avoided looking at the commit message because it also makes GitHub load the huge patch :D

[lucaswerkmeister]

We could also provide this class:

shared abstract class AbstractVisitor() satisfies Visitor {}

If your visitors extend that class instead of satisfying Visitor, the compiler doesn’t have to generate a thousand methods that just delegate to Visitor$impl. If you have a lot of visitors, this might have a significant impact on bytecode size – AbstractVisitor.class is 100K.

[jvasileff]

Interesting idea (AbstractVisitor)

I gave it a try to see if it would help with hotspot (less code to optimize). But it didn't seem to make a difference.

Since this is something space conscious users can do on their own, I think I'd probably wait to see what happens with the Java 8 version of the compiler.

[lucaswerkmeister]

Yeah, once we compile to Java 8’s default interfaces this should be obsolete anyways.

[lucaswerkmeister]

Merged in 3fc1045.

[lucaswerkmeister]

Oops, wasn’t quite done yet.

[lucaswerkmeister]

Now done in 5cd395b, I hope.