Switch benchmark.ShouldTearDownStateAfterEachStoryRun to return True by default |
|||||||||
Issue descriptionIn the past, we reuse the browser instance between multiple stories run in a same benchmark to reduce the cycle time on the bots. This practice, however, poses a bunch of problems: 1) Since the same browser instance is reused across runs, performance bugs like memory leaks can make the performance metrics of later pages very unstable. 2) When we bisect the regression on a single page, we can't use the --story-filter flag to reduce the cycle time of each run because the pages are not truly independent. At present time, we have a lot more bots hardware and are better at sharding the workload. We also think it's not worth trading off saving resources for the complexity & stability of performance test, hence we decide to switch ShouldTearDownStateAfterEachStoryRun to return True by default. This may expect to affect performance metrics of almost all the telemetry benchmarks.
,
Aug 25 2016
+1 even though this might require adding new long-running benchmarks/stories (so that we don't lose their implicit coverage we currently have)
,
Aug 25 2016
Sounds good to me. And we should probably do this soon after the next branch point in a few more days (to avoid complication with health plan measurements). We already have memory.top_10_mobile_stress explicitly *not* tearing down between pages/pagesets. We should make sure that it stays that way and, as Petr mentions, consider adding some more.
,
Aug 25 2016
+1 for doing this, and +1 for soon after the branch point.
,
Aug 26 2016
,
Aug 31 2016
Juan: can you ping me after release point so I can switch the flag?
,
Aug 31 2016
We should be able to do the switch when issue 641497 is fixed and we have some new numbers flowing into the internal bots.
,
Sep 1 2016
,
Sep 2 2016
,
Sep 2 2016
,
Sep 2 2016
This is now (nearly) unblocked, preparing a CL for the switch.
,
Sep 2 2016
,
Sep 2 2016
Thanks for taking this, Juan!
,
Sep 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ef104f29d113767b0667c5bd5d1b989286b76a90 commit ef104f29d113767b0667c5bd5d1b989286b76a90 Author: perezju <perezju@chromium.org> Date: Fri Sep 02 23:20:23 2016 [tools/perf] Explicitly disable state tear down on some benchmarks We're planning to switch the default of ShouldTearDownStateAfterEachStoryRun to True; but some benchmarks would not work under that setting. BUG= 640990 Review-Url: https://codereview.chromium.org/2301383002 Cr-Commit-Position: refs/heads/master@{#416383} [modify] https://crrev.com/ef104f29d113767b0667c5bd5d1b989286b76a90/tools/perf/benchmarks/memory_infra.py
,
Sep 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b6ec1c01c7dac3ee76f9245e4ee442f1c669c55 commit 6b6ec1c01c7dac3ee76f9245e4ee442f1c669c55 Author: catapult-deps-roller <catapult-deps-roller@chromium.org> Date: Sat Sep 03 11:52:21 2016 Roll src/third_party/catapult/ 54acca7c4..4f812c903 (1 commit). https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/54acca7c4f74..4f812c9035e6 $ git log 54acca7c4..4f812c903 --date=short --no-merges --format='%ad %ae %s' 2016-09-03 perezju [Telemetry] Switch ShouldTearDownStateAfterEachStoryRun to True as default BUG= 640990 TBR=catapult-sheriff@chromium.org Review-Url: https://codereview.chromium.org/2307273002 Cr-Commit-Position: refs/heads/master@{#416453} [modify] https://crrev.com/6b6ec1c01c7dac3ee76f9245e4ee442f1c669c55/DEPS
,
Sep 3 2016
,
Sep 7 2016
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by nedngu...@google.com
, Aug 25 2016