Analyze the code once per run, not once per child process - #2984
Merged
Conversation
Tests that go through Invoke-InNewProcess collect coverage in the child, and each child called Enter-CoverageAnalysis for the whole target to get there. That walks the Ast of every analyzed file to produce about 10k tracer points, and it produces the same points every time, so with 31 such tests we did the same work 32 times per run. test.ps1 already has that list for its own tracer. Write it out once, and let the children build their tracer from the file instead of deriving it again. They only report back path and 'line:column', so the coordinates are all they need and the command text stays out of the file. Local run of test.ps1 -CI -CC: 188s to 87s. Coverage is unchanged, still 31 child runs, 71 merged points, 87.8%. Split the translation out of Start-TraceScript as Get-TracerPoint so both sides can use it, and fail the run when no child coverage arrives. A child cannot report a failure, it falls back to a plain run, so a break in this plumbing would otherwise only show up as coverage silently going down. 🤖
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
1 participant
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.
Every test that goes through
Invoke-InNewProcesscollects coverage in the child, and it got there by callingEnter-CoverageAnalysisfor the whole target. That walks the Ast of every analyzed file to build the ~10k tracer points, and the points come out the same every time, so a run with 31 such tests did that work 32 times.test.ps1already has the list for its own tracer, so it writes it out once now and the children build their tracer from the file. A child only reports back path andline:column, so the coordinates are all it needs and the command text stays out of the file.Coverage output is the same, still 31 child runs, 71 merged points, 87.8%.
Time
test.ps1 -CI -CClocally went from 188s to 87s.On CI, this PR (build 3940) against #2982 (build 3939), same pipeline on the same base:
The legs run in parallel, so a PR takes the build stage plus the slowest leg, 16:10 down to 13:13. Build 3940 finished in 14:00, the rest is the stage transitions. I am not comparing against 3939's own total of 24:18, it was queued next to another build and that number says more about the queue than about this change.
The 32 minutes of machine time is what comes back when several PRs run at once, which is when the queue is what we actually wait for.
PS 5.1 gains the least and sets the pace now, 12:07 of the 13:13. That is a separate problem from this one.
Split the translation out of
Start-TraceScriptintoGet-TracerPointso both sides use it, added tests for it and for the text form the children read, and made the run fail when no child coverage arrives. A child cannot report a failure, it falls back to a plain run, so a break in this plumbing would otherwise only show as coverage quietly going down.🤖