Fix Histogram.AddMetric losing values after single-bucket to array transition
Histogram.AddMetric incorrectly updated m_singleBucketValue instead of the array after transitioning from single-bucket optimization to array mode, causing subsequent additions to the original bucket to be lost.
Root Cause
The method checked if (m_singleBucketNum == bucket) before checking if (m_buckets == null), so after array creation, calls to the original bucket still matched the single-bucket path but the indexer only read from the array.
Example failure sequence:
histogram.AddMetric(1.0f, 0); // m_singleBucketValue = 1.0
histogram.AddMetric(2.0f, 1); // Creates array, m_buckets[0] = 1.0, m_buckets[1] = 2.0
histogram.AddMetric(3.0f, 0); // BUG: adds to m_singleBucketValue, not m_buckets[0]
// histogram[0] returns 1.0 instead of 4.0
Changes
-
Restructured AddMetric logic: Check
m_buckets == nullfirst to determine mode before applying single-bucket optimization -
Clear single-bucket tracking on transition: Set
m_singleBucketNum = -1after creating array to prevent reentry into single-bucket path - Added test coverage: 4 tests covering the bug scenario, multiple bucket patterns, single-bucket mode preservation, and AddScaled interaction
Code Review Note
Review identified a pre-existing bug where AddScaled's weight parameter is not applied when the source histogram uses array mode (only applied in single-bucket mode). This is orthogonal to the reported issue and left unmodified per minimal change principle.
Original prompt
This section details on the original issue you should resolve
<issue_title>A bug in Histogram?</issue_title> <issue_description>I was looking around the source code and the following piece of code seems a bit buggy to me: https://github.com/microsoft/perfview/blob/c671d460fdd4abd915a96beb1410c8c7748432fb/src/TraceEvent/Stacks/Histogram.cs#L48-L68 Let's say the following sequence of
AddMetricis called:AddMetric(1.0f, 0); AddMetric(2.0f, 1); AddMetric(3.0f, 0);The second time will cause the array to be constructed, and
m_singleBucketValueis populated into the array. But within the third call,if (m_singleBucketNum == bucket)will still be true, and the metric is still added intom_singleBucketValueinstead of the array. Inside the indexer: https://github.com/microsoft/perfview/blob/c671d460fdd4abd915a96beb1410c8c7748432fb/src/TraceEvent/Stacks/Histogram.cs#L119-L126 them_singleBucketValueisn't used when the array is not null, causing the entry form_singleBucketNumto be returned with incorrected value.I don't have the whole knowledge of how Histogram is used, so I don't sure if this is really a bug. And this problem might not be a big issue due to the usage pattern, cause I don't see any bucket to be obviously wrong in the GUI.</issue_description>
Comments on the Issue (you are @copilot in this section)
- Fixes microsoft/perfview#2006
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.