Skip to content

[do not merge] remove magic with benchmark tests #1229

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

Merged
merged 2 commits into from
May 30, 2016

Conversation

liufengyun
Copy link
Contributor

@liufengyun liufengyun commented Apr 20, 2016

  • remove the magic with ScalaMeter test
  • remove pos tests
  • merge all dotc related tests to a single dotty test
  • remove Ycheck:all

The rationale for merging dotc tests is that measurement units should be either big(whole project) or as small as a single file in order to shed light on which part of the compiler might be slower or faster.

We can add specific single file bench test as we go. Please review @DarkDimius , thanks.

@liufengyun
Copy link
Contributor Author

I've rebased the PR. The bench test runs without problem on my machine, but it generates following error on the benchmark testing machine:

java.lang.ClassCastException: scala.tools.nsc.backend.jvm.BTypes$ClassBType cannot be cast to scala.tools.nsc.backend.jvm.BTypes$ArrayBType

I guess it's related to some environment settings, I'll need to investigate more.

@DarkDimius DarkDimius changed the title remove magic with benchmark tests [do not merge] remove magic with benchmark tests May 25, 2016
@liufengyun
Copy link
Contributor Author

Thanks @DarkDimius for your clue, now the cause of exception on benchmark machine is spotted and fixed in 23d1435. The cause is that bench project uses a different version of scala-compiler. Explicit specifying the version fixes the problem.

I've also rebased this PR over master. If the tests pass, it's ready to be merged and we'll start tracking performance with just two projects: stdlib and dotty. We can add more standard projects as well.

The benchmarks are at macro-level. We can add interesting single file micro-level tests as we go. And I'll play with flame graph later so that we'll have tools to diagnose micro-level benchmarks if there's significant regression.

@liufengyun
Copy link
Contributor Author

One tricky issue with using a static version of dotty for benchmarks is that the older version of dotty may fail to compile. E.g., the introduction of multiverse makes previous version of dotty code illegal.

If such things do happen, we'll have to update dotty-src, which breaks our objective to use a static version of code for benchmarks.

- remove dependence on junit
- remove `pos` tests
- merge all dotc related tests to a single `dotty` test
- remove `Ycheck:all`
The benchmark project generates a dubious exception when compiling
scala stdlib:

    java.lang.ClassCastException: scala.tools.nsc.backend.jvm.BTypes$ClassBType
    cannot be cast to scala.tools.nsc.backend.jvm.BTypes$ArrayBType

Upon investigation, this turns out to be a backend incompatibility
problem. The bench project runs with a previous version of scala-compiler.
Explicitly designating the scala-compiler version for bench fixes
the problem.
@liufengyun
Copy link
Contributor Author

Due to the fact that using a static version of dotty may not compile later, I've reverted back to always compile the latest version of dotty.

sbt incremental compilation will not interfere after this PR is merged, as I've separated bench tests from regression tests.

Review @DarkDimius .

@DarkDimius
Copy link
Contributor

LGTM!
Now we need to start adding small tests that benchmark specific features.
For start, i propose patmatexhaust-huge.scala for benchrmarking pattern matcher & exhaustivity checker.

@DarkDimius DarkDimius merged commit 5838fda into scala:master May 30, 2016
@liufengyun
Copy link
Contributor Author

Thanks @DarkDimius . I'll make a tests/bench/<feature>/ folder, and we can add feature-specific benchmarks there.

@liufengyun liufengyun deleted the bench branch May 30, 2016 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants