Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2248693002: [system-health] Wait for 10 seconds after each story (except for memory benchmarks) (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 1 month ago by petrcermak
Modified:
1 year ago
Reviewers:
nednguyen, charliea, perezju
CC:
chromium-reviews, telemetry-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[system-health] Wait for 10 seconds after each story (except for memory benchmarks) Non-memory SH benchmarks: All SH stories will wait for 10 seconds after loading. Memory SH benchmarks: All SH stories will re-use action_runner.MeasureMemory(). BUG=589726, 637158 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 Committed: https://crrev.com/4cd6d6ceb622b79120215cae46df22f8abe68466 Cr-Commit-Position: refs/heads/master@{#413128}

Patch Set 1 #

Patch Set 2 : Don't modify memory benchmarks yet #

Total comments: 2

Patch Set 3 : Add comment & TODO #

Messages

Total messages: 25 (11 generated)
petrcermak
PTAL. Note that system health benchmarks will now effectively be waiting for 16 seconds after ...
1 year, 1 month ago (2016-08-15 13:37:42 UTC) #2
nednguyen
On 2016/08/15 13:37:42, petrcermak wrote: > PTAL. > > Note that system health benchmarks will ...
1 year, 1 month ago (2016-08-15 13:48:32 UTC) #3
petrcermak
On 2016/08/15 13:48:32, nednguyen (ooo til 8-13) wrote: > On 2016/08/15 13:37:42, petrcermak wrote: > ...
1 year, 1 month ago (2016-08-15 13:54:20 UTC) #4
nednguyen
On 2016/08/15 13:54:20, petrcermak wrote: > On 2016/08/15 13:48:32, nednguyen (ooo til 8-13) wrote: > ...
1 year, 1 month ago (2016-08-15 13:59:16 UTC) #5
petrcermak
On 2016/08/15 13:59:16, nednguyen (ooo til 8-13) wrote: > On 2016/08/15 13:54:20, petrcermak wrote: > ...
1 year, 1 month ago (2016-08-15 15:04:02 UTC) #6
petrcermak
PTAL. Thanks, Petr
1 year, 1 month ago (2016-08-16 18:09:37 UTC) #11
nednguyen
lgtm
1 year, 1 month ago (2016-08-16 18:17:35 UTC) #12
chromium-reviews
On Tue, 16 Aug 2016 at 11:24, Juan Antonio Navarro Pérez < perezju@chromium.org> wrote: > ...
1 year, 1 month ago (2016-08-16 19:55:30 UTC) #13
charliea
lgtm
1 year, 1 month ago (2016-08-16 20:26:42 UTC) #16
perezju
lgtm but I think that ShouldMeasureMemory is not needed, since the check of whether tracing ...
1 year, 1 month ago (2016-08-17 16:24:45 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2248693002/40001
1 year, 1 month ago (2016-08-19 10:47:42 UTC) #20
commit-bot: I haz the power
Committed patchset #3 (id:40001)
1 year, 1 month ago (2016-08-19 13:56:16 UTC) #22
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/4cd6d6ceb622b79120215cae46df22f8abe68466 Cr-Commit-Position: refs/heads/master@{#413128}
1 year, 1 month ago (2016-08-19 13:59:01 UTC) #24
petrcermak
1 year ago (2016-08-23 14:34:40 UTC) #25
Message was sent while issue was closed.
I've just realized that I haven't sent the reply to Juan's comment.

https://codereview.chromium.org/2248693002/diff/20001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/system_health_story.py (right):

https://codereview.chromium.org/2248693002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/system_health_story.py:61: return True
On 2016/08/17 16:24:45, perezju wrote:
> This check is already happening within MeasureMemory.

We actually need this for when we record the stories (they currently can't be
recorded through the storyset itself because it has extra constructor
parameters). I'll add a TODO to fix this.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b