New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 893937 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 890733



Sign in to add a comment

Some histogram samples not being reported via autotestPrivate.getHistogram

Project Member Reported by derat@chromium.org, Oct 10

Issue description

hiroh@ has been working on adding a new Chrome OS integration test at https://crrev.com/c/1249401 that plays a video and watches for changes to the Media.GpuVideoDecoderInitializeStatus enum histogram to determine if hardware decode was used successfully.

He's seeing an initial bucket/count of [15,16):1 for the histogram in the test, but not an expected second sample with value 0.

The test uses the code at https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/HEAD/src/chromiumos/tast/local/chrome/metrics/histogram.go to read histograms, which calls the autotestPrivate.getHistogram function that I added in https://crrev.com/c/1200840 (just for this purpose). The relevant Chrome API code at https://cs.chromium.org/chromium/src/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.cc?q=autotest_private_api&sq=package:chromium&g=0&l=593 looks like this:

  const base::HistogramBase* histogram =
      base::StatisticsRecorder::FindHistogram(params->name);
  ...
  std::unique_ptr<base::HistogramSamples> samples =
      histogram->SnapshotSamples();
  ...
  for (std::unique_ptr<base::SampleCountIterator> it = samples->Iterator();
       !it->Done(); it->Next()) {
    base::HistogramBase::Sample min = 0;
    int64_t max = 0;
    base::HistogramBase::Count count = 0;
    it->Get(&min, &max, &count);

By adding logging to the test, I can see that it's in fact only receiving the [15,16) bucket from autotestPrivate.getHistogram. When I request the same histogram via chrome://histograms/Media.GpuVideoDecoderInitializeStatus immediately after the test times out, I can see the [0,1) bucket with a single sample:

Histogram: Media.GpuVideoDecoderInitializeStatus recorded 2 samples, mean = 7.5 (flags = 0x41)
0   ------------------------------------------------------------------------O (1 = 50.0%)
1   ...
15  ------------------------------------------------------------------------O (1 = 50.0%) {50.0%}
16  ...

I've tried adding code in Chrome to report an identical histogram through calls like this:

  UMA_HISTOGRAM_ENUMERATION("Tast.SomeHistogram", 15, 23);
  UMA_HISTOGRAM_ENUMERATION("Tast.SomeHistogram", 0, 23);

When querying this new histogram using autotestPrivate.getHistogram in a test, I see both the initial [15,16):1 bucket and the new [0,1):1 bucket.

So I'm extremely confused about why autotestPrivate.getHistogram isn't reporting the second sample for Media.GpuVideoDecoderInitializeStatus. The only thing I can come up with is that maybe it's not being seen because it's apparently reported from a renderer (although I'm not sure why the first sample, which is reported from the same place, is seen, then).

Mark, do you see anything obviously wrong with the C++ code above that would explain this? Thanks!
 
FWIW, using the StatisticsRecorder APIs directly definitely seems somewhat error-prone. Could you instead use a combination of the APIs provided by histogram_functions.h and histogram_tester.h?
#1: The functions in histogram_functions.h are just for recording histograms, right? autotestPrivate.getHistogram wants to read histograms. The test is running outside of Chrome (and interacting with it through the Chrome DevTools Protocol), so I'm not sure that base/test/metrics/histogram_tester.h is applicable here either.

Intriguingly, I just updated this test code to forgo using autotestPrivate.getHistogram entirely, instead using using Browser.getHistogram method from CDP: https://chromedevtools.github.io/devtools-protocol/tot/Browser#method-getHistogram

With that, I'm *still* not seeing the second "0" sample get reported. There seems to be something more going on here...
hiroh@ just mentioned, and I confirmed, that after fetching chrome://histograms/Media.GpuVideoDecoderInitializeStatus, the [0,1):1 bucket is immediately reflected by both autotestPrivate.getHistogram and CDP's Browser.GetHistogram method.
Ah. Is this histogram being recorded from a process other than the browser process? If so, you'll need to manually synchronize the histograms prior to trying to read them. I think that HistogramSynchronizer is a good entry point for doing so, though there might be an even more appropriate API available.
Status: Started (was: Assigned)
#4: Thanks! Yeah, the histogram is apparently being recorded from renderers, so that seems like the cause.

I'll look into making getHistogram call content::FetchHistogramsAsynchronously. It should probably just do it at most once a second (or something similar) instead of every time.
I've uploaded https://crrev.com/c/1274892 to make autotestPrivate.getHistogram incorporate data recorded by other processes.
Cc: -mpear...@chromium.org
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d21c42593b6f372d43e65856beb558153e96f09

commit 3d21c42593b6f372d43e65856beb558153e96f09
Author: Daniel Erat <derat@chromium.org>
Date: Fri Oct 12 00:31:21 2018

chromeos: Fix autotestPrivate.getHistograms for other procs.

Make the chrome.autotestPrivate API's getHistograms function
call content::FetchHistogramsAsynchronously() and
base::StatisticsRecorder::ImportProvidedHistograms() to
fetch and incorporate histogram data from other processes,
similar to what chrome://histograms does.

Without this, the function (which is called over the Chrome
DevTools Protocol by Chrome OS integration tests) serves
stale data. For example,
Media.GpuVideoDecoderInitializeStatus samples are missing
since that histogram is recorded from renderer processes.

Bug:  893937 
Change-Id: Id6415f1a32b41ad7ddeb251216f09b2314eb5980
Reviewed-on: https://chromium-review.googlesource.com/c/1274892
Commit-Queue: Dan Erat <derat@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599035}
[modify] https://crrev.com/3d21c42593b6f372d43e65856beb558153e96f09/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.cc
[modify] https://crrev.com/3d21c42593b6f372d43e65856beb558153e96f09/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.h

Status: Fixed (was: Started)

Sign in to add a comment