Add an aggregate logger for inheritance margin#60493
Merged
Cosifne merged 2 commits intodotnet:mainfrom Mar 31, 2022
Merged
Conversation
ghost
added
the
dibarbet
approved these changes
Mar 31, 2022
| internal static class InheritanceMarginLogger | ||
| { | ||
| // 1 sec per bucket, and if it takes more than 1 min, then this log is considered as time-out in the last bucket. | ||
| private static readonly HistogramLogAggregator s_histogramLogAggregator = new(1000, 60000); |
Contributor
There was a problem hiding this comment.
Suggested change
| private static readonly HistogramLogAggregator s_histogramLogAggregator = new(1000, 60000); | |
| private static readonly HistogramLogAggregator s_histogramLogAggregator = new(TimeSpan.FromSecons(1), TimeSpan.FromMinutes(1)); |
Member
Author
There was a problem hiding this comment.
It seems like this is accepting int? https://sourceroslyn.io/#Microsoft.CodeAnalysis.Workspaces/Log/HistogramLogAggregator.cs,20
Member
Author
|
/azp run roslyn-CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
333fred
added a commit
that referenced
this pull request
Apr 4, 2022
…ures/semi-auto-props * upstream/main: (110 commits) Add Rebuild badge to README (#60298) Update PublishData.json for 17.3 P1 (#60559) Note auto-default merged in feature status doc (#60564) Update Roslyn.Diagnostics.Analyzers and remove RS0005 suppressions Cleanup unused resources in Features layer Remove unnecessary `<Compile Remove` Remove overrides in Features Remove overrides in Analyzers Remove the single unused read of CodeFixCategory Remove unused abstract property Update Language Feature Status.md (#60482) Document ROSLYN_TEST_USEDASSEMBLIES (#60478) Add BoundArrayInitialization.IsInferred (#60391) Add an aggregate logger for inheritance margin (#60493) Move StackTraceAnalyzer over to VirtualCharSequence (#60404) PR feedback Fix test and review feedback Update Spanish queue to Windows.10.Amd64.Server2022.ES.Open Enable NRT in AbstractSyncNamespaceCodeRefactoringProvider test ...
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
3 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.
As discussed offline, because 'GetInheritanceItem' is frequently triggered by the view tagger, it generates a lot of telemetry log.
This PR change it to use a HistogramLogger, which reports the time of 'GetInheritanceItem' based on seconds. (0s -> 60s)
I feel it should still meet our needs since it doesn't make a lot of sense to know things like
'It takes 5.321 seconds to get the background data ready',
If it takes more than 1min to get it, then it is put in the last bucket and we should consider it as a time-out log