Skip to content
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

Engine benchmark regression on 2024-03-04 #9563

Closed
Akirathan opened this issue Mar 27, 2024 · 3 comments · Fixed by #9646
Closed

Engine benchmark regression on 2024-03-04 #9563

Akirathan opened this issue Mar 27, 2024 · 3 comments · Fixed by #9646
Assignees
Labels
--low-performance -compiler p-medium Should be completed in the next few sprints

Comments

@Akirathan
Copy link
Member Author

Akirathan commented Mar 27, 2024

engine.BackgroundCompilation was newly set to false in https://github.com/enso-org/enso/pull/9190/files#diff-7c0bf5c634ebfb606d229c275ac2cba4f74f2b156a6a674a43a6bc22d81968aaR58-R71. This is very bad idea for benchmarks. Let's revert this.

GitHub
Pull Request Description Fixes #8645 by recognizing ~ prefix to constructor names. Checklist Please ensure that the following checklist has been satisfied before submitting the PR:

The documentat...

@Akirathan
Copy link
Member Author

Akirathan commented Mar 27, 2024

@JaroslavTulach's theory is that engine.BackgroundCompilation=false should not affect performance. Let's ensure that this is the case (DISCLAIMER: It is not - it causes huge regressions).

Experimenting with:

sbt:runtime-benchmarks> run -wi 3 -w 10 -i 3 -r 10 org.enso.interpreter.bench.benchmarks.semantic.WarningBenchmarks.noWarningsVecSum

(noWarningsVecSum benchmark with 3x10 seconds warmup and 3x10 seconds measurement)

Local score with --engine.BackgroundCompilation=false: 0.183
Local score with --engine.BackgroundCompilation=true: 0.009

Setting --engine.MultiTier to either false or true does not seem to have any impact on the final score.

Run with CompilationStatisticDetails=true

Running with --engine.CompilationStatisticDetails reveals that:

BackgroundCompilation=false

Compilations: 25
Success: 24
Temporary bailouts: 0
Permanent bailouts: 1

image

BackgroundCompilation=true

Compilations: 83
Success: 76
Temporary bailouts: 6
Permanent bailouts: 1

image

This just tells us that there is a lot more compilation for engine.BackgroundCompilation=true, which is a bit obvious.

TL;DR;

--engine.BackgroundCompilation=false causes huge regressions. We should not use it. Moreover, it is not used in production, so let's not use it for benchmarking. The same is true for --engine.MultiTier=false. In production, we use --engine.MultiTier=true (the default). Why use something different in benchmarks?

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Mar 29, 2024

MultiTier

In production, we use --engine.MultiTier=true (the default). Why use something different in benchmarks?

The IGV graphs are simpler to analyze with MultiTier=false and as you write...

Setting --engine.MultiTier to either false or true does not seem to have any impact on the final score.

...it doesn't affect the result of final compilation. E.g. easier to disable multi tier compilation in benchmarks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--low-performance -compiler p-medium Should be completed in the next few sprints
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants