Implementing cache_temperature logic for v8.top25 |
||||||
Issue descriptionWe would like to enable different cache temperature runs for V8's top25 benchmark to see the impact of V8 code caching on load time. I've tried using the current cache_temparature.py support in telemetery however there are a couple of issues which mean I can't use it as-is. Currently the cache_temperature loads the page multiple times in the same renderer to get to the required hotness - i.e., for HOT it will load the page three times in the same renderer. For V8 we would like to test code-caching from the disk cache (not in-memory caches we have in the renderer) so what I would like is for each run to kill the browser, but retain the browser profile (i.e., disk-cache) so that the next run can load cached code from the disk-cache, but is a new renderer process that doesn't share any memory with the previous run. Currently there doesn't seem to be a way to do this. After talking with Juan my intention is to create a subclass of shared_page_state for v8_top_25 which controls the browser startup sequence to ensure we can retain the same profile for WARM/HOT runs, however it would be nice if we could unify this with the current cache_temperature approach. Kouhei / Pasko - I think you both have tests which rely on cache_temperature - do they need to reuse the same renderer process?
,
Nov 20 2017
> Is there a worry that subsequent loads of optimized stuff from disk cache can modify the disk cache as well? (Like: more execution profile is collected, more code optimized, more flushed). No I'm not worried about this - in fact I want to depend on this (for code-caching, the JS-file will be added to the disk-cache on the COLD run, then in the WARM run we will serialize the code and add that to the disk cache, so on the HOT run I want the updates from the WARM run to be there so that it deserializes the code directly from the disk-cache instead of compiling. > If it is a concern, then clearing and re-pushing disk cache before each run would help reduce the measurement 'pseudo-noise'. Doing this without crear+repush would require re-creating the disk cache between benchmarking runs, and this is several times slower (3x?). I explicitly want to use exactly the same disk-cache between the three runs as mentioned above. I'm not sure on the best way to do this - whether to pull and repush the disk-cache, or just add logic that instructs telemetry not to try and repush a disk-cache on the WARM / HOT runs.
,
Nov 20 2017
> No I'm not worried about this - in fact I want to depend on this (for > code-caching, the JS-file will be added to the disk-cache on the COLD run, > then in the WARM run we will serialize the code and add that to the disk > cache, so on the HOT run I want the updates from the WARM run to be there so > that it deserializes the code directly from the disk-cache instead of > compiling. OK. I thought you may later decide that for some optimizations you need 5 more runs to make them useful to serialize to disk cache. I guess there are no such plans in near future :) To rephrase my benchmarking speed concern: it seems that you want to make as many HOT runs as there are COLD and WARM ones, while useful data is contained only in HOT runs. This (N * (COLD + WARM + HOT)) is ~3 times slower than running COLD + WARM + N * (REPUSH + HOT) for large N and cheap REPUSH.
,
Nov 20 2017
#0: Hmhh, I think we shouldn't to create a new shared_state to get v8 disk cache working. By default, Telemetry already always restart the browser in between runs (in https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/page/shared_page_state.py?rcl=37921f135dc61021b8ced4c73375339658ac9953&l=152). So in SharedState.WillRunStory (https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/page/shared_page_state.py?rcl=37921f135dc61021b8ced4c73375339658ac9953&l=217), you can add s.t like: # ... disk_cache_temperature.EnsureDiskCacheTemperature(...) # Logic to start the browser go here # Logic to ensure in memory cache go here. cache_temperature.EnsurePageCacheTemperature(..) In general, I think we should aim for making the telemetry story runs indepedent from each other & straightforward to understand. The benchmark runtime concern can be addressed by sharding & throwing more hardwares into the lab later. Hardware is cheap, and complex code is expensive :-)
,
Nov 20 2017
re #3: We are actually equally interested in COLD, WARM and HOT. We also only indent to run each story once in each phase, so I don't think we need the complexity of REPUSH. re #4: Hmm that's an interesting approach - it does mean we need to load the page 1 + 2 + 3 = 5 times instead of 3 times, but that seems to be similar to how the cache_temperature logic currently works and avoids us having to play games with the browser's profile. I'll give this a go.
,
Nov 20 2017
re #5: I obviously assumed HOT are more representative and that HOT may be multiple loads away from WARM. Good to know where we are, thanks.
,
Nov 20 2017
I tried the disk_cache_temperature approach in #4 but it wasn't as simple as I thought, since by default newly started browsers don't share their profile so we would need to manage that. I came up with what I think is a simpler approach though - all I care about is killing the renderer not the browser, so I can create a new cache mode (HOT_BROWSER) which does the same as the current HOT temperature (which I propose renaming HOT_RENDERER) but kills and restarts the tab once it's warmed the cache. Any objections? If not, I'll get something ready tomorrow.
,
Nov 20 2017
#7: the new simpler approach SGTM. Kill & restart the tabs is always faster than restarting browser, so this is a win in both runtime & simplicity :-)
,
Nov 20 2017
,
Nov 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/2ffb57b10257d6577907105f83cbb4ae4063397c commit 2ffb57b10257d6577907105f83cbb4ae4063397c Author: Ross McIlroy <rmcilroy@chromium.org> Date: Wed Nov 22 14:22:39 2017 Refactor cache_temperature to pass around tab instead of browser. BUG= chromium:786988 Change-Id: If37d0dcf7353b866aee688fa3629a14b71411c0f Reviewed-on: https://chromium-review.googlesource.com/785352 Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> [modify] https://crrev.com/2ffb57b10257d6577907105f83cbb4ae4063397c/telemetry/telemetry/page/cache_temperature.py
,
Nov 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/854541376b3c890683e3a85e32dd998c8ed15ed3 commit 854541376b3c890683e3a85e32dd998c8ed15ed3 Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Wed Nov 22 17:27:34 2017 Roll src/third_party/catapult/ d0e5c9c00..2ffb57b10 (1 commit) https://chromium.googlesource.com/catapult.git/+log/d0e5c9c00f4e..2ffb57b10257 $ git log d0e5c9c00..2ffb57b10 --date=short --no-merges --format='%ad %ae %s' 2017-11-22 rmcilroy Refactor cache_temperature to pass around tab instead of browser. Created with: roll-dep src/third_party/catapult BUG= 786988 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. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: I8fee57d85e764c519accaf31ced1d618bf6d32c2 Reviewed-on: https://chromium-review.googlesource.com/785790 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#518670} [modify] https://crrev.com/854541376b3c890683e3a85e32dd998c8ed15ed3/DEPS
,
Nov 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/ae685e7e94fbae79334c4f1c2f296af70e041810 commit ae685e7e94fbae79334c4f1c2f296af70e041810 Author: Ross McIlroy <rmcilroy@chromium.org> Date: Thu Nov 23 17:30:08 2017 Add support for warming up the browser's cache but running in a fresh renderer. Adds WARM_BROWSER and HOT_BROWSER cache temperatures to the cache_temperature module. These new temperatures pre-warm the browser cache in a different tab to the one on which the test will be run, avoiding warming the renderer's cache. BUG= chromium:786988 Change-Id: Ia61cca26e97993535140769641dac5351fc3b813 Reviewed-on: https://chromium-review.googlesource.com/782179 Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> Reviewed-by: Ned Nguyen <nednguyen@google.com> [modify] https://crrev.com/ae685e7e94fbae79334c4f1c2f296af70e041810/telemetry/telemetry/page/cache_temperature.py [modify] https://crrev.com/ae685e7e94fbae79334c4f1c2f296af70e041810/telemetry/telemetry/page/cache_temperature_unittest.py
,
Nov 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/9147b5e57e4c8cce448dcac68a3dbd576d507fb8 commit 9147b5e57e4c8cce448dcac68a3dbd576d507fb8 Author: Juan Antonio Navarro Pérez <perezju@chromium.org> Date: Fri Nov 24 10:06:33 2017 Revert "Add support for warming up the browser's cache but running in a fresh renderer." This reverts commit ae685e7e94fbae79334c4f1c2f296af70e041810. Reason for revert: This is failing on the catapult auto-roller, e.g.: https://chromium-review.googlesource.com/c/chromium/src/+/788750 Original change's description: > Add support for warming up the browser's cache but running in a fresh renderer. > > Adds WARM_BROWSER and HOT_BROWSER cache temperatures to the cache_temperature > module. These new temperatures pre-warm the browser cache in a different > tab to the one on which the test will be run, avoiding warming the renderer's > cache. > > BUG= chromium:786988 > > Change-Id: Ia61cca26e97993535140769641dac5351fc3b813 > Reviewed-on: https://chromium-review.googlesource.com/782179 > Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> > Reviewed-by: Ned Nguyen <nednguyen@google.com> TBR=rmcilroy@chromium.org,nednguyen@google.com Change-Id: I256cbf46b3d907209a6f75b9cfd23b82d4be295d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:786988 Reviewed-on: https://chromium-review.googlesource.com/788810 Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org> Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org> [modify] https://crrev.com/9147b5e57e4c8cce448dcac68a3dbd576d507fb8/telemetry/telemetry/page/cache_temperature.py [modify] https://crrev.com/9147b5e57e4c8cce448dcac68a3dbd576d507fb8/telemetry/telemetry/page/cache_temperature_unittest.py
,
Nov 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/030b0600bb0cbf31dfc30e5c1e6ffecf0b6124bc commit 030b0600bb0cbf31dfc30e5c1e6ffecf0b6124bc Author: Ross McIlroy <rmcilroy@chromium.org> Date: Fri Nov 24 11:22:44 2017 Reland "Add support for warming up the browser's cache but running in a fresh renderer." This is a reland of ae685e7e94fbae79334c4f1c2f296af70e041810 Original change's description: > Add support for warming up the browser's cache but running in a fresh renderer. > > Adds WARM_BROWSER and HOT_BROWSER cache temperatures to the cache_temperature > module. These new temperatures pre-warm the browser cache in a different > tab to the one on which the test will be run, avoiding warming the renderer's > cache. > > BUG= chromium:786988 > > Change-Id: Ia61cca26e97993535140769641dac5351fc3b813 > Reviewed-on: https://chromium-review.googlesource.com/782179 > Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> > Reviewed-by: Ned Nguyen <nednguyen@google.com> Bug: chromium:786988 Change-Id: Iab074273a15d91841322c1f7ae59e68e134844c5 Reviewed-on: https://chromium-review.googlesource.com/788811 Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org> Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> [modify] https://crrev.com/030b0600bb0cbf31dfc30e5c1e6ffecf0b6124bc/telemetry/telemetry/page/cache_temperature.py [modify] https://crrev.com/030b0600bb0cbf31dfc30e5c1e6ffecf0b6124bc/telemetry/telemetry/page/cache_temperature_unittest.py
,
Nov 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a8229307727c55fdafcd527f28c783204c72136 commit 2a8229307727c55fdafcd527f28c783204c72136 Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Fri Nov 24 12:58:53 2017 Roll src/third_party/catapult/ 86c17b98f..9147b5e57 (2 commits) https://chromium.googlesource.com/catapult.git/+log/86c17b98f536..9147b5e57e4c $ git log 86c17b98f..9147b5e57 --date=short --no-merges --format='%ad %ae %s' 2017-11-24 perezju Revert "Add support for warming up the browser's cache but running in a fresh renderer." 2017-11-23 rmcilroy Add support for warming up the browser's cache but running in a fresh renderer. Created with: roll-dep src/third_party/catapult BUG= 786988 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. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: I34cc9cb1ed2584359c0532ac34796a1b96dc1b57 Reviewed-on: https://chromium-review.googlesource.com/788830 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#519094} [modify] https://crrev.com/2a8229307727c55fdafcd527f28c783204c72136/DEPS
,
Nov 28 2017
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/632ad259224e11d5bdfa8bd9a1f08f6dc50747ab commit 632ad259224e11d5bdfa8bd9a1f08f6dc50747ab Author: Ross McIlroy <rmcilroy@chromium.org> Date: Wed Nov 29 17:41:26 2017 [Metrics] Avoid collecting sub-group events in RuntimeStatsMetric. Only collect and upload the total group metrics for RuntimeStatsMetric to avoid uploading too many metrics when we enable multiple cache temperature runs on the v8.top_25 benchmark. BUG= chromium:786988 Change-Id: I5db7379bc4afbea39feeddca3a81597c4a7baec5 Reviewed-on: https://chromium-review.googlesource.com/793154 Reviewed-by: Ethan Kuefner <eakuefner@chromium.org> Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> [modify] https://crrev.com/632ad259224e11d5bdfa8bd9a1f08f6dc50747ab/tracing/tracing/metrics/v8/runtime_stats_metric_test.html [modify] https://crrev.com/632ad259224e11d5bdfa8bd9a1f08f6dc50747ab/tracing/tracing/metrics/v8/runtime_stats_metric.html
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6a27113240f1aba02da4c36a2f06ac8d285c401 commit d6a27113240f1aba02da4c36a2f06ac8d285c401 Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Wed Nov 29 19:36:23 2017 Roll src/third_party/catapult/ c5bfbbc86..81d3bd921 (2 commits) https://chromium.googlesource.com/catapult.git/+log/c5bfbbc86903..81d3bd9213f4 $ git log c5bfbbc86..81d3bd921 --date=short --no-merges --format='%ad %ae %s' 2017-11-28 benjhayden Fix "View Options" dropdown in trace viewer. 2017-11-28 rmcilroy [Metrics] Avoid collecting sub-group events in RuntimeStatsMetric. Created with: roll-dep src/third_party/catapult BUG= 786988 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. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: Iec704d0f260deb823313dd427b4d2bfb14c034b7 Reviewed-on: https://chromium-review.googlesource.com/797472 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#520199} [modify] https://crrev.com/d6a27113240f1aba02da4c36a2f06ac8d285c401/DEPS
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee3fd090c8b2cf711b0cd23bf70d0c0be58d3d15 commit ee3fd090c8b2cf711b0cd23bf70d0c0be58d3d15 Author: Ross McIlroy <rmcilroy@chromium.org> Date: Wed Nov 29 21:42:31 2017 [Perf] Enable WARM_BROWSER and HOT_BROWSER runs on v8.top-25 BUG= chromium:786988 Change-Id: I05ab0973681d66e90be9a743eb01c4c7b2f588af Reviewed-on: https://chromium-review.googlesource.com/782323 Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> Reviewed-by: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#520255} [modify] https://crrev.com/ee3fd090c8b2cf711b0cd23bf70d0c0be58d3d15/tools/perf/benchmarks/v8.py [modify] https://crrev.com/ee3fd090c8b2cf711b0cd23bf70d0c0be58d3d15/tools/perf/page_sets/v8_top_25.py
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd56612d995a6986fa67f2527e94fc3734bc9d48 commit cd56612d995a6986fa67f2527e94fc3734bc9d48 Author: Ross McIlroy <rmcilroy@chromium.org> Date: Thu Nov 30 16:16:58 2017 [Perf] Add short wait during v8.top_25 warmup runs. Fixes an issue on the wikipedia edit page where the warmup run would cause an alert error which would cause the story to time-out. BUG= chromium:786988 Change-Id: I9ab9c41b21bb6f44229b895b6e18074eec1afcab Reviewed-on: https://chromium-review.googlesource.com/800690 Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> Cr-Commit-Position: refs/heads/master@{#520560} [modify] https://crrev.com/cd56612d995a6986fa67f2527e94fc3734bc9d48/tools/perf/page_sets/v8_top_25.py
,
Aug 28
I think this is fixed?
,
Sep 25
Sorry missed this. Done.
,
Jan 16
(6 days ago)
,
Jan 16
(6 days ago)
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by pasko@chromium.org
, Nov 20 2017