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

Check performance with/without analyzers. #9988

Closed
Tracked by #9829
AR-May opened this issue Apr 10, 2024 · 11 comments
Closed
Tracked by #9829

Check performance with/without analyzers. #9988

AR-May opened this issue Apr 10, 2024 · 11 comments

Comments

@AR-May
Copy link
Member

AR-May commented Apr 10, 2024

Context
When analyzers are disabled ('/analyze' not passed on command line) there should be no to minimal impact on the build perf. There should be an acceptable overhead for when the analyzers are enabled.

Goals

  • Check the performance of current state (with and without analyzers).
@AR-May AR-May self-assigned this Apr 10, 2024
@AR-May AR-May added the triaged label Apr 10, 2024
@AR-May
Copy link
Member Author

AR-May commented Apr 10, 2024

I have checked on our automated tests in PerfStar and I do not notice any regression without the analyzers on.

As for the run of tests with analyzers, I see insignificant regression (0.4% with previous CI run and 1.4% with 30-days mean) which is on the top edge of the possible variance.

I would say it is an acceptable overhead for the new capabilities. However, not many analyzers are implemented, so it is expected that the overhead would not be high at this moment of development.

TODO: run manual perf tests on bigger repositories.

@JanKrivanek
Copy link
Member

FYI @baronfel

@baronfel
Copy link
Member

This is an encouraging result! Can you link to the perfstar runs/charts here for posterity?

@AR-May
Copy link
Member Author

AR-May commented Apr 18, 2024

I have some screenshots:
image
image

@AR-May
Copy link
Member Author

AR-May commented Apr 24, 2024

@JanKrivanek checked current main: ran msbuild on OC and console app solutions with and without analyzers.

OC rebuild with analyzers without analyzers
median 85053 73716
mean 88154 77364
     
console app rebuild with analyzers without analyzers
median 8982 8912
mean 8969 8915

For small solutions overhead is small, but for bigger one it is quite noticeable.

@JanKrivanek
Copy link
Member

Oh - that is almost 14% :-O
Do we have any mid or larger size solution in the batter of repos in PerfStar? It's be nice to be able to catch those slowdowns autmatically.

As for now - we'll likely need a measurement of OC build

  • with MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION env variable set system wide to "1" (this is required by the analyzers currently to provide data for analysis).
  • with the sole analyzer turned off -
    new BuildAnalyzerConfiguration() { Severity = BuildAnalyzerResultSeverity.Warning, IsEnabled = true });
    IsEnabled set to false. So that we can disect the impact of infra and the analyzer.

@AR-May - can you create items and handle those?

It'd be good to test on a version without editorconfig changes (#9811) merged - as that will bring yet another big variable to the perf equation.
It might be interesting to test with #10016 (once it's finlized) - as it'd get the tracing a good test-drive (with ability to compare the results reported by TracingModule and actual 'wall clock time measurements') - FYI @maridematte (you might want to team up with @AR-May on this)

@AR-May
Copy link
Member Author

AR-May commented Apr 24, 2024

Do we have any mid or larger size solution in the batter of repos in PerfStar? It's be nice to be able to catch those slowdowns automatically.

They are currently broken; we have an issue to fix them: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1896972

I will create items and investigate further the source of this perf issue. I agree that's a big difference.

It'd be good to test on a version without editorconfig changes (#9811) merged - as that will bring yet another big variable to the perf equation.

editorconfig changes are not yet merged, so I tested without them.

@AR-May
Copy link
Member Author

AR-May commented Apr 24, 2024

with MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION env variable set system wide to "1"

Done, here it is. Does not explain the difference in ~10 seconds of the build time.

Time Elapsed 00:01:32.61
MSBuild.SharedOutputPathAnalyzer: 00:00:00.0213450

@AR-May
Copy link
Member Author

AR-May commented Apr 24, 2024

with the sole analyzer turned off

I turned off the analyzer and ran again with /analyze and without. Also ran a baseline with commit fc97b2d

Results:

OC rebuild with analyze without analyze baseline
median 81445 71151 72182
mean 82251.14286 73447.57143 71997

It seems like most of the perf overhead lays in the infra part and not in the only analyzer.
The baseline and run without /analyze are same, difference is withing the variance.

@AR-May
Copy link
Member Author

AR-May commented Apr 24, 2024

With @maridematte changes:

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:01:30.07
BuildCheck run times

Infrastructure run times
analyzerAcquisitionTime,00:00:00
analyzerSetDataSourceTime,00:00:00.0000075
newProjectAnalyzersTime,00:00:00.0001385

Analyzer run times
MSBuild.SharedOutputPathAnalyzer,00:00:00.0153347

@AR-May
Copy link
Member Author

AR-May commented Apr 24, 2024

Closing this issue - we measured the perf.
Created a new one to investigate (and hopefully fix) the perf issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants