New issue
Advanced search Search tips

Issue 780477 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Telemetry histograms grow and use a lot of memory

Project Member Reported by perezju@chromium.org, Nov 1 2017

Issue description

From issue 778647:

The PageTestResults._histograms object grows by about 14 MiB on each story run (on a memory benchmark).

For a long running benchmark, some quick maths shows 14 MiB x 20 Stories x 60 Repeats ≈ 17 GiB; which makes the host machine run out of memory.

Figure out a way prevent the memory footprint from growing without bounds. Maybe the histograms can be serialized to disk as they are being produced?
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 6 2017

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

commit 85e17d274ae42150f1d071822393d6680c08de32
Author: benshayden <benjhayden@chromium.org>
Date: Mon Nov 06 22:31:22 2017

Copy-on-write HistogramBins in python.

Histogram currently always creates fresh HistogramBins. This wastes
memory for empty bins.
See https://chromium-review.googlesource.com/c/catapult/+/754453

This CL copies HistogramBins on write in order to share empty
HistogramBins between many Histograms.
This reduces HistogramSet heap usage by 78%:
http://www/~benjhayden/754119.html#r=TOT&o=avg.%25Δavg.count.%25Δcount.%25Δsum.sum&l=%25Δsum&a=&c=2&d=
Those results were produced by heap_metric:
https://chromium-review.googlesource.com/c/catapult/+/754453

No functional changes, so no test changes.

Bug:  chromium:780477 
Change-Id: I269cb5bd2be6621eeb9e8cb2903501014678b01e
Reviewed-on: https://chromium-review.googlesource.com/754119
Commit-Queue: Ben Hayden <benjhayden@chromium.org>
Reviewed-by: Ethan Kuefner <eakuefner@chromium.org>
Reviewed-by: Simon Hatch <simonhatch@chromium.org>

[modify] https://crrev.com/85e17d274ae42150f1d071822393d6680c08de32/tracing/tracing/value/histogram.html
[modify] https://crrev.com/85e17d274ae42150f1d071822393d6680c08de32/tracing/tracing/value/histogram.py

Owner: perezju@chromium.org
Status: Fixed (was: Available)
perezju: Please try your big benchmark now?
78% memory saving, nice!
Status: Assigned (was: Fixed)
Thanks! I'll reopen and keep assigned to myself to re-enable histograms in that benchmark.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 8 2017

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

commit 29c782aba9e1608e206de80ce684db02e36bfca0
Author: benshayden <benjhayden@chromium.org>
Date: Wed Nov 08 19:53:00 2017

Save memory with __slots__ in python Histogram library.

Python's __slots__ feature saves several bytes of memory per object
instance by omitting the __dict__ property.
https://stackoverflow.com/questions/472000/usage-of-slots

After making HistogramBins copy-on-write, this saves 29% more
memory, or an additional 6% of the original 7.4MiB:
http://www/~benjhayden/754634.html#r=TOT&o=avg.%25Δavg.count.sum.%25Δsum&l=%25Δsum&a=&g=name&c=3&d=

SlotsMetaclass is added to ensure that new Diagnostic subclasses
remember to specify __slots__ in order to prevent regressing
memory consumption.

Memory Usage of PageTestResults in Telemetry investigation
(sorry, Google only):
https://docs.google.com/document/d/1_Xugc-yR-hFm5Sx9hpbpnrXdtOcJj5oBbFr7atdt_fg

No functional changes.

Bug:  chromium:780477 
Change-Id: Icdc2d0daad6af801a6952928420a8f66bc4f90c6
Reviewed-on: https://chromium-review.googlesource.com/754634
Commit-Queue: Ben Hayden <benjhayden@chromium.org>
Reviewed-by: Ethan Kuefner <eakuefner@chromium.org>
Reviewed-by: Simon Hatch <simonhatch@chromium.org>

[add] https://crrev.com/29c782aba9e1608e206de80ce684db02e36bfca0/common/py_utils/py_utils/slots_metaclass_unittest.py
[modify] https://crrev.com/29c782aba9e1608e206de80ce684db02e36bfca0/tracing/tracing/value/diagnostics/diagnostic.py
[modify] https://crrev.com/29c782aba9e1608e206de80ce684db02e36bfca0/tracing/tracing/value/histogram.py
[add] https://crrev.com/29c782aba9e1608e206de80ce684db02e36bfca0/common/py_utils/py_utils/slots_metaclass.py

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 8 2017

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

commit 6fd72afa72ded77daa42a877fc2e425aae7418fd
Author: Juan A. Navarro Perez <perezju@chromium.org>
Date: Wed Nov 08 22:20:00 2017

[Telemetry] Log Telemetry's memory usage after collecting all results

This will help quantify the efforts in reducing the memory used by the
results object (mostly histograms).

Bug:  chromium:780477 
Change-Id: I59498c0f7e52d6528e301c19e8f6db2fc5d8fe73
Reviewed-on: https://chromium-review.googlesource.com/759000
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/6fd72afa72ded77daa42a877fc2e425aae7418fd/telemetry/telemetry/internal/story_runner.py

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/275b8d22c885986a00a18001394e074ac6a04686

commit 275b8d22c885986a00a18001394e074ac6a04686
Author: Juan Antonio Navarro Pérez <perezju@google.com>
Date: Thu Nov 09 02:48:06 2017

Status: Verified (was: Assigned)
Confirmed on the bot, memory usage went down from ~ 15.6 GiB (the total memory available on the machine) down to 3.3 GiB. Which is pretty spot on w.r.t. the expected 78% savings predicted in #5.

Awesome!

(INFO) 2017-11-12 20:53:44,661 memory_debug.LogHostMemoryUsage:60  Current process:
(INFO) 2017-11-12 20:53:44,661 memory_debug._LogProcessInfo:38  - 3.3 GiB (pid=14525) python
https://logs.chromium.org/v/?s=chrome%2Fbb%2Finternal.client.clank%2Fintegration-android-low-end-phone%2F4134%2F%2B%2Frecipes%2Fsteps%2Fmemory.long_running_dual_browser_test%2F0%2Fstdout
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 22

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

commit ab50c9185a1b8235ad3d4c967f3de19835adead0
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Sat Dec 22 19:23:26 2018

Roll src/third_party/catapult d7d78924cced..7c1d51b169ed (1 commits)

https://chromium.googlesource.com/catapult.git/+log/d7d78924cced..7c1d51b169ed


git log d7d78924cced..7c1d51b169ed --date=short --no-merges --format='%ad %ae %s'
2018-12-22 benjhayden@chromium.org Histogram-based heap profiler


Created with:
  gclient setdep -r src/third_party/catapult@7c1d51b169ed

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:780477 
TBR=sullivan@chromium.org

Change-Id: I27bc982e06fdbc4d0234601c6f74bbe9673205ad
Reviewed-on: https://chromium-review.googlesource.com/c/1390139
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#618760}
[modify] https://crrev.com/ab50c9185a1b8235ad3d4c967f3de19835adead0/DEPS

Comment 10 by benhenry@google.com, Jan 16 (6 days ago)

Components: Test>Telemetry

Comment 11 by benhenry@google.com, Jan 16 (6 days ago)

Components: -Speed>Telemetry

Sign in to add a comment