New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Assorted compiler optimizations #6209
Conversation
Selected from #6115 |
6e22faf
to
0f66371
Compare
05fc487
to
0094ced
Compare
Here are the benchmark charts for the recent history before this PR, and for the commits in this PR. The aggregate improvement appears to be ~6%. Only the final commit is benchmarked so far. The other points will fill in over the coming hours. |
|
The biggest wins appear to be
The The "optimize enclosingRootClass" needed some last minute changes to be able to distinguish |
0094ced
to
44cc7e1
Compare
I've pushed the trimmed down version discussed above. For posterity, the original sequence of commits are in branch faster/december-take1. |
/sync |
Notably, this will be used in `List(a, b, c)`.
44cc7e1
to
1bc77ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes to th statistics infrastructure LGTM. I would also welcome a more aggressive move of some other compiler-specific (and not potentially interesting to users) to the hot mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it a bit surprising that nonEmpty
-> !isEmpty
makes a measurable difference.. Do you have an explanation? Could we add an override somewhere in the collections?
i -= 1 | ||
} | ||
result | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @szeiger - does that need to be ported to collection-strawman? i couldn't find WrappedArray
there.
To avoid the reversed iteration order, we could use a version that mutates ::.tl
override def toList: List[A] = {
if (length == 0) Nil
else {
val res = immutable.::(apply(0), Nil)
var cur = res
var i = 1
while (i < length) {
val next = immutable.::(apply(i), Nil)
cur.tl = next
cur = next
i += 1
}
res
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably local val length as well
I've applied b175218 in my scalac fork to compile Scalatest's test suite with statistics enabled, and I'm seeing a 20s improvements (from 111s to 90s). The Scalatest test suite has around 300.000 LOC. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the effect with statistics disabled. It seems that this includes removal of lots of statistics capture but I am only looking at a quick summary on my phone
My understanding is that the statistics is low to nil overhead with statistics disabled and that is the case that we are optimising for. Statistics are useful to identify the bottlenecks but the only performance figures that matter are in the production use case IMO
It would be good to see a breakdown of the different changes for this pr rather that just a summary. I have some tooling to assist with that is you need it contact me or @rorygraves for details
No description provided.