Issue metadata
Sign in to add a comment
|
Audit memory.top_10_mobile interactions with the browser |
||||||||||||||||||||
Issue descriptionIn the context of issue 748566 , where we're upgrading the mechanism by which benchmarks may control whether the browser should be closed between stories or not, I found something which may or may not be intended behavior of memory.top_10_mobile. What it currently actually does: A. Open browser B. For each of ten URLs: 1. Navigate to URL in browser 2. Measure memory (*foreground*) 3. Navigate to about:blank in browser 4. Push browser to background 5. Measure memory (*background*) 6. Bring browser back to foreground C. Close browser Is that step 3 actually intended? Also thinking about the possibilities that the new mechanism to control the browser will bring; we could now open and close the browser within that loop, i.e. replace A and B with steps 0 and 7. That should make measurements more stable for the benchmark since only each foreground/background pair would depend on each other, but otherwise such pairs of stories would be independent of each other. This also opens up the possibility of faster and more reliable bisects for this benchmark (with a carefully curated --story-filter). I know we plan to deprecate memory.top_10_mobile in the near(-ish?) future, but it does seem like they should provide some short term benefits until the benchmark is indeed deprecated. Of course these changes would affect our current measurements, so need to be carefully orchestrated around not to disrupt the system health releasing process too much.
,
Sep 5 2017
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59bf3021bcf91a57dd3e3972c9cfcb0c8e83b117 commit 59bf3021bcf91a57dd3e3972c9cfcb0c8e83b117 Author: Juan A. Navarro Perez <perezju@chromium.org> Date: Tue Sep 12 14:58:15 2017 [tools/perf] Tweak memory.top_10_mobile story flow This change modifies the benchmark's story flow in two ways: - about:blank will no longer be loaded before pushing the browser to the background. Thus making the background measurement more realistic - the browser will now be restarted between each foreground/background pair of stories; this will make each pair stories independent of each other, helping to make results more reproducible and less noisy. Perf sheriffs warning: This is expected to affect memory measurements for this benchmark. Bug: 750055 Change-Id: I65ea9431d52f63ee3e7dfccad68a62476018ea99 Reviewed-on: https://chromium-review.googlesource.com/612980 Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org> Reviewed-by: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#501274} [modify] https://crrev.com/59bf3021bcf91a57dd3e3972c9cfcb0c8e83b117/tools/perf/page_sets/memory_top_10_mobile.py
,
Sep 13 2017
Change landed, benchmark seems to be working correctly as expected. We run 100 tests and the browser is closed 50 times (i.e. after each background story) [e.g. 1]. Most metrics seem to have gone down, and java heap a bit up: https://chrome-health.googleplex.com/health-plan/android-chrome/memory/nexus5/ All seems reasonable, but will check when those jumps start triggering alerts. Next steps here are to tweak story filters for bisects so they can make use of this new advantage! [1]: https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Finternal.client.clank%2Fhealth-plan-webview-phone%2F5620%2F%2B%2Frecipes%2Fsteps%2Fmemory.top_10_mobile%2F0%2Fstdout
,
Sep 19 2017
The NextAction date has arrived: 2017-09-19
,
Sep 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/30eb733502be6c433f1b04e9b864b51bcf160cd3 commit 30eb733502be6c433f1b04e9b864b51bcf160cd3 Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Tue Sep 19 18:42:13 2017 Roll src/third_party/catapult/ 08d6322ff..8380d62bc (4 commits) https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/08d6322ff7a5..8380d62bc249 $ git log 08d6322ff..8380d62bc --date=short --no-merges --format='%ad %ae %s' 2017-09-19 perezju [dashboard] Allow story filters for memory.top_10_mobile 2017-09-19 estevenson [Dashboard] Add M60-M62 milestones to speed releasing dashboard. 2017-09-19 jbudorick [devil] Update chromium build target deps. 2017-09-19 nednguyen [Clean up] Remove code related to install test ca in Telemetry Created with: roll-dep src/third_party/catapult BUG= 750055 ,765703,763167, 766278 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: I77b8b70416ab5323615b7a8054ea5cfbfe32b3c9 Reviewed-on: https://chromium-review.googlesource.com/673344 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#502900} [modify] https://crrev.com/30eb733502be6c433f1b04e9b864b51bcf160cd3/DEPS
,
Sep 20 2017
Work completed here. Benchmark looking fine.
,
Sep 25 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/clank/internal/apps/+/da7cdd03ea3b1327a3f8d78351fe2293b43c125d commit da7cdd03ea3b1327a3f8d78351fe2293b43c125d Author: Juan A. Navarro Perez <perezju@google.com> Date: Mon Sep 25 15:50:41 2017 |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by primiano@chromium.org
, Jul 28 2017