New issue
Advanced search Search tips

Issue 826384 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Reduce noise of memory metric

Project Member Reported by u...@chromium.org, Mar 27 2018

Issue description

Investigate the source of noise in memory metric and reduce it.

This will make perf alerts more actionable.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 11 2018

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

commit 64d08119f342961a5dd64049a03a50d36585899c
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Wed Apr 11 11:57:03 2018

Reduce noise of memory metric.

The deterministic mode of ActionRunner.MeasureMemory forces garbage
collection to get stable results.

The ForceGarbageCollection method has two issues that make the results
noisy:
1) The method waits for 6 seconds after doing GC. During that time
   the web page can allocate more garbage.
2) A single GC might not be sufficient for V8 and Blink because some
   objects are retained across V8-Blink boundary and need
   V8=>Blink=>V8 round-trip.

In local testing, this patch reduces the noise of V8 memory metric
by order of magnitude on many of the pages (see goo.gl/22oKuo for
all the results).

Example (v8:heap:allocated_objects_size metric):
load:news:qq, with and without patch:
  avg 97.5 MiB   104.4 MiB
  max 97.6 MiB   107.4 MiB
  min 97.5 MiB   101.7 MiB
  std 24.8 KiB   2.8 MiB

load:tools:drive, with and without patch:
  avg 31.5 MiB   48.4 MiB
  max 32.7 MiB   57.1 MiB
  min 31.2 MiB   42.6 MiB
  std 689.0 KiB  7.0 MiB

Bug:  chromium:826384 
Change-Id: Ifd555dcd950d1cd1bbad22c0b79b7ee5d2fa97b2
Reviewed-on: https://chromium-review.googlesource.com/983775
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>

[modify] https://crrev.com/64d08119f342961a5dd64049a03a50d36585899c/telemetry/telemetry/internal/actions/action_runner.py

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 12 2018

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

commit 267d66ce578fbf8d490cf8eb6e088c8cd720c5ee
Author: catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Apr 12 05:59:38 2018

Roll src/third_party/catapult/ a22719853..cc64c0d81 (4 commits)

https://chromium.googlesource.com/catapult.git/+log/a22719853354..cc64c0d81aab

$ git log a22719853..cc64c0d81 --date=short --no-merges --format='%ad %ae %s'
2018-04-10 simonhatch Dashboard - Fix stats being generated in some cases
2018-04-11 eakuefner [TBMv2] Add HAD_FAILURES diagnostic and use it to avoid merging
2018-04-11 agrieve device_utils.PushChangedFiles: Fix caching logic when pushing files
2018-04-09 ulan Reduce noise of memory metric.

Created with:
  roll-dep src/third_party/catapult
BUG= chromium:831388 , chromium:826384 


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

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.


TBR=sullivan@chromium.org

Change-Id: I4216dad219b5d0fdaf0af49063866dae9fe9bba8
Reviewed-on: https://chromium-review.googlesource.com/1007325
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@{#550036}
[modify] https://crrev.com/267d66ce578fbf8d490cf8eb6e088c8cd720c5ee/DEPS

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 19 2018

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

commit 10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Thu Apr 19 16:13:04 2018

[heap] Do eager unmapping in CollectAllAvailableGarbage.

The memory metric samples memory usage immediately after forcing GC via
LowMemoryNotification. This makes the metric sensitive to the unmapper
tasks timing.

This patch forces eager unmapping in CollectAllAvailableGarbage.

It also forces eager unmapping of non-regular chunks at the beginning
of Mark-Compact to avoid accumulation of non-regular chunks.

Bug:  chromium:833291 ,  chromium:826384 
Change-Id: Iddf02cd4ab8613385d033899d29525fe6ee47fdd
Reviewed-on: https://chromium-review.googlesource.com/1017102
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52696}
[modify] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/src/heap/heap.cc
[modify] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/src/heap/mark-compact.cc
[modify] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/src/heap/spaces.cc
[modify] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/src/heap/spaces.h
[modify] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/src/isolate.cc
[modify] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/test/cctest/BUILD.gn
[modify] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/test/cctest/heap/test-page-promotion.cc
[modify] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/test/cctest/heap/test-spaces.cc
[add] https://crrev.com/10fce9c80a6557c7e2f60bf24b61118eaa8ce0b9/test/cctest/heap/test-unmapper.cc

Should this be marked as fixed or is there some work left?
Status: Fixed (was: Assigned)

Sign in to add a comment