From eb5f3c44e0552b6081dc2ea2cfd218eb767aedc9 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Fri, 21 Nov 2025 01:12:48 +0000
Subject: [PATCH 1/5] Initial plan
From 4919dadc70bb4c25863d65124148d869d071e7b4 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Fri, 21 Nov 2025 01:23:22 +0000
Subject: [PATCH 2/5] Add test cases and fix for Histogram.AddMetric bug
Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
---
src/TraceEvent/Stacks/Histogram.cs | 25 +++--
.../TraceEvent.Tests/Stacks/HistogramTests.cs | 94 +++++++++++++++++++
2 files changed, 109 insertions(+), 10 deletions(-)
create mode 100644 src/TraceEvent/TraceEvent.Tests/Stacks/HistogramTests.cs
diff --git a/src/TraceEvent/Stacks/Histogram.cs b/src/TraceEvent/Stacks/Histogram.cs
index 0f13227e2..c52cdb68d 100644
--- a/src/TraceEvent/Stacks/Histogram.cs
+++ b/src/TraceEvent/Stacks/Histogram.cs
@@ -49,20 +49,25 @@ public void AddMetric(float metric, int bucket)
{
Debug.Assert(0 <= bucket && bucket < Count);
- if (m_singleBucketNum < 0)
- {
- m_singleBucketNum = bucket;
- }
-
- if (m_singleBucketNum == bucket)
- {
- m_singleBucketValue += metric;
- return;
- }
if (m_buckets == null)
{
+ // We're in single-bucket mode
+ if (m_singleBucketNum < 0)
+ {
+ m_singleBucketNum = bucket;
+ }
+
+ if (m_singleBucketNum == bucket)
+ {
+ m_singleBucketValue += metric;
+ return;
+ }
+
+ // Need to transition to array mode
m_buckets = new float[Count];
m_buckets[m_singleBucketNum] = m_singleBucketValue;
+ // Clear the single bucket tracking since we're now using the array
+ m_singleBucketNum = -1;
}
m_buckets[bucket] += metric;
}
diff --git a/src/TraceEvent/TraceEvent.Tests/Stacks/HistogramTests.cs b/src/TraceEvent/TraceEvent.Tests/Stacks/HistogramTests.cs
new file mode 100644
index 000000000..c5abab123
--- /dev/null
+++ b/src/TraceEvent/TraceEvent.Tests/Stacks/HistogramTests.cs
@@ -0,0 +1,94 @@
+using Microsoft.Diagnostics.Tracing.Stacks;
+using Xunit;
+
+namespace TraceEventTests.Stacks
+{
+ public class HistogramTests
+ {
+ ///
+ /// Test for the bug where AddMetric doesn't properly transition from single-bucket to array mode.
+ /// After creating the array, subsequent calls to AddMetric with the original bucket should update
+ /// the array, not the m_singleBucketValue.
+ ///
+ [Fact]
+ public void AddMetric_TransitionToArrayMode_OriginalBucketUpdatesArray()
+ {
+ // Create a simple histogram controller for testing
+ var stackSource = new CopyStackSource();
+ var callTree = new CallTree(ScalingPolicyKind.TimeMetric)
+ {
+ StackSource = stackSource
+ };
+ var controller = new TimeHistogramController(callTree, 0, 100);
+ var histogram = new Histogram(controller);
+
+ // Step 1: Add metric to bucket 0 (single bucket mode)
+ histogram.AddMetric(1.0f, 0);
+ Assert.Equal(1.0f, histogram[0]);
+
+ // Step 2: Add metric to bucket 1 (should trigger array creation)
+ histogram.AddMetric(2.0f, 1);
+ Assert.Equal(1.0f, histogram[0]); // Bucket 0 should still have 1.0
+ Assert.Equal(2.0f, histogram[1]); // Bucket 1 should have 2.0
+
+ // Step 3: Add metric to bucket 0 again (this is where the bug manifests)
+ // The value should be added to the array, not to m_singleBucketValue
+ histogram.AddMetric(3.0f, 0);
+ Assert.Equal(4.0f, histogram[0]); // Bucket 0 should now have 1.0 + 3.0 = 4.0
+ Assert.Equal(2.0f, histogram[1]); // Bucket 1 should still have 2.0
+ }
+
+ ///
+ /// Test that multiple buckets work correctly after transitioning to array mode.
+ ///
+ [Fact]
+ public void AddMetric_MultipleBuckets_AllValuesCorrect()
+ {
+ var stackSource = new CopyStackSource();
+ var callTree = new CallTree(ScalingPolicyKind.TimeMetric)
+ {
+ StackSource = stackSource
+ };
+ var controller = new TimeHistogramController(callTree, 0, 100);
+ var histogram = new Histogram(controller);
+
+ // Add to multiple buckets in various order
+ histogram.AddMetric(10.0f, 5);
+ histogram.AddMetric(20.0f, 10);
+ histogram.AddMetric(30.0f, 5); // Add more to bucket 5
+ histogram.AddMetric(40.0f, 15);
+ histogram.AddMetric(50.0f, 10); // Add more to bucket 10
+
+ // Verify all values are correct
+ Assert.Equal(0.0f, histogram[0]);
+ Assert.Equal(40.0f, histogram[5]); // 10 + 30
+ Assert.Equal(70.0f, histogram[10]); // 20 + 50
+ Assert.Equal(40.0f, histogram[15]);
+ }
+
+ ///
+ /// Test single bucket mode (when only one bucket is ever used).
+ ///
+ [Fact]
+ public void AddMetric_SingleBucketOnly_ValuesCorrect()
+ {
+ var stackSource = new CopyStackSource();
+ var callTree = new CallTree(ScalingPolicyKind.TimeMetric)
+ {
+ StackSource = stackSource
+ };
+ var controller = new TimeHistogramController(callTree, 0, 100);
+ var histogram = new Histogram(controller);
+
+ // Add multiple values to the same bucket
+ histogram.AddMetric(1.0f, 7);
+ histogram.AddMetric(2.0f, 7);
+ histogram.AddMetric(3.0f, 7);
+
+ // Verify the value accumulated correctly
+ Assert.Equal(6.0f, histogram[7]);
+ Assert.Equal(0.0f, histogram[0]);
+ Assert.Equal(0.0f, histogram[10]);
+ }
+ }
+}
From 0f52fc3733b5ecfa6801ef346f0a85fe97ab2e96 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Fri, 21 Nov 2025 01:28:41 +0000
Subject: [PATCH 3/5] Add test for AddScaled after array transition
---
.../TraceEvent.Tests/Stacks/HistogramTests.cs | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/src/TraceEvent/TraceEvent.Tests/Stacks/HistogramTests.cs b/src/TraceEvent/TraceEvent.Tests/Stacks/HistogramTests.cs
index c5abab123..5c47e642a 100644
--- a/src/TraceEvent/TraceEvent.Tests/Stacks/HistogramTests.cs
+++ b/src/TraceEvent/TraceEvent.Tests/Stacks/HistogramTests.cs
@@ -90,5 +90,35 @@ public void AddMetric_SingleBucketOnly_ValuesCorrect()
Assert.Equal(0.0f, histogram[0]);
Assert.Equal(0.0f, histogram[10]);
}
+
+ ///
+ /// Test AddScaled works correctly after histogram transition to array mode.
+ ///
+ [Fact]
+ public void AddScaled_AfterArrayTransition_ValuesCorrect()
+ {
+ var stackSource = new CopyStackSource();
+ var callTree = new CallTree(ScalingPolicyKind.TimeMetric)
+ {
+ StackSource = stackSource
+ };
+ var controller = new TimeHistogramController(callTree, 0, 100);
+
+ // Create source histogram with multiple buckets
+ var sourceHistogram = new Histogram(controller);
+ sourceHistogram.AddMetric(10.0f, 5);
+ sourceHistogram.AddMetric(20.0f, 10);
+
+ // Create target histogram that starts in single-bucket mode
+ var targetHistogram = new Histogram(controller);
+ targetHistogram.AddMetric(5.0f, 5);
+
+ // Add scaled source to target - this should transition target to array mode
+ targetHistogram.AddScaled(sourceHistogram);
+
+ // Verify values
+ Assert.Equal(15.0f, targetHistogram[5]); // 5 + 10
+ Assert.Equal(20.0f, targetHistogram[10]); // 0 + 20
+ }
}
}
From ad3fdbbc5d8c6c1e2dfeabb6947b81fa91f2bbe9 Mon Sep 17 00:00:00 2001
From: Brian Robbins
Date: Mon, 12 Jan 2026 14:38:06 -0800
Subject: [PATCH 4/5] Clear m_singleBucketValue when expanding to array.
---
src/TraceEvent/Stacks/Histogram.cs | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/TraceEvent/Stacks/Histogram.cs b/src/TraceEvent/Stacks/Histogram.cs
index c52cdb68d..9c220a72b 100644
--- a/src/TraceEvent/Stacks/Histogram.cs
+++ b/src/TraceEvent/Stacks/Histogram.cs
@@ -68,6 +68,7 @@ public void AddMetric(float metric, int bucket)
m_buckets[m_singleBucketNum] = m_singleBucketValue;
// Clear the single bucket tracking since we're now using the array
m_singleBucketNum = -1;
+ m_singleBucketValue = 0;
}
m_buckets[bucket] += metric;
}
From 36c6428b27816c6c9c28ffbf0f00ce89e44871c6 Mon Sep 17 00:00:00 2001
From: Brian Robbins
Date: Tue, 13 Jan 2026 10:48:51 -0800
Subject: [PATCH 5/5] Code Review Feedback
---
src/TraceEvent/Stacks/Histogram.cs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/TraceEvent/Stacks/Histogram.cs b/src/TraceEvent/Stacks/Histogram.cs
index 9c220a72b..1b9cfed35 100644
--- a/src/TraceEvent/Stacks/Histogram.cs
+++ b/src/TraceEvent/Stacks/Histogram.cs
@@ -47,11 +47,11 @@ public void AddSample(StackSourceSample sample)
/// The bucket to add to.
public void AddMetric(float metric, int bucket)
{
- Debug.Assert(0 <= bucket && bucket < Count);
+ Debug.Assert(0 <= bucket && bucket < Count, $"Bucket index is out of range. Bucket: {bucket}, Count: {Count}");
if (m_buckets == null)
{
- // We're in single-bucket mode
+ // We have not expanded to multiple buckets yet
if (m_singleBucketNum < 0)
{
m_singleBucketNum = bucket;