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

Issue 852415 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Record V8 heap size UMA similar to Memory.Renderer.PrivateMemoryFootprint

Project Member Reported by u...@chromium.org, Jun 13 2018

Issue description

Currently V8 heap size UMA metrics recording happens in the renderer process on GC-related events. This makes the metrics sensitive to GC schedule and incomparable to Memory.Renderer.PrivateMemoryFootprint

We should record the V8 heap size exactly at the same time as Memory.Renderer.PrivateMemoryFootprint is recorded.




 

Comment 1 by u...@chromium.org, Jun 13 2018

Cc: erikc...@chromium.org
Erik, is Memory.Renderer.PrivateMemoryFootprint recorded in chrome/browser/metrics/process_memory_metrics_emitter.cc?

MEMORY_METRICS_HISTOGRAM_MB(
   std::string(UMA_PREFIX) + process_name + ".PrivateMemoryFootprint",
   pmd.os_dump().private_footprint_kb / 1024);

Would it be okay to add V8 specific metrics there?

> is Memory.Renderer.PrivateMemoryFootprint recorded in chrome/browser/metrics/process_memory_metrics_emitter.cc?

yes

> Would it be okay to add V8 specific metrics there?

yes. I believe we already have v8 effective size:
https://cs.chromium.org/chromium/src/chrome/browser/metrics/process_memory_metrics_emitter.cc?type=cs&q=process_memory_metrics_emitter.cc&sq=package:chromium&g=0&l=146

We could report more metrics - all the plumbing is already set up to pull results from v8 MDP into UMA/UKMs.

Comment 3 by u...@chromium.org, Jun 13 2018

Thank you! That helps a lot.

In addition to the effective size, it would be great to get the "allocated_objects_size" metric:
https://cs.chromium.org/chromium/src/gin/v8_isolate_memory_dump_provider.cc?rcl=4945dd5ba4eb9bf1baddb65d1b5d9b203e0c3417&l=160

That would show us if heap fragmentation is a problem.

Would it okay if I add that metric too?
Sure! sounds good
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 19 2018

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

commit 48ddede68e9281aaae740e44ccac7943ada47db1
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Tue Jun 19 10:49:22 2018

Add Memory.Experimental.Renderer2.V8.AllocatedObjects UMA and UKM metric

The metric tracks the size of all objects allocated in V8 and it is
similar to the existing Memory.Experimental.Renderer2.V8 metric.

The difference between the two metrics shows fragmentation of V8's heap.

The naming scheme is chosen similar to the existing BlinkGC and
BlinkGC.AllocatedObjects metrics.

Bug: 852415
Change-Id: Id1addec3bc0e09b3a16a16afbec0a634e5c26fe5
Reviewed-on: https://chromium-review.googlesource.com/1101020
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568397}
[modify] https://crrev.com/48ddede68e9281aaae740e44ccac7943ada47db1/chrome/browser/metrics/process_memory_metrics_emitter.cc
[modify] https://crrev.com/48ddede68e9281aaae740e44ccac7943ada47db1/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/48ddede68e9281aaae740e44ccac7943ada47db1/tools/metrics/ukm/ukm.xml

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 2

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

commit a3cb39f1a8f0e0c3ab1131bc0693a57feef7c03a
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Mon Jul 02 12:20:19 2018

Add Memory.Experimental.Renderer2.V8.AllocatedObjects to histograms.xml

This is a follow-up to https://chromium-review.googlesource.com/1101020

Bug: 852415
Change-Id: If80d21fcd23749f33d8ca95543e56c8b990ed231
Reviewed-on: https://chromium-review.googlesource.com/1112015
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571868}
[modify] https://crrev.com/a3cb39f1a8f0e0c3ab1131bc0693a57feef7c03a/tools/metrics/histograms/histograms.xml

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 13

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f9be5db02590a5715da682f2d409165d48468acd

commit f9be5db02590a5715da682f2d409165d48468acd
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Fri Jul 13 17:17:44 2018

Remove V8.MemoryHeapCommitted and V8.MemoryHeapUsed histograms.

They are replaced by
- Memory.Experimental.Renderer2.V8
- Memory.Experimental.Renderer2.V8.AllocatedObjects

Bug: chromium:852415
Change-Id: I64285e5067304319acadc0d64a05aa553d8ae6e0
Reviewed-on: https://chromium-review.googlesource.com/1101197
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54444}
[modify] https://crrev.com/f9be5db02590a5715da682f2d409165d48468acd/src/counters.cc
[modify] https://crrev.com/f9be5db02590a5715da682f2d409165d48468acd/src/counters.h
[modify] https://crrev.com/f9be5db02590a5715da682f2d409165d48468acd/src/heap/gc-tracer.cc
[modify] https://crrev.com/f9be5db02590a5715da682f2d409165d48468acd/src/heap/heap.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 17

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

commit 9ffaf123d87e39934a64b82faaeaee17fbeebb93
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Tue Jul 17 16:46:26 2018

Deprecate V8.MemoryHeapCommitted and V8.MemoryHeapUsed histograms.

They are replaced by
- Memory.Experimental.Renderer2.V8
- Memory.Experimental.Renderer2.V8.AllocatedObjects

The corresponding CL in V8 that removes recording:
https://chromium-review.googlesource.com/c/v8/v8/+/1101197

Bug: 852415
Change-Id: I1357ced8536386d7083666edc66862661e073e84
Reviewed-on: https://chromium-review.googlesource.com/1101033
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575676}
[modify] https://crrev.com/9ffaf123d87e39934a64b82faaeaee17fbeebb93/tools/metrics/histograms/histograms.xml

After discussions in [1], we concluded that the best approach to ensure static path to V8 heap space dumps would be to group V8 memory dumps by isolate types (main, workers, utility).

The current dump structure:
- V8
   - isolate_0x1234
    - heap_spaces
      - code_space
      ...
   - isolate_0x5678
      ...
   - isolate_0x9012

will change to:
- V8
  - isolate
    - main
      - heap
        - code_space
        ...
    - workers
      - heap
        - code_space
	  - isolate_0x5678
	  - isolate_0x9012


[1] https://docs.google.com/document/d/1Xrzy2IRUN4YQgQhEKQd7uYBnRj0VVi2TZPLM7vefqx0/edit
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 23

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

commit 51945ab84f62bf856b46809f4d3669eb3af4549a
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Thu Aug 23 14:12:58 2018

Make IsolateHolder aware of the isolate type.

This introduces four isolate types (main, worker, utility, test) and
stores the isolate type in IsolateHolder.

V8's memory dump provider will group memory dumps by the isolate type,
which is necessary for UMA/UKM reporting of V8's memory metrics.

Bug: chromium:852415
Change-Id: Id9a31404e6bc62562545518aaa4e35cae00a97d6
Reviewed-on: https://chromium-review.googlesource.com/1183194
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585480}
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/extensions/renderer/bindings/api_binding_test.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/gin/isolate_holder.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/gin/public/isolate_holder.h
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/gin/shell/gin_main.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/gin/shell_runner_unittest.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/gin/test/v8_test.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/net/proxy_resolution/proxy_resolver_v8.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/pdf/pdfium/pdfium_engine.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/third_party/blink/renderer/core/fetch/data_consumer_handle_test_util.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/third_party/blink/renderer/platform/bindings/v8_per_isolate_data.cc
[modify] https://crrev.com/51945ab84f62bf856b46809f4d3669eb3af4549a/third_party/blink/tools/audit_non_blink_usage.py

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 28

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/18259e73a8b71a6026d7fdc60936ca68edda3f9a

commit 18259e73a8b71a6026d7fdc60936ca68edda3f9a
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Tue Aug 28 14:19:04 2018

Update memory metric for the upcoming V8 memory dump provider changes.

This makes backwards compatible change to the memory metric,
so that it can handle the existing V8 memory dumps and the upcoming
V8 memory dumps.

The V8 memory dump provider changes are described in
https://chromium-review.googlesource.com/c/chromium/src/+/1181263

Bug: chromium:852415
Change-Id: I4e39758e1f916c6b36ee9c62896d31b5e989a4a6
Reviewed-on: https://chromium-review.googlesource.com/1181569
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>

[modify] https://crrev.com/18259e73a8b71a6026d7fdc60936ca68edda3f9a/tracing/tracing/metrics/system_health/memory_metric_test.html
[modify] https://crrev.com/18259e73a8b71a6026d7fdc60936ca68edda3f9a/tracing/tracing/metrics/system_health/memory_metric.html

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 28

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

commit bd386b7f89e5a47220133ac0449bf1a7eb2da027
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Aug 28 16:40:18 2018

Roll src/third_party/catapult 44e5893116c6..18259e73a8b7 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/44e5893116c6..18259e73a8b7


git log 44e5893116c6..18259e73a8b7 --date=short --no-merges --format='%ad %ae %s'
2018-08-28 ulan@chromium.org Update memory metric for the upcoming V8 memory dump provider changes.


Created with:
  gclient setdep -r src/third_party/catapult@18259e73a8b7

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:852415
TBR=sullivan@chromium.org

Change-Id: I4004ea4b2a408e6e67e8aea8596320cc814f2da2
Reviewed-on: https://chromium-review.googlesource.com/1193587
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#586741}
[modify] https://crrev.com/bd386b7f89e5a47220133ac0449bf1a7eb2da027/DEPS

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 3

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

commit 1d31633ba55632244e5df4522583de947491af14
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Mon Sep 03 14:19:08 2018

Restructure V8 isolate memory dumps

The motivation for this change is to make the paths to heap space
nodes static so that they can be plumbed into UMA/UKM reporting.

This groups the nodes by the isolate type and turns the dynamic
"isolate_0x*" into leaf nodes in the tree.

V8 memory dump before:
- V8
  - isolate_0x1234
    - heap_spaces
      - code_space
      ...
  - isolate_0x5678
    ...
  - isolate_0x9012

V8 memory dump after:
- V8
  - main
    - heap
      - code_space
      ...
  - workers
    - heap
      - code_space
        - isolate_0x5678
        - isolate_0x9012
        ...

In this example, "isolate_0x1234" is the main isolate, so it is removed.
The other isolate nodes are for workers, so they are made into leaf nodes.

Besides that there are other minor changes:
- "heap_spaces" is renamed to "heap".
- the V8 code stats are reported under "code_stats" node.

Bug: chromium:852415
Change-Id: I6e4c6e6fddb6e8cf9be3fe6d5fca837ac6fbdc34
Reviewed-on: https://chromium-review.googlesource.com/1181263
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588387}
[modify] https://crrev.com/1d31633ba55632244e5df4522583de947491af14/base/trace_event/memory_infra_background_whitelist.cc
[modify] https://crrev.com/1d31633ba55632244e5df4522583de947491af14/gin/public/isolate_holder.h
[modify] https://crrev.com/1d31633ba55632244e5df4522583de947491af14/gin/v8_isolate_memory_dump_provider.cc
[modify] https://crrev.com/1d31633ba55632244e5df4522583de947491af14/gin/v8_isolate_memory_dump_provider_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 25

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

commit abd1f2ca5a6d15155a6ae2996dfaff49308d65d0
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Tue Sep 25 05:43:17 2018

Add detailed UMA and UKM memory metrics for V8.

Currently we have two V8 memory metrics:
- Memory.Experimental.Renderer2.V8,
- Memory.Experimental.Renderer2.V8.AllocatedObjects

This patch adds memory metrics of
- the main isolate of V8,
- the heap spaces in the main isolate of V8,
- the malloced memory in the main isolate of V8,
- worker isolates of V8.

Note that the heap spaces and the malloced memory of
worker isolates are omitted.

Privacy review doc: https://goo.gl/X5eYHh

List of all new metrics:
- Memory.Experimental.Renderer2.V8.Main,
- Memory.Experimental.Renderer2.V8.Main.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Heap,
- Memory.Experimental.Renderer2.V8.Main.Heap.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Heap.CodeSpace,
- Memory.Experimental.Renderer2.V8.Main.Heap.CodeSpace.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Heap.LargeObjectSpace,
- Memory.Experimental.Renderer2.V8.Main.Heap.LargeObjectSpace.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Heap.MapSpace,
- Memory.Experimental.Renderer2.V8.Main.Heap.Map.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Heap.NewLargeObjectSpace,
- Memory.Experimental.Renderer2.V8.Main.Heap.NewLargeObjectSpace.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Heap.NewSpace,
- Memory.Experimental.Renderer2.V8.Main.Heap.NewSpace.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Heap.OldSpace,
- Memory.Experimental.Renderer2.V8.Main.Heap.OldSpace.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Heap.ReadOnlySpace,
- Memory.Experimental.Renderer2.V8.Main.Heap.ReadOnlySpace.AllocatedObjects,
- Memory.Experimental.Renderer2.V8.Main.Malloc
- Memory.Experimental.Renderer2.V8.Workers,
- Memory.Experimental.Renderer2.V8.Workers.AllocatedObjects,

Bug: 852415
Change-Id: I4727c1159617208908925f79de82bc755edea696
Reviewed-on: https://chromium-review.googlesource.com/1213169
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593830}
[modify] https://crrev.com/abd1f2ca5a6d15155a6ae2996dfaff49308d65d0/chrome/browser/metrics/process_memory_metrics_emitter.cc
[modify] https://crrev.com/abd1f2ca5a6d15155a6ae2996dfaff49308d65d0/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/abd1f2ca5a6d15155a6ae2996dfaff49308d65d0/tools/metrics/ukm/ukm.xml

Sign in to add a comment