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

perf fixes copy from dotutils and MSBuildStructuredLog #10792

Merged

Conversation

JanProvaznik
Copy link
Member

@JanProvaznik JanProvaznik commented Oct 10, 2024

Fixes #10674 #10675

Context

dotutils/streamutils#2 makes more performant custom implementations of streams which can be copied here

KirillOsenkov/MSBuildStructuredLog#821 indroduced changes to BuildEventArgsReader which should be the same in MSBuildStructuredLog, this PR copies these changes.

Changes Made

3 commits

  1. override custom stream methods
  2. adopt changes to BuildEventArgsReader
  3. make non-params versions of resource methods

Testing

perf improvement validation setup:

  1. get validation .binlog from msbuild's own build on this branch `.\build /bl /p:Configuration=Release,
  2. also do this for a baseline from main 94941d9
  3. measurement of replay speed in powershell script: run with each version bootstrapped replay versionX\artifacts\bin\bootstrap\core\dotnet.exe build ThisPRVersion\msbuild.binlog
  4. 100x measurements for both versions alternating their runs
  5. mean, stdev, t-test

Results on Devbox machine:

Command 1: .\msbuild\artifacts\bin\bootstrap\core\dotnet.exe build .\msbuild\msbuild.binlog
Average runtime: 1790.30201 ms
Standard deviation: 247.701794004614 ms

Command 2: .\msbuildbase\artifacts\bin\bootstrap\core\dotnet.exe build .\msbuild\msbuild.binlog
Average runtime: 2202.75113 ms
Standard deviation: 146.88933492219 ms

T-statistic: -14.3221294766752
degrees of freedom 100-1
pvalue <0.0001, we can reject the hypothesis that the distributions have the same mean, the alternate hypothesis is that the change is faster.

We got a ~23% speedup for this scenario.

(even better results on OrchardCore, but less rigorously obtained)

Notes

@JanProvaznik JanProvaznik changed the title Dev/janpro/perf fixes dotutils viewer perf fixes copy from dotutils and MSBuildStructuredLog Oct 10, 2024
src/Shared/ResourceUtilities.cs Outdated Show resolved Hide resolved
Copy link
Member

@YuliiaKovalova YuliiaKovalova left a comment

Choose a reason for hiding this comment

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

It's amazing ! Great job :)

@JanProvaznik JanProvaznik enabled auto-merge (squash) October 14, 2024 07:48
@JanProvaznik JanProvaznik merged commit b8aaabf into dotnet:main Oct 14, 2024
10 checks passed
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.

Copy perf fixes to BuildEventArgsReader from viewer repo
3 participants