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

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Alex Gaynor [:Alex_Gaynor]]

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.

Comment 1

[Georg Fritzsche [:gfritzsche]]

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?

Comment 2

[Alex Thayer [:alexical] (she/her)]

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.

Comment 3

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1388748 - Don't resize histogram ranges on init

We were using std::vector::assign, which resizes the vector to
match the incoming data. This isn't what we want, as ranges_ has
already been sized to bucket_count_ + 1. Instead, just use a
memcpy.

Review commit: https://reviewboard.mozilla.org/r/166678/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166678/

Comment 4

[Georg Fritzsche [:gfritzsche]]

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.

Comment 5

[Alex Thayer [:alexical] (she/her)]

Comment on attachment 8895477 [details]
Bug 1388748 - Don't resize histogram ranges on init

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166678/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f636e51b75dd
Don't resize histogram ranges on init r=gfritzsche

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f636e51b75dd

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

Comment 9

[Alex Thayer [:alexical] (she/her)]

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.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

FYI I'm using an artifact build locally. Not sure if that is related to this mode only.

Comment 11

[Alex Thayer [:alexical] (she/her)]

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).

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sorry for the noise. As it looks like it's a problem for me building an artifact build locally. I filed bug 1390239.