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

Issue 756549 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Regression in browser histogram memory use

Project Member Reported by asvitk...@chromium.org, Aug 17 2017

Issue description

Regression in browser histogram memory use.

Just noticed this now, when looking at histogram memory use data for histograms.

Specifically, this graph, which looks at Android channel-split data for percent histogram use:
https://uma.googleplex.com/timeline_v2/?sid=82ff6ccb4f3dfbb13a59d451ca4e3d20

We also see the effect on Windows, though the graphs a bit more noisy:
https://uma.googleplex.com/timeline_v2/?sid=3a9e8caacc31fa9faa0822fb3ba60591

Specifically, I am talking about the regression that took place on Canary on 60.0.3100.0. Naively, it looks like it's a revert of the change that happened on 60.0.3087.3, however we know that change is the single sample optimization and it *did not* get reverted after landing for that version: https://codereview.chromium.org/2811713003

The only base/metrics/ change in the range for the regression is:
[histogram] Make histograms more resistant to overflows.
https://codereview.chromium.org/2867303004 

So we suspect that's the cause. The Windows graphs narrows CL range to the following and this change is still in range:
https://chromium.googlesource.com/chromium/src/+log/60.0.3097.0..60.0.3098.0?pretty=fuller&n=10000

The graphs also show that it only regresses things on the browser side, and does not affect the renderer-side memory use (i.e. the single-sample improvement sticks).

Assigning to altimin@, the author of the change. Ideally, we could investigate and try to understand why the change causes extra memory use and then see if we can find a way to mitigate it.

 
Cc: -bcwh...@chromium.org altimin@chromium.org
Owner: bcwh...@chromium.org
The change to unlogged/logged over total/logged violated an assumption used as the basis of an optimization of the SingleSample, notably that it was increment only.

The unlogged/logged method ends up doing "subtract" operations on the sample vectors.  These negative increments cause the SingleSample to "fail" and expand to the full counts vector.

I'll take a look at it and see if can't be changed back to allowing decrement of single samples.

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 25 2017

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

commit 96a00c6b0f29258607dccbf35c98f5c95acfbe98
Author: Brian White <bcwhite@chromium.org>
Date: Fri Aug 25 17:58:37 2017

Support decrement of single sample.

When originally written, stored samples generally didn't get
decremented but a change from storing total/logged to unlogged/logged
means a subtraction within stored samples.  The single-sample was
optimized assuming an increment-only model and so doing the
subtraction would cause the realization of the full sample set
even when only one bucket was being utilized.

Also, add dedicated unit-test for AtomicSingleSample.

Bug:  756549 
Change-Id: I0a96e83dd2c1c288c3890ac9b05f3763dc6be3d4
Reviewed-on: https://chromium-review.googlesource.com/633893
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Alexei Svitkine (very slow) <asvitkine@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497462}
[modify] https://crrev.com/96a00c6b0f29258607dccbf35c98f5c95acfbe98/base/BUILD.gn
[modify] https://crrev.com/96a00c6b0f29258607dccbf35c98f5c95acfbe98/base/metrics/histogram_samples.cc
[modify] https://crrev.com/96a00c6b0f29258607dccbf35c98f5c95acfbe98/base/metrics/histogram_samples.h
[add] https://crrev.com/96a00c6b0f29258607dccbf35c98f5c95acfbe98/base/metrics/histogram_samples_unittest.cc

Status: Verified (was: Assigned)
Graphs show marked decrease in memory use.

Sign in to add a comment