Scoverage: do not instrument methods that have too large bodies#25629
Open
anatoliykmetyuk wants to merge 2 commits intoscala:mainfrom
Open
Scoverage: do not instrument methods that have too large bodies#25629anatoliykmetyuk wants to merge 2 commits intoscala:mainfrom
anatoliykmetyuk wants to merge 2 commits intoscala:mainfrom
Conversation
Coverage instrumentation adds ~15 bytes per statement via Invoker.invoked() calls. For very large method bodies or val initializers (e.g., thousands of method calls or huge array literals), this can push the generated bytecode over the JVM's 64KB method size limit. Skip coverage instrumentation for DefDefs and ValDefs whose RHS tree exceeds 3000 nodes. These bodies are left uninstrumented to prevent "Method too large" emission errors, while the rest of the compilation unit is instrumented normally. Fixes coverage compilation for: large2.scala, i7034.scala, i20521.scala, bridges.scala, t10594.scala.
SolalPirelli
approved these changes
Mar 26, 2026
Contributor
SolalPirelli
left a comment
SolalPirelli
left a comment
There was a problem hiding this comment.
Looks like a reasonable workaround; for posterity, is there some underlying reason why this skip logic can't be moved to coverage generation so it checks the actual covered size?
Contributor
Author
|
Thank you for the review @SolalPirelli ! The reason for using a heuristic approach is that we don't know the emitted bytecode's actual size until the backend phase at the very end of the compilation chain. Coverage instrumentation is a tree transformation step that happens much earlier. If we did the check at backend, there would be no trivial, unintrusive way to revert the instrumentation step. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While instrumenting trees, coverage instrumentation increases their size. There is a fundamental limit of 64KB on method body size on JVM. When a large method is instrumented, it can exceed this limit and cause compilation to fail with
Method too large.This PR opts out coverage instrumentation for oversized
DefDefandValDefbodies, the size of which is greater than 3000. The rationale for the number being that the per-statement overhead ofInvoker.invoked()is ~15 bytes, and roughly half of the tree nodes in a large body would each add that overhead. So, 3000 nodes would be about 24KB of added instrumentation bytecode.If a method is opted out from coverage due to size, a warning is emitted.
How much have you relied on LLM-based tools in this contribution?
Moderately.
How was the solution tested?
sbt --client testCompilation