From cb8f686d3b2f887b1d46316e33b71922d9b33dfb Mon Sep 17 00:00:00 2001
From: Cijo Thomas <cithomas@microsoft.com>
Date: Fri, 10 Sep 2021 10:01:00 -0700
Subject: [PATCH] Modify MetricPoint to avoid copy (#2321)

---
 src/OpenTelemetry/Metrics/BatchMetricPoint.cs | 30 ++++++++--------
 .../Metrics/MetricCollectBenchmarks.cs        | 35 +++++++++++++------
 2 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/src/OpenTelemetry/Metrics/BatchMetricPoint.cs b/src/OpenTelemetry/Metrics/BatchMetricPoint.cs
index c18a58c1ba1..9223d0f49a3 100644
--- a/src/OpenTelemetry/Metrics/BatchMetricPoint.cs
+++ b/src/OpenTelemetry/Metrics/BatchMetricPoint.cs
@@ -64,15 +64,20 @@ public struct Enumerator : IEnumerator
 
             internal Enumerator(MetricPoint[] metricsPoints, long targetCount, DateTimeOffset start, DateTimeOffset end)
             {
-                this.Current = default;
                 this.metricsPoints = metricsPoints;
                 this.targetCount = targetCount;
-                this.index = 0;
+                this.index = -1;
                 this.start = start;
                 this.end = end;
             }
 
-            public MetricPoint Current { get; private set; }
+            public ref MetricPoint Current
+            {
+                get
+                {
+                    return ref this.metricsPoints[this.index];
+                }
+            }
 
             /// <inheritdoc/>
             object IEnumerator.Current => this.Current;
@@ -84,22 +89,19 @@ public void Dispose()
             /// <inheritdoc/>
             public bool MoveNext()
             {
-                var metricPoints = this.metricsPoints;
-                if (this.index < this.targetCount && metricPoints[this.index].StartTime == default)
+                while (++this.index < this.targetCount)
                 {
-                    this.index++;
-                }
+                    ref var metricPoint = ref this.metricsPoints[this.index];
+                    if (metricPoint.StartTime == default)
+                    {
+                        continue;
+                    }
 
-                if (this.index < this.targetCount)
-                {
-                    metricPoints[this.index].StartTime = this.start;
-                    metricPoints[this.index].EndTime = this.end;
-                    this.Current = metricPoints[this.index];
-                    this.index++;
+                    metricPoint.StartTime = this.start;
+                    metricPoint.EndTime = this.end;
                     return true;
                 }
 
-                this.Current = default;
                 return false;
             }
 
diff --git a/test/Benchmarks/Metrics/MetricCollectBenchmarks.cs b/test/Benchmarks/Metrics/MetricCollectBenchmarks.cs
index 47696bdd297..c16a8ecdc13 100644
--- a/test/Benchmarks/Metrics/MetricCollectBenchmarks.cs
+++ b/test/Benchmarks/Metrics/MetricCollectBenchmarks.cs
@@ -32,10 +32,10 @@
   DefaultJob : .NET Core 3.1.18 (CoreCLR 4.700.21.35901, CoreFX 4.700.21.36305), X64 RyuJIT
 
 
-|  Method | ExportDelta |      Mean |    Error |   StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
-|-------- |------------ |----------:|---------:|---------:|------:|------:|------:|----------:|
-| Collect |       False | 100.59 us | 1.970 us | 5.490 us |     - |     - |     - |     136 B |
-| Collect |        True |  98.41 us | 1.861 us | 4.670 us |     - |     - |     - |     136 B |
+|  Method | UseWithRef |     Mean |    Error |   StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
+|-------- |----------- |---------:|---------:|---------:|------:|------:|------:|----------:|
+| Collect |      False | 51.38 us | 1.027 us | 1.261 us |     - |     - |     - |     136 B |
+| Collect |       True | 33.86 us | 0.716 us | 2.088 us |     - |     - |     - |     136 B |
 */
 
 namespace Benchmarks.Metrics
@@ -43,7 +43,7 @@ namespace Benchmarks.Metrics
     [MemoryDiagnoser]
     public class MetricCollectBenchmarks
     {
-        private Counter<long> counter;
+        private Counter<double> counter;
         private MeterProvider provider;
         private Meter meter;
         private CancellationTokenSource token;
@@ -55,18 +55,33 @@ public class MetricCollectBenchmarks
         private Random random = new Random();
 
         [Params(false, true)]
-        public bool ExportDelta { get; set; }
+        public bool UseWithRef { get; set; }
 
         [GlobalSetup]
         public void Setup()
         {
-            var metricExporter = new TestMetricExporter(ProcessExport, this.ExportDelta ? AggregationTemporality.Delta : AggregationTemporality.Cumulative);
+            var metricExporter = new TestMetricExporter(ProcessExport);
             void ProcessExport(IEnumerable<Metric> batch)
             {
+                double sum = 0;
                 foreach (var metric in batch)
                 {
-                    foreach (var metricPoint in metric.GetMetricPoints())
+                    if (this.UseWithRef)
                     {
+                        // The performant way of iterating.
+                        foreach (ref var metricPoint in metric.GetMetricPoints())
+                        {
+                            sum += metricPoint.LongValue;
+                        }
+                    }
+                    else
+                    {
+                        // The non-performant way of iterating.
+                        // This is still "correct", but less performant.
+                        foreach (var metricPoint in metric.GetMetricPoints())
+                        {
+                            sum += metricPoint.LongValue;
+                        }
                     }
                 }
             }
@@ -78,7 +93,7 @@ void ProcessExport(IEnumerable<Metric> batch)
                 .Build();
 
             this.meter = new Meter("TestMeter");
-            this.counter = this.meter.CreateCounter<long>("counter");
+            this.counter = this.meter.CreateCounter<double>("counter");
             this.token = new CancellationTokenSource();
             this.writeMetricTask = new Task(() =>
             {
@@ -87,7 +102,7 @@ void ProcessExport(IEnumerable<Metric> batch)
                     var tag1 = new KeyValuePair<string, object>("DimName1", this.dimensionValues[this.random.Next(0, 10)]);
                     var tag2 = new KeyValuePair<string, object>("DimName2", this.dimensionValues[this.random.Next(0, 10)]);
                     var tag3 = new KeyValuePair<string, object>("DimName3", this.dimensionValues[this.random.Next(0, 10)]);
-                    this.counter.Add(100, tag1, tag2, tag3);
+                    this.counter.Add(100.00, tag1, tag2, tag3);
                 }
             });
             this.writeMetricTask.Start();