-
Notifications
You must be signed in to change notification settings - Fork 744
Description
I was looking around the source code and the following piece of code seems a bit buggy to me:
perfview/src/TraceEvent/Stacks/Histogram.cs
Lines 48 to 68 in c671d46
| 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:
perfview/src/TraceEvent/Stacks/Histogram.cs
Lines 119 to 126 in c671d46
| 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.