Skip to content

Pool analyzer diagnostic reporters#39034

Merged
agocke merged 1 commit intodotnet:masterfrom
agocke:pool-diagnostic-reporters
Oct 9, 2019
Merged

Pool analyzer diagnostic reporters#39034
agocke merged 1 commit intodotnet:masterfrom
agocke:pool-diagnostic-reporters

Conversation

@agocke
Copy link
Member

@agocke agocke commented Oct 3, 2019

This has shown up as a significant source of allocations. The design
here is that when an analyzer reports a diagnostic we may need to filter
or alter it based on some configuration. Previously we stored the
configuration information by closing over captured variables, but that
could be very expensive if it's being done for every analyzer
invocation. Since we can only really execute one analyzer per running
thread on the machine, the actual number of concurrent objects we need
to create is approximately the number of running threads. Pooling is a
good solution to this problem.

Before:


|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 16.28 s | 0.149 s | 0.268 s | 16.23 s | 15.76 s | 16.97 s | 200000.0000 | 59000.0000 | 2000.0000 |   1.48 GB |

After:

|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 15.77 s | 0.176 s | 0.519 s | 15.67 s | 14.96 s | 17.07 s | 184000.0000 | 55000.0000 | 2000.0000 |   1.36 GB |

Note: I don't think this is that much faster -- my machine wasn't completely quiet during the noise measurements, but I do believe that bytes allocated is accurate and a significant improvement.

@agocke agocke force-pushed the pool-diagnostic-reporters branch 2 times, most recently from 1974cad to fc4bf87 Compare October 8, 2019 20:42
@agocke agocke changed the title WIP Pool analyzer diagnostic reporters Oct 8, 2019
@agocke agocke marked this pull request as ready for review October 8, 2019 20:46
@agocke agocke requested a review from a team as a code owner October 8, 2019 20:46
@agocke agocke force-pushed the pool-diagnostic-reporters branch 2 times, most recently from 80bb300 to bf2563a Compare October 8, 2019 21:30
This has shown up as a significant source of allocations. The design
here is that when an analyzer reports a diagnostic we may need to filter
or alter it based on some configuration. Previously we stored the
configuration information by closing over captured variables, but that
could be very expensive if it's being done for every analyzer
invocation. Since we can only really execute one analyzer per running
thread on the machine, the actual number of concurrent objects we need
to create is approximately the number of running threads. Pooling is a
good solution to this problem.
@agocke agocke force-pushed the pool-diagnostic-reporters branch from bf2563a to f34b4a7 Compare October 8, 2019 21:37
Copy link
Member

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

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

awesome!

@RikkiGibson RikkiGibson requested a review from a team October 8, 2019 21:48
Copy link
Member

@jaredpar jaredpar left a comment

Choose a reason for hiding this comment

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

:shipit:

@agocke agocke merged commit e2879fc into dotnet:master Oct 9, 2019
@agocke agocke deleted the pool-diagnostic-reporters branch October 9, 2019 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants