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

Issue 763335 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

16.7% regression in memory.desktop at 500110:500207

Project Member Reported by rmcilroy@chromium.org, Sep 8 2017

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=763335

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=8c8eb7f27d009dceb5004103dbb41d7da34f2f5e696f2f4429c05b715bca3e4a


Bot(s) for this bug's original alert(s):

chromium-rel-mac12
Cc: tapted@chromium.org
Owner: tapted@chromium.org
Status: Assigned (was: Untriaged)

=== Auto-CCing suspected CL author tapted@chromium.org ===

Hi tapted@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Trent Apted
  Commit : 47862bc479eeb61ef945d3c23d1ba34c11873bfc
  Date   : Thu Sep 07 02:22:40 2017
  Subject: Use GPU memory buffer resources and zero copy for MacViews.

Bisect Details
  Configuration: mac_10_12_perf_bisect
  Benchmark    : memory.desktop
  Metric       : memory:chrome:all_processes:reported_by_chrome:gpu:effective_size_avg/TrivialFullscreenVideoPageSharedPageState
  Change       : 16.52% | 12473166.2222 -> 14533477.3333

Revision             Result                   N
chromium@500109      12473166 +- 53900.8      6      good
chromium@500158      12463325 +- 0.0          6      good
chromium@500183      12463325 +- 0.0          6      good
chromium@500186      12473166 +- 53900.8      6      good
chromium@500188      12463325 +- 0.0          6      good
chromium@500189      14533477 +- 0.0          6      bad       <--
chromium@500195      14533477 +- 0.0          6      bad
chromium@500207      14533477 +- 0.0          6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=TrivialFullscreenVideoPageSharedPageState memory.desktop

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8969026116715701248


For feedback, file a bug with component Speed>Bisection

Comment 4 by tapted@chromium.org, Sep 11 2017

Status: Started (was: Assigned)
I'll investigate!

Initial observations 
 - it looks like this test hasn't been around very long
 - it's gone up and down quite a lot in the last 2 months
 - I think this is just looking at the GPU process? That CL switches to using GPU resources so there might be a corresponding decrease in some non-GPU memory. (Or perhaps this is just the tradeoff involved to get the performance benefit).

Screen Shot 2017-09-11 at 10.18.05 am.png
67.4 KB View Download
Cc: primiano@chromium.org erikc...@chromium.org benjhayden@chromium.org perezju@chromium.org hjd@chromium.org
+some memory and dashboard folks: it would be really great if there was an easy way for tapted@ to know which metric to look at next to see if there was a decrease in some non-GPU memory (see #4)

Comment 6 by tapted@chromium.org, Sep 11 2017

Cc: ccameron@chromium.org
+ccameron might have pointers where to look, or know whether this is the expected effect of GpuMemoryBuffers/ZeroCopy. I might be able to dig out a stack to the memory allocation that's causing the blame. But at a high-level, my understanding of this flag is that it enables DMA in the GPU process for a memory region on the GPU board. That's going to grow the virtual address space, but not actually allocate any extra system memory (resident set size?).

I'm starting a collection - https://chromeperf.appspot.com/report?sid=474c25800003be1e46e565ff3f77e11b04acd211441544ea8047ad3cf7a283ef&start_rev=499539&end_rev=500815

- There's a mirrored-in-y-axis plot in memory:chrome:all_processes:reported_by_chrome:gpumemorybuffer:effective_size_avg but the magnitude is kinda tiny.
- memory:chrome:gpu_process:reported_by_chrome:allocated_objects_size_avg has a downward trend.
- none of the reported_by_os measurements seem to be affected, but they're pretty noisy.


There might be some other ways to have impact on this. E.g. we could free up the fullscreen bubble UI after it fades out rather than merely hide it - I've been meaning to do that for a while, but since there can only ever be 1 fullscreen bubble it's been low priority.

Comment 7 by tapted@chromium.org, Sep 11 2017

Note https://chromium.googlesource.com/chromium/src/+/lkcr/docs/memory-infra/probe-cc.md#Other-Areas-of-Interest says,

> Many of the allocations under CC are aliased with memory in the Browser or GPU process. When investigating CC memory it may be worth looking at the following external categories:
> 1. GPU Process / GPU Category - All GPU resources allocated by CC have a counterpart in the GPU/GPU category. This includes GL Textures, buffers, and other GPU backed objects such as Native GPU Memory Buffers.
> 2. Browser Process / GpuMemoryBuffer Category - Resources backed by Shared Memory GpuMemoryBuffers are allocated by the browser and will be tracked in this category.


It certainly seems that this DMA region is going to show up in the memory metrics. So, I'm pretty sure that CL is WAI, and this Issue is not reflective of a real change in memory usage.

Comment 8 by hjd@google.com, Sep 11 2017

Ben's work to surface related histograms on the dashboard will really help here! Hopefully when memory 'moves' between metrics most of the time those metrics will be related and we'll immediately see the changes in the sparkline.

The analysis in #7 seems accurate, the change is in the browser process[1] and appears as a new 2MB entry under:
gpu/mapped_memory/manager_2/chunk_8 which is backed by shared memory.
The change seems to have been slightly mitigated by a decrease in unspecified malloc in the browser process of about 500mb.

Does that sound right?

(Re #4, you probably discovered a lot of this already but: all_processes:reported_by_chrome:gpu:effective_size_avg shows the memory blamed on the 'gpu' component summed across all processes.

We can look at the breakdown by process by looking at:
  browser_process:reported_by_chrome:gpu:effective_size_avg (+2MB)
  renderer_processes:reported_by_chrome:gpu:effective_size_avg (~0)
  gpu_process:reported_by_chrome:gpu:effective_size_avg (~0)

And we can look the the summation across all components:
  all_processes:reported_by_chrome:effective_size_avg (noisy: slight decrease?)

And the summation of all components broken down by process:
  browser_process:reported_by_chrome:effective_size_avg (+1.5MB)
  renderer_processes:reported_by_chrome:effective_size_avg (noisy: slight decrease?)
  gpu_process:reported_by_chrome:effective_size_avg (noisy: slight decrease?))

Often when the issue is an more of an accounting problem the total metric (browser_process:reported_by_chrome:effective_size_avg) won't move and the one of the other component metrics (browser_process:reported_by_chrome:X:effective_size_avg for other values of X) will decrease (to make up for the increase). In this case that is sort of happening since browser_process:reported_by_chrome:malloc:effective_size_avg went down not enough to match the increase.)

[1]: See https://chromeperf.appspot.com/report?sid=b26b3836c53877c270d218f3d94ef448b743a63612880d20144c16048ef76580&start_rev=473536&end_rev=500831
Before trace: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_5-2017-09-06_16-35-09-3350.html
After trace: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_5-2017-09-06_11-52-13-94819.html




before.png
40.7 KB View Download
after.png
45.6 KB View Download
change.png
72.1 KB View Download

Comment 9 by tapted@chromium.org, Sep 12 2017

Status: WontFix (was: Started)
Thanks! That's super helpful - I wasn't sure how to drill down into the component details after seeing them mentioned in docs/memory-infra/cc-probe.md . But that makes it clear that the extra ~2MB is just the GPU memory getting mapped for DMA (before, the memory would have been behind an opengl texture handle). And since there is DMA, there's less copying between buffers, so fewer mallocs.

I toyed a bit with some traces around TileManager::AssignGpuMemoryToTiles() [1] to see why it's such a round number. Looks as though the window gets broken up into 256x256 tiles and does some crazy memory recycling so that the buffers can get reused, hence there's some padding. Things only get evicted when limits are reached - (i.e. FreeTileResourcesUntilUsageIsWithinLimit(..)).

This all seems WAI, so closing out.

[1] https://cs.chromium.org/chromium/src/cc/tiles/tile_manager.cc?l=643
That makes sense to me too -- using IOSurfaces where we used to use GL textures will likely result in virtual memory reservations to map the IOSurfaces for CPU access.

Sign in to add a comment