Closed Bug 1388748 Opened 6 years ago Closed 6 years ago

Large volume of warnings from mozilla-central/ipc/chromium/src/base/histogram.cc

Categories

(Toolkit :: Telemetry, defect)

Product:
Toolkit
Component:
Telemetry
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: Alex_Gaynor, Assigned: dthayer)

References

Details

Attachments

(1 file)

Bug 1388748 - Don't resize histogram ranges on init
59 bytes, text/x-review-board-request
gfritzsche
: review+
Details 
I just noticed this today (though it's possible it was introduced a while ago). Starting up firefox and doing anything with it generates a ton of warnings like this:

[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 358
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 349
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 358
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 349
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 358
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 349
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 358
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 349
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 358
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 349
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 358
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 349
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 358
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 349
[Parent 96654] WARNING: file /Users/agaynor/projects/mozilla-central/ipc/chromium/src/base/histogram.cc, line 358

I suspect this was introduced by bug 1383210.
This is here and suggests the bucket calculations may be off now:
https://hg.mozilla.org/mozilla-central/annotate/b8e78ebe648e/ipc/chromium/src/base/histogram.cc#l349
https://hg.mozilla.org/mozilla-central/annotate/b8e78ebe648e/ipc/chromium/src/base/histogram.cc#l358

I can take a closer tomorrow. Doug, do you have an idea in the meantime?
Flags: needinfo?(dothayer)
Ack. It's due to my use of ranges_.assign(...), which resizes the vector to bucket_count() even though it's already had bucket_count() + 1 reserved. I'll just assert that it's been reserved and use a memcpy.
Assignee: nobody → dothayer
Status: NEW → ASSIGNED
Flags: needinfo?(dothayer)
Comment on attachment 8895477 [details]
Bug 1388748 - Don't resize histogram ranges on init

https://reviewboard.mozilla.org/r/166678/#review172252

Thanks, this looks good to me with the below fixed.

::: ipc/chromium/src/base/histogram.cc:277
(Diff revision 1)
>    DCHECK(ValidateBucketRanges());
>  }
>  
>  void Histogram::InitializeBucketRangeFromData(const int* buckets) {
> -  ranges_.assign(buckets, buckets + bucket_count());
> +  DCHECK_EQ(bucket_count_ + 1, ranges_.size());
> +  memcpy(ranges_.data(), buckets, bucket_count_);

Lets use `std::copy_n()` instead.
Attachment #8895477 - Flags: review?(gfritzsche) → review+
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f636e51b75dd
Don't resize histogram ranges on init r=gfritzsche
https://hg.mozilla.org/mozilla-central/rev/f636e51b75dd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
This has been marked as fixed bug I can still reproduce it with a central checkout from today (https://hg.mozilla.org/mozilla-central/rev/df9beb781895).

> 0:00.01 LOG: MainThread mozversion INFO platform_changeset: df9beb781895fcd0493c21e95ad313e0044515ec
> 0:00.01 LOG: MainThread mozversion INFO platform_version: 57.0a1
> 0:00.01 LOG: MainThread INFO Application command: /Volumes/data/code/gecko/obj/debug/dist/NightlyDebug.app/Contents/MacOS/firefox -no-remote -marionette -foreground -profile /var/folders/4k/sf4gz5fn3kl9hr3nd7pzbvhc0000gn/T/tmpqS7a1q.mozrunner
>[6831] WARNING: file /home/worker/workspace/build/src/ipc/chromium/src/base/histogram.cc, line 358
>[6831] WARNING: file /home/worker/workspace/build/src/ipc/chromium/src/base/histogram.cc, line 349
>[6831] WARNING: file /home/worker/workspace/build/src/ipc/chromium/src/base/histogram.cc, line 358
>[6831] WARNING: file /home/worker/workspace/build/src/ipc/chromium/src/base/histogram.cc, line 349
Flags: needinfo?(gfritzsche)
Flags: needinfo?(dothayer)
Seeing if I can reproduce. It's weird that the line numbers are the same, considering they should have been bumped up by one with this change.
Flags: needinfo?(dothayer)
FYI I'm using an artifact build locally. Not sure if that is related to this mode only.
Hmm. Not sure what you're seeing here. I can't reproduce on an artifact build of df9beb781895fcd0493c21e95ad313e0044515ec or when building locally. The warnings correctly go away for me with 3bb168fae615 (the patch for this bug).
Sorry for the noise. As it looks like it's a problem for me building an artifact build locally. I filed bug 1390239.
Flags: needinfo?(gfritzsche)
You need to log in before you can comment on or make changes to this bug.