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

Detect compilation while benchmarking #10574

Merged
merged 9 commits into from
Jul 18, 2024

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Jul 17, 2024

Pull Request Description

Enables engine.TruffleCompilation in std-benchmarks, collects the logs and dumps compilation into to System.err when a benchmark is influenced by dynamic compilation.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Java,
  • Run benchmarks with compilation detection on: benchmark run

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Jul 17, 2024
@JaroslavTulach JaroslavTulach self-assigned this Jul 17, 2024
@JaroslavTulach
Copy link
Member Author

Nice "side-effect" of this change is: We can see the errors from the compiler like this one:

PermanentBailoutException: Too deep inlining, probably caused by recursive inlining.
Inlined methods ordered by inlining frequency:
org.enso.interpreter.runtime.data.Type.allTypes(EnsoContext)

I guess we should finally fix this allTypes problem!

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Jul 17, 2024

There seems to be 26 compilation detected messages. Those benchmarks might benefit from increasing the warmup time.

Update: there seems to be (at least) hundred "compilation detected" messages in the new run (with 0862a0f).

@JaroslavTulach
Copy link
Member Author

0862a0f fixes the problem with too deep inlining in Type.allTypes. Running the benchmarks again to verify the current state.

@JaroslavTulach JaroslavTulach added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Jul 17, 2024
@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Jul 18, 2024

Update: there seems to be (at least) hundred "compilation detected" messages in the new run (with 0862a0f).

The benchmarks finish without errors, but some results are really skewed:

Collections_list_fold

as can be seen the second iteration is marked as Iteration 2: <failure>... but that's not the cause of such a dramatic slowdown - that's missing @ExplodeLoop - added in a5f84ff

Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to be able to see the compilation problems in the benchmarks :)

@JaroslavTulach JaroslavTulach force-pushed the wip/jtulach/DetectCompilation6271 branch from 5427549 to 00d6f2c Compare July 18, 2024 07:57
@JaroslavTulach JaroslavTulach force-pushed the wip/jtulach/DetectCompilation6271 branch from 00d6f2c to 5acbda0 Compare July 18, 2024 08:01
@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Jul 18, 2024

We have a usage of blacklisted string concatenation method. Hidden behind @TruffleBoundary in fe6d2be

@JaroslavTulach
Copy link
Member Author

Final benchmark run scheduled. It just dumps info, it doesn't throw AssertionError. As such the run should be green, but the log can be used to find out whether a particular benchmark was influenced by compilation or not. That's probably good enough to start with for now.

Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the fact that the Truffle engine compilation is logged into System.err and no Exception is thrown from within the @Teardown benchmark method, as that might cause some issue with result reporting. I hope this will enable us to set better parameters for the benchmarks.

@JaroslavTulach JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Jul 18, 2024
@mergify mergify bot merged commit c20eab2 into develop Jul 18, 2024
44 checks passed
@mergify mergify bot deleted the wip/jtulach/DetectCompilation6271 branch July 18, 2024 15:49
@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Jul 18, 2024

The benchmarks finish without errors, but some results are really skewed:
... but that's not the cause of such a dramatic slowdown - that's missing @ExplodeLoop - added in a5f84ff

obrazek

Collections_fold_list is really fixed. Other stdlib benchmarks also look good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants