|
|
Description[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
Committed: https://crrev.com/ef104f29d113767b0667c5bd5d1b989286b76a90
Cr-Commit-Position: refs/heads/master@{#416383}
Patch Set 1 #
Total comments: 3
Patch Set 2 : petr's comment #Messages
Total messages: 25 (13 generated)
Description was changed from ========== [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 ========== to ========== [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 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ==========
perezju@chromium.org changed reviewers: + nednguyen@google.com, petrcermak@chromium.org
https://codereview.chromium.org/2301383002/diff/1/tools/perf/benchmarks/memor... File tools/perf/benchmarks/memory_infra.py (right): https://codereview.chromium.org/2301383002/diff/1/tools/perf/benchmarks/memor... tools/perf/benchmarks/memory_infra.py:66: return False I've just realized we cannot tear down the state on this benchmark since it also measures background memory usage, and a dead browser won't be of use for that. :(
The CQ bit was checked by perezju@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM with one comment. Please wait for Ned's approval (I'm owner only for SH stuff). Thanks, Petr https://codereview.chromium.org/2301383002/diff/1/tools/perf/benchmarks/memor... File tools/perf/benchmarks/memory_infra.py (right): https://codereview.chromium.org/2301383002/diff/1/tools/perf/benchmarks/memor... tools/perf/benchmarks/memory_infra.py:95: return False You don't need this here because the class inherits from MemoryBenchmarkTop10Mobile.
https://codereview.chromium.org/2301383002/diff/1/tools/perf/benchmarks/memor... File tools/perf/benchmarks/memory_infra.py (right): https://codereview.chromium.org/2301383002/diff/1/tools/perf/benchmarks/memor... tools/perf/benchmarks/memory_infra.py:95: return False On 2016/09/02 09:40:55, petrcermak wrote: > You don't need this here because the class inherits from > MemoryBenchmarkTop10Mobile. Good catch.
lgtm
The CQ bit was checked by perezju@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from petrcermak@chromium.org Link to the patchset: https://codereview.chromium.org/2301383002/#ps20001 (title: "petr's comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_s5_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL) linux_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL) mac_retina_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL) winx64_10_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by perezju@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)
Description was changed from ========== [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 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ========== to ========== [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 ==========
On 2016/09/02 17:33:40, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > linux_perf_cq on master.tryserver.chromium.perf (JOB_TIMED_OUT, no build URL) Removed CQ_EXTRA_TRYBOT
The CQ bit was checked by nednguyen@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [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 ========== to ========== [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 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [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 ========== to ========== [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 Committed: https://crrev.com/ef104f29d113767b0667c5bd5d1b989286b76a90 Cr-Commit-Position: refs/heads/master@{#416383} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ef104f29d113767b0667c5bd5d1b989286b76a90 Cr-Commit-Position: refs/heads/master@{#416383} |