Improve performance of atPos under -Yrangepos#8858
Conversation
|
In retronym/compiler-benchmark#2, I measured a 15% reduction in the allocations in repeatedly parsing a file full of case classes. This exercises the |
| } | ||
| def apply(tree: Tree): Unit = { | ||
| wrappingPosAccumulator.reset() | ||
| if (!tree.isEmpty && tree.canHaveAttrs && tree.pos == NoPosition) { |
There was a problem hiding this comment.
Maybe test for UndefinedPosition instead of NoPosition?
There was a problem hiding this comment.
I tried to keep the logic as it was before. UndefinedPosition also includes FakePos, that that appears only to be used to report errors through APIs that expect a position in contexts when we don't have a SourceFile. I wouldn't expect it to be attached to a tree.
|
|
||
| /** Equivalent to `this.children.headOption.getOrElse(EmptyTree)`, but more efficient. */ | ||
| final def onlyChild: Tree = { | ||
| onlyChildAccumulator.using(accum => { foreachChild(accum); accum.result()}) |
There was a problem hiding this comment.
Would it be worth the effort to break out after seeing the second child?
There was a problem hiding this comment.
Okay, the callback to foreachChild can how return false to stop traversal.
8db250c to
3ba534f
Compare
Don't materialize lists of child nodes, and instead use
a new method,
foreachChildto iterate through them.Existing tests of range positions such as
test/files/run/dynamic-applyDynamic.scala
... give good coverage of the logic.