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

Metrics Perf Improvement- Use KeyValuePair<string, object>[] to store dimensions #4059

Merged
merged 11 commits into from
Jan 7, 2023

Conversation

utpilla
Copy link
Contributor

@utpilla utpilla commented Jan 6, 2023

On the metrics hot path, for every update call, we copy the metric dimensions received from the DiagnosticSource callback to find the right MetricPoint to be updated. This PR updates the internal structure used to store dimensions from a combination of string[] and object[] to KeyValuePair<string, object>[].

From the benchmark results, it looks like it takes lesser time to copy KeyValuePair<string, object>[] than copying both string[] and object[]. The perf benefit becomes more visible with higher number of dimensions.

Benchmark results

There is a considerable perf improvement across the board, with the perf improvement for seven dimensions (cumulative) going up to ~18%: an improvement of 60ns

BenchmarkDotNet=v0.13.3, OS=Windows 11 (10.0.22621.963)
Intel Core i7-9700 CPU 3.00GHz, 1 CPU, 8 logical and 8 physical cores
.NET SDK=7.0.101
[Host] : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2
DefaultJob : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2

main branch

Method AggregationTemporality Mean Error StdDev Allocated
CounterHotPath Cumulative 16.85 ns 0.363 ns 0.798 ns -
CounterWith1LabelsHotPath Cumulative 74.30 ns 0.925 ns 0.865 ns -
CounterWith3LabelsHotPath Cumulative 160.92 ns 3.157 ns 3.635 ns -
CounterWith5LabelsHotPath Cumulative 245.35 ns 4.856 ns 5.196 ns -
CounterWith6LabelsHotPath Cumulative 264.72 ns 1.103 ns 1.032 ns -
CounterWith7LabelsHotPath Cumulative 313.69 ns 4.041 ns 3.583 ns -
CounterHotPath Delta 15.37 ns 0.034 ns 0.030 ns -
CounterWith1LabelsHotPath Delta 63.10 ns 0.208 ns 0.194 ns -
CounterWith3LabelsHotPath Delta 149.17 ns 2.027 ns 1.896 ns -
CounterWith5LabelsHotPath Delta 234.79 ns 0.546 ns 0.427 ns -
CounterWith6LabelsHotPath Delta 261.82 ns 0.591 ns 0.524 ns -
CounterWith7LabelsHotPath Delta 297.78 ns 1.062 ns 0.887 ns -

With this PR

Method AggregationTemporality Mean Error StdDev Allocated
CounterHotPath Cumulative 14.45 ns 0.082 ns 0.068 ns -
CounterWith1LabelsHotPath Cumulative 60.54 ns 0.473 ns 0.442 ns -
CounterWith3LabelsHotPath Cumulative 134.96 ns 1.543 ns 1.368 ns -
CounterWith5LabelsHotPath Cumulative 206.31 ns 1.219 ns 1.081 ns -
CounterWith6LabelsHotPath Cumulative 230.43 ns 0.898 ns 0.840 ns -
CounterWith7LabelsHotPath Cumulative 254.62 ns 1.156 ns 1.082 ns -
CounterHotPath Delta 13.79 ns 0.021 ns 0.019 ns -
CounterWith1LabelsHotPath Delta 58.06 ns 0.227 ns 0.212 ns -
CounterWith3LabelsHotPath Delta 131.89 ns 0.304 ns 0.285 ns -
CounterWith5LabelsHotPath Delta 207.92 ns 1.050 ns 0.982 ns -
CounterWith6LabelsHotPath Delta 232.65 ns 1.011 ns 0.896 ns -
CounterWith7LabelsHotPath Delta 266.57 ns 5.200 ns 5.988 ns -

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

  • Appropriate CHANGELOG.md updated for non-trivial changes

@utpilla utpilla requested a review from a team January 6, 2023 01:05
@codecov
Copy link

codecov bot commented Jan 6, 2023

Codecov Report

Merging #4059 (8480145) into main (9708349) will increase coverage by 0.05%.
The diff coverage is 81.66%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4059      +/-   ##
==========================================
+ Coverage   85.54%   85.60%   +0.05%     
==========================================
  Files         289      289              
  Lines       11259    11226      -33     
==========================================
- Hits         9632     9610      -22     
+ Misses       1627     1616      -11     
Impacted Files Coverage Δ
src/OpenTelemetry/Metrics/ThreadStaticStorage.cs 70.83% <42.10%> (+8.86%) ⬆️
src/OpenTelemetry/Metrics/AggregatorStore.cs 82.65% <100.00%> (-0.49%) ⬇️
src/OpenTelemetry/Metrics/MetricPoint.cs 75.44% <100.00%> (ø)
src/OpenTelemetry/Metrics/Tags.cs 83.33% <100.00%> (+2.56%) ⬆️
src/OpenTelemetry/ReadOnlyTagCollection.cs 100.00% <100.00%> (ø)
...tation/OpenTelemetryProtocolExporterEventSource.cs 85.00% <0.00%> (-15.00%) ⬇️
...porter.OpenTelemetryProtocol/OtlpMetricExporter.cs 72.72% <0.00%> (-13.64%) ⬇️
...p/Implementation/HttpInstrumentationEventSource.cs 76.00% <0.00%> (+4.00%) ⬆️
...lementation/SqlClientInstrumentationEventSource.cs 75.00% <0.00%> (+4.16%) ⬆️
...Propagators/OpenTelemetryPropagatorsEventSource.cs 100.00% <0.00%> (+12.50%) ⬆️

Copy link
Member

@alanwest alanwest left a comment

Choose a reason for hiding this comment

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

🔥

@utpilla utpilla merged commit 1a02683 into open-telemetry:main Jan 7, 2023
@utpilla utpilla deleted the utpilla/Update-AggregatorStore branch November 23, 2023 03:35
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.

3 participants