Skip to content

Commit

Permalink
Fix incorrect accumulation count.
Browse files Browse the repository at this point in the history
The previous count was increment every time the result was negative
which meant it was counted many times since a negative count is likely
to remain negative for some time.

Also add some new histograms that are overflowing to the enum.

Bug: 682680
Change-Id: Ib7c7c5040c6a507e3b125280e975db4bff37a56b
Reviewed-on: https://chromium-review.googlesource.com/718977
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508798}
  • Loading branch information
Brian White authored and Commit Bot committed Oct 13, 2017
1 parent 8ebaa4a commit d7b4dac
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
7 changes: 5 additions & 2 deletions base/metrics/persistent_sample_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ enum NegativeSampleReason {
PERSISTENT_SPARSE_ADD_OVERFLOW,
PERSISTENT_SPARSE_ACCUMULATE_NEGATIVE_COUNT,
PERSISTENT_SPARSE_ACCUMULATE_WENT_NEGATIVE,
DEPRECATED_PERSISTENT_SPARSE_ACCUMULATE_OVERFLOW,
PERSISTENT_SPARSE_ACCUMULATE_OVERFLOW,
MAX_NEGATIVE_SAMPLE_REASONS
};
Expand Down Expand Up @@ -132,8 +133,10 @@ void PersistentSampleMap::Accumulate(Sample value, Count count) {
reason = PERSISTENT_SPARSE_ACCUMULATE_WENT_NEGATIVE;
*local_count_ptr += count;
} else {
*local_count_ptr += count;
if (*local_count_ptr < 0)
Sample old_value = *local_count_ptr;
Sample new_value = old_value + count;
*local_count_ptr = new_value;
if ((new_value >= 0) != (old_value >= 0))
reason = PERSISTENT_SPARSE_ACCUMULATE_OVERFLOW;
}
if (reason != MAX_NEGATIVE_SAMPLE_REASONS) {
Expand Down
11 changes: 9 additions & 2 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19792,6 +19792,8 @@ uploading your change for review. These are checked by presubmit scripts.
<int value="-2055973512" label="Login.ConsumerNewUsersAllowed"/>
<int value="-2021219906"
label="DataUse.MessageSize.AllServices.Downstream.Foreground.NotCellular"/>
<int value="-1724216561"
label="Extensions.Functions.SucceededTime.LessThan1ms"/>
<int value="-1620411628"
label="DataUse.MessageSize.AllServices.Downstream.Unknown.NotCellular"/>
<int value="-1607339868"
Expand All @@ -19818,6 +19820,7 @@ uploading your change for review. These are checked by presubmit scripts.
label="DataUse.MessageSize.AllServices.Downstream.Unknown.Cellular"/>
<int value="424952287"
label="Network.Shill.Wifi.LinkMonitorResponseTimeSample"/>
<int value="433324211" label="Extensions.FunctionCalls"/>
<int value="628921860"
label="Network.Shill.Ethernet.LinkMonitorResponseTimeSample"/>
<int value="662206917"
Expand All @@ -19826,6 +19829,7 @@ uploading your change for review. These are checked by presubmit scripts.
label="DataUse.MessageSize.AllServices.Upstream.Foreground.Cellular"/>
<int value="1370064090"
label="DataUse.MessageSize.AllServices.Upstream.Background.Cellular"/>
<int value="1514844119" label="DataUse.Sync.Upload.Bytes"/>
<int value="1753226325"
label="DataUse.MessageSize.AllServices.Downstream.Foreground.Cellular"/>
<int value="1977321258"
Expand Down Expand Up @@ -27152,8 +27156,11 @@ from previous Chrome versions.
label="Persistent sparse histogram accumulated a negative count that
caused a negative value."/>
<int value="7"
label="Persistent sparse histogram active sample overflowed during
accumulation and became negative."/>
label="Persistent sparse histogram active sample was negative after
accumulation (deprecated)."/>
<int value="8"
label="Persistent sparse histogram active sample wrapped 2^31 during
accumulation."/>
</enum>

<enum name="NetCacheState">
Expand Down

0 comments on commit d7b4dac

Please sign in to comment.