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

Metric Aggregation metadata is stored into a field instead of properties dictionary #929

Merged
merged 5 commits into from
Oct 3, 2018

Conversation

cijothomas
Copy link
Contributor

@cijothomas cijothomas commented Oct 2, 2018

Metric Aggregation metadata is stored into a field instead of properties dictionary, to improve performance.
When someone read the properties, the field value is copied to the properties, so change in behavior.

Since metric aggregator sits before Sampling, this avoids the Properties (ConcurrentDictionary) access until late in the pipeline (serialization). In effect, for items that are dropped after sampling, no Properties access.

Fix #930

For significant contributions please make sure you have completed the following items:

  • Design discussion issue #
  • Changes in public surface reviewed
  • CHANGELOG.md updated with one line description of the fix, and a link to the original issue.
  • The PR will trigger build, unit test, and functional tests automatically. If your PR was submitted from fork - mention one of committers to initiate the build for you.

…ties in Properties Getter method. This should improve performance as MetricExtractor sits before sampling, and accessing Properties ConcurrentDictionary is avoided until later in the pipeline. With sampling most items are dropped and hence this gives perf boost.
@cijothomas cijothomas closed this Oct 2, 2018
@cijothomas cijothomas reopened this Oct 2, 2018
@cijothomas cijothomas closed this Oct 2, 2018
@cijothomas cijothomas reopened this Oct 2, 2018
@cijothomas cijothomas closed this Oct 2, 2018
@cijothomas cijothomas reopened this Oct 2, 2018
{
extractionPipelineInfo = extractionPipelineInfo + "; ";
}
extractionPipelineInfo = String.Empty;
Copy link
Contributor

Choose a reason for hiding this comment

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

is stylecop fine with String uppercased? I'm surprised

@cijothomas cijothomas merged commit a8db87a into develop Oct 3, 2018
TimothyMothra added a commit that referenced this pull request Oct 25, 2019
@cijothomas cijothomas deleted the cithomas/metricinfoasfieldforperf branch December 16, 2021 19:05
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.

Store Metric Aggregation metadata into a field rather than properties dictionary to improve performance
3 participants