Fix #1619: Reduce collision rate in NameTable (II)

[jvican]

The fix is consistent with Scalac: https://github.com/scala/scala/pull/5474/files.

[odersky]

Since the aim of the PR is speed it would be good to see benchmarks that show how it compares to the baseline. How much faster is ScalaTest compilation with the change? Do we see a slowdown elsewhere?

[jvican]

Jason did some benchmarks here: scala/scala-dev#246 (comment). He only found a negligible (+2ms) increase for normal libraries like better-files. If you think it would be interesting to do the benchmarks for Dotty, I'm happy to double-check the results are the same.

[retronym]

I haven't merged this change yet into scala/scala because one of my benchmarks (compiling scalap) seemed to slow down by 1%. I haven't had time to dive into this result to see what's going on.

ScalaTest performs a self-test by generating tonnes of code to compile that exercise different combinations of ScalaTest base classes, testing DSLs, etc.

A combination of factors led to the pathological case:

• Use of globally numbered names "_123.type"for existentials captured from the prefix in AsSeenFrom
• ... which hash to a handful of buckets
• over the course of a big compilation run (in Dotty's case, this could be a long running process as the name table isn't cleared)

[odersky]

Yes, I think it would be good to do the benchmarks for dotty. Also, the question is whether we see a slowdown for ScalaTest. The specific cases that Jason noted which ruin hashcodes for scalac are not present in dotty.

[jvican]

Perfect, will submit the results.

[retronym]

@odersky I'm also proposing a change to use number those fresh existential symbols relative to the current asSeenFrom, rather than using a global counter. Do you see any gotchas with that change?

[retronym]

Incidentally, the string table in HotSpot VM (for storing names of classes, methods, etc) uses the equivalent of String::hashCode to start with and switches over to MurmurHash on the fly if a bucket exceeds 60x the expected length.

[odersky]

Will benchmarks be forthcoming? If not we should close.

[jvican]

Yes, I'm working on them. Will report soon.

[odersky]

OK thanks!

…

On Sat, Dec 17, 2016 at 6:44 PM, Jorge ***@***.***> wrote: Yes, I'm working on them. Will report soon. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAwlVhlFXBsaFznW9t_6G3FmZhYrnqsHks5rJB9igaJpZM4K3pwM> . {"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/lampepfl/dotty","title":" lampepfl/dotty","subtitle":"GitHub repository","main_image_url":" https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/lampepfl/dotty"}}," ***@***.*** in #1727: Yes, I'm working on them. Will report soon."}],"action":{"name":"View Pull Request","url":"#1727 (comment) 267776092"}}}

-- Prof. Martin Odersky LAMP/IC, EPFL

[jvican]

Now we have a JMH-based Dotty benchmarking tool to add community projects and benchmark them as scalac does: https://github.com/jvican/compiler-benchmark. It is a modified fork of @retronym's compiler benchmarks that for now only allow us to benchmark running times of the compiler, not the internals. I guessed this would be useful in the future, so I put some time into it. We are now compiling a modified version of better-files and squants, and almost all scalap for which I have to file a Tasty bug and try to minimize it.

I will explain the benchmark results of this change very soon, I need to have a careful look to the numbers.

[jvican]

These are the results of the benchmarks for squants and better-files.

They were run in a Linux 4.8.13-1 with an Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz with TurboBoost disabled, 16GB of RAM and no swapping. The results tell that this change slightly slows down the compilation for cold compilations of better-files and squants and behaves "similarly" for warmed JVMs. Both are ~1 second slower in cold start but get better results in hot JVMs (results differ from slightly slower to slightly faster depending on the percentile). These are the jmh options used.

Unfortunately, we cannot compile scalap yet. I think that scalap would throw more interesting results since it is the only library to which this change has had some impact in scalac. Judging by what we have right now, I think that it's maybe better to close this and revisit it in the future when performance fixes are more required. This one is not very favourable to Dotty and I'm unsure we'll ever need it...

[odersky]

Thanks for the detailed results @jvician! This is very helpful. What's the problem compiling scalap? Can we fix it? In that case I would propose to leave this issue open until we have can test with scalap.

[jvican]

@odersky I made some tweaks in scalap to make it compile (there were 16 compilation failures IIRC). I think I removed all of them but Dotty then threw a pickling exception -- for some reason, Dotty was failing to unpickle the symbol of a type parameter B. The error was a little bit cryptic, so I'll look into it and report the issue in detail. I'm also opening other issues for the other fixes that I made, some of them were unintuitive and need to be simplified. I'll take care of this tomorrow 😄.