Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 21, 2025

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 == null first to determine mode before applying single-bucket optimization
  • Clear single-bucket tracking on transition: Set m_singleBucketNum = -1 after 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:

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)
{
m_buckets = new float[Count];
m_buckets[m_singleBucketNum] = m_singleBucketValue;
}
m_buckets[bucket] += metric;
}

Let's say the following sequence of AddMetric is 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_singleBucketValue is populated into the array.
But within the third call, if (m_singleBucketNum == bucket) will still be true, and the metric is still added into m_singleBucketValue instead of the array.
Inside the indexer:

if (m_buckets != null)
{
return m_buckets[index];
}
else if (m_singleBucketNum == index)
{
return m_singleBucketValue;
}

the m_singleBucketValue isn't used when the array is not null, causing the entry for m_singleBucketNum to 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)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix bug in Histogram metric addition Fix Histogram.AddMetric losing values after single-bucket to array transition Nov 21, 2025
Copilot AI requested a review from brianrob November 21, 2025 01:31
Copilot finished work on behalf of brianrob November 21, 2025 01:31
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.

A bug in Histogram?

2 participants