Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Code changes
  • Loading branch information
utpilla committed Sep 12, 2023
commit dc20feb8a36b51cb088dc5d3f14b9080b2a0e1db
11 changes: 7 additions & 4 deletions src/OpenTelemetry/Metrics/AggregatorStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,22 +109,25 @@ internal AggregatorStore(

this.emitOverflowAttribute = emitOverflowAttribute;

var reservedMetricPointsCount = 1;

if (emitOverflowAttribute)
{
// Setting metricPointIndex to 1 as we would reserve the metricPoints[1] for overflow attribute.
// Newer attributes should be added starting at the index: 2
this.metricPointIndex = 1;
reservedMetricPointsCount++;
}

if (this.OutputDelta)
{
this.availableMetricPoints = new Queue<int>(maxMetricPoints - 1);
this.availableMetricPoints = new Queue<int>(maxMetricPoints - reservedMetricPointsCount);

// There is no overload which only takes capacity as the parameter
// Using the DefaultConcurrencyLevel defined in the ConcurrentDictionary class: https://github.com/dotnet/runtime/blob/v7.0.5/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs#L2020
// We expect at the most (maxMetricPoints - 1) * 2 entries- one for sorted and one for unsorted input
// We expect at the most (maxMetricPoints - reservedMetricPointsCount) * 2 entries- one for sorted and one for unsorted input
this.tagsToMetricPointIndexDictionaryDelta =
new ConcurrentDictionary<Tags, LookupData>(concurrencyLevel: Environment.ProcessorCount, capacity: (maxMetricPoints - 1) * 2);
new ConcurrentDictionary<Tags, LookupData>(concurrencyLevel: Environment.ProcessorCount, capacity: (maxMetricPoints - reservedMetricPointsCount) * 2);

this.metricPointReclamationThreshold = maxMetricPoints * 3 / 4;

Expand Down Expand Up @@ -792,7 +795,7 @@ private int RemoveStaleEntriesAndGetAvailableMetricPointRare(LookupData lookupDa
// Try to remove stale entries from dictionary
// Get the index for a new MetricPoint (it could be self-claimed or from another thread that added a fresh entry)
// If self-claimed, then add a fresh entry to the dictionary
// If an available MetricPoint is found, then increment the ReferenceCount
// If an available MetricPoint is found, then only increment the ReferenceCount

// Delete the entry for these Tags and get another MetricPoint.
lock (this.tagsToMetricPointIndexDictionaryDelta)
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Metrics/LookupData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal sealed class LookupData
public Tags SortedTags;
public Tags GivenTags;

public LookupData(int index, Tags sortedTags, Tags givenTags)
public LookupData(int index, in Tags sortedTags, in Tags givenTags)
{
this.Index = index;
this.SortedTags = sortedTags;
Expand Down
5 changes: 3 additions & 2 deletions src/OpenTelemetry/Metrics/MetricPoint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ public struct MetricPoint
{
// Represents the number of update threads using this MetricPoint at any given point of time.
// If the value is equal to int.MinValue which is -2147483648, it means that this MetricPoint is available for reuse.
// We never increment the ReferenceCount for MetricPoint with no tags (index == 0) but we always decrement it (in the Update methods).
// This should be fine. ReferenceCount doesn't matter for MetricPoint with no tags as it is never reclaimed.
// We never increment the ReferenceCount for MetricPoint with no tags (index == 0) and the MetricPoint for overflow attribute,
// but we always decrement it (in the Update methods). This should be fine.
// ReferenceCount doesn't matter for MetricPoint with no tags and overflow attribute as they are never reclaimed.
internal int ReferenceCount;
Copy link
Member

Choose a reason for hiding this comment

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

@alanwest @vishweshbankwar This is going to add to our memory consumption (4 bytes x MaxMetricPoints). Below there is another field AggregationType aggType which I'm guessing is also taking up 4 bytes. I don't think either of those two things really need the full 4 bytes, what if we did some bit manipulation on a single int field to process both needs with better memory utilization?


// When the AggregatorStore is reclaiming MetricPoints, this serves the purpose of validating the a given thread is using the right
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Metrics/Tags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace OpenTelemetry.Metrics;

internal readonly struct Tags : IEquatable<Tags>
{
public static Tags EmptyTags = new Tags(Array.Empty<KeyValuePair<string, object>>());
public static readonly Tags EmptyTags = new Tags(Array.Empty<KeyValuePair<string, object>>());

private readonly int hashCode;

Expand Down
32 changes: 16 additions & 16 deletions test/Benchmarks/Metrics/MetricsBenchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,27 @@
using OpenTelemetry.Tests;

/*
BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.23424.1000)
BenchmarkDotNet v0.13.6, Windows 11 (10.0.23424.1000)
Intel Core i7-9700 CPU 3.00GHz, 1 CPU, 8 logical and 8 physical cores
.NET SDK=7.0.203
[Host] : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2
DefaultJob : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2
.NET SDK 7.0.400
[Host] : .NET 7.0.10 (7.0.1023.36312), X64 RyuJIT AVX2
DefaultJob : .NET 7.0.10 (7.0.1023.36312), X64 RyuJIT AVX2


| Method | AggregationTemporality | Mean | Error | StdDev | Allocated |
|-------------------------- |----------------------- |----------:|---------:|---------:|----------:|
| CounterHotPath | Cumulative | 21.62 ns | 0.201 ns | 0.188 ns | - |
| CounterWith1LabelsHotPath | Cumulative | 71.12 ns | 0.509 ns | 0.476 ns | - |
| CounterWith3LabelsHotPath | Cumulative | 156.46 ns | 1.512 ns | 1.340 ns | - |
| CounterWith5LabelsHotPath | Cumulative | 235.59 ns | 1.273 ns | 1.190 ns | - |
| CounterWith6LabelsHotPath | Cumulative | 264.38 ns | 2.671 ns | 2.368 ns | - |
| CounterWith7LabelsHotPath | Cumulative | 302.73 ns | 1.558 ns | 1.457 ns | - |
| CounterHotPath | Delta | 27.11 ns | 0.194 ns | 0.172 ns | - |
| CounterWith1LabelsHotPath | Delta | 90.23 ns | 0.270 ns | 0.225 ns | - |
| CounterWith3LabelsHotPath | Delta | 165.63 ns | 0.671 ns | 0.524 ns | - |
| CounterWith5LabelsHotPath | Delta | 254.29 ns | 0.897 ns | 0.795 ns | - |
| CounterWith6LabelsHotPath | Delta | 281.36 ns | 1.066 ns | 0.945 ns | - |
| CounterWith7LabelsHotPath | Delta | 316.55 ns | 3.161 ns | 2.957 ns | - |
| CounterHotPath | Cumulative | 21.61 ns | 0.084 ns | 0.078 ns | - |
| CounterWith1LabelsHotPath | Cumulative | 69.08 ns | 0.261 ns | 0.244 ns | - |
| CounterWith3LabelsHotPath | Cumulative | 149.77 ns | 0.549 ns | 0.486 ns | - |
| CounterWith5LabelsHotPath | Cumulative | 236.47 ns | 1.684 ns | 1.493 ns | - |
| CounterWith6LabelsHotPath | Cumulative | 276.48 ns | 1.442 ns | 1.349 ns | - |
| CounterWith7LabelsHotPath | Cumulative | 294.09 ns | 2.354 ns | 2.202 ns | - |
| CounterHotPath | Delta | 27.32 ns | 0.380 ns | 0.355 ns | - |
| CounterWith1LabelsHotPath | Delta | 80.83 ns | 0.219 ns | 0.183 ns | - |
| CounterWith3LabelsHotPath | Delta | 162.48 ns | 1.053 ns | 0.985 ns | - |
| CounterWith5LabelsHotPath | Delta | 255.48 ns | 1.807 ns | 1.602 ns | - |
| CounterWith6LabelsHotPath | Delta | 281.75 ns | 2.761 ns | 2.583 ns | - |
| CounterWith7LabelsHotPath | Delta | 310.29 ns | 1.817 ns | 1.611 ns | - |
*/

namespace Benchmarks.Metrics;
Expand Down