Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1056623002:
Telemetry: Add simple startup benchmark that uses a large profile. (Closed)
DescriptionTelemetry: Add simple startup benchmark that uses a large profile.
This CL adds two benchmarks: "startup.large_profile.cold.blank_page" and
"startup.large_profile.warm.blank_page". It also adds pre-generated large
profiles for windows, mac, and linux.
BUG=472711
Committed: https://crrev.com/bc7b42b8c88eb24f92b62010d37d8c53d9b242aa
Cr-Commit-Position: refs/heads/master@{#323777}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : Check that the possible browser is local before downloading profiles. #
Total comments: 2
Patch Set 4 : Lower page_set repeat to 1. #
Messages
Total messages: 30 (8 generated)
erikchen@chromium.org changed reviewers: + nednguyen@google.com
nednguyen: Please review. Once Telemetry is automatically generating large profiles, we'll need to tweak run_benchmark to have smarter logic for choosing a large profile.
Can you run win, mac & linux trybot on these 2 newly added benchmarks?
The CQ bit was checked by erikchen@chromium.org to run a CQ dry run
The CQ bit was unchecked by erikchen@chromium.org
On 2015/04/01 21:30:39, nednguyen wrote: > Can you run win, mac & linux trybot on these 2 newly added benchmarks? There's a nifty "CQ dry run" button I've never seen before. I'll try that.
The CQ bit was checked by erikchen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1056623002/20001
On 2015/04/01 21:34:43, erikchen wrote: > On 2015/04/01 21:30:39, nednguyen wrote: > > Can you run win, mac & linux trybot on these 2 newly added benchmarks? > > There's a nifty "CQ dry run" button I've never seen before. I'll try that. I think Ned meant to try the benchmark on the perf trybots? http://www.chromium.org/developers/performance-try-bots We're not running full benchmarks on the CQ yet, although we have some changes in the works to make it easier. Note that you may get failures on the "running without patch step" which is expected because it's trying to run a benchmark that doesn't exist without your patch. "Running with patch" should pass.
On 2015/04/01 21:36:45, I haz the power (commit-bot) wrote: > Dry run: CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/1056623002/20001 Ah sorry, I mean to run the bisect bot on the new benchmarks.
The CQ bit was unchecked by erikchen@chromium.org
On 2015/04/01 21:48:57, nednguyen wrote: > On 2015/04/01 21:36:45, I haz the power (commit-bot) wrote: > > Dry run: CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/patch-status/1056623002/20001 > > Ah sorry, I mean to run the bisect bot on the new benchmarks. <-- Please ignore this. What Annie says is correct.
On 2015/04/01 21:57:30, nednguyen wrote: > On 2015/04/01 21:48:57, nednguyen wrote: > > On 2015/04/01 21:36:45, I haz the power (commit-bot) wrote: > > > Dry run: CQ is trying da patch. Follow status at > > > https://chromium-cq-status.appspot.com/patch-status/1056623002/20001 > > > > Ah sorry, I mean to run the bisect bot on the new benchmarks. > <-- Please ignore this. What Annie says is correct. Running with patch fails on android: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_p... I thought the benchmark would be disabled on android - is this because ./run_benchmarks --browser=trybot-all ignores the @Enabled decorators, or is it because 2 @Enabled decorators doesn't have the effect I expect it to have?
sullivan@chromium.org changed reviewers: + sullivan@chromium.org
+prasadv Prasad, any idea why this ran on the Android perf trybots?
On 2015/04/03 01:30:16, sullivan wrote: > +prasadv > > Prasad, any idea why this ran on the Android perf trybots? I wonder whether that's due to this: https://code.google.com/p/chromium/issues/detail?id=468427
On 2015/04/03 02:12:43, nednguyen wrote: > On 2015/04/03 01:30:16, sullivan wrote: > > +prasadv > > > > Prasad, any idea why this ran on the Android perf trybots? > > I wonder whether that's due to this: > https://code.google.com/p/chromium/issues/detail?id=468427 Yeah, I think it's related. This lgtm to land.
On 2015/04/03 02:15:23, sullivan wrote: > On 2015/04/03 02:12:43, nednguyen wrote: > > On 2015/04/03 01:30:16, sullivan wrote: > > > +prasadv > > > > > > Prasad, any idea why this ran on the Android perf trybots? > > > > I wonder whether that's due to this: > > https://code.google.com/p/chromium/issues/detail?id=468427 > > Yeah, I think it's related. This lgtm to land. The tryjobs succeeded on Mac and Linux: http://build.chromium.org/p/tryserver.chromium.perf/builders/linux_perf_bisec... http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_perf_bisect/... But the tryjobs always time out on Windows (5+ attempts): http://build.chromium.org/p/tryserver.chromium.perf/builders/win_x64_perf_bis... http://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect/... The benchmark runs fine on two different local windows machines, and has never had any problems. It does take a very long time to run on Windows. I'm guessing we're hitting the 1-hour timeout? Not really sure since there are no logs at all.
On 2015/04/03 04:31:27, erikchen wrote: > On 2015/04/03 02:15:23, sullivan wrote: > > On 2015/04/03 02:12:43, nednguyen wrote: > > > On 2015/04/03 01:30:16, sullivan wrote: > > > > +prasadv > > > > > > > > Prasad, any idea why this ran on the Android perf trybots? > > > > > > I wonder whether that's due to this: > > > https://code.google.com/p/chromium/issues/detail?id=468427 > > > > Yeah, I think it's related. This lgtm to land. > > The tryjobs succeeded on Mac and Linux: > http://build.chromium.org/p/tryserver.chromium.perf/builders/linux_perf_bisec... > http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_perf_bisect/... > > But the tryjobs always time out on Windows (5+ attempts): > http://build.chromium.org/p/tryserver.chromium.perf/builders/win_x64_perf_bis... > http://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect/... > > The benchmark runs fine on two different local windows machines, and has never > had any problems. It does take a very long time to run on Windows. I'm guessing > we're hitting the 1-hour timeout? Not really sure since there are no logs at > all. I think we should disable it on windows for now before landing to make sure.
https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... File tools/perf/benchmarks/startup.py (right): https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... tools/perf/benchmarks/startup.py:13: options = {'pageset_repeat': 5} Can we lower the pageset_repeat in your LargeProfile benchmark and see if it helps the cycle time? Of course we'll get more noise, but I'm not sure what else we can do here.
https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... File tools/perf/benchmarks/startup.py (right): https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... tools/perf/benchmarks/startup.py:13: options = {'pageset_repeat': 5} On 2015/04/03 13:59:43, sullivan wrote: > Can we lower the pageset_repeat in your LargeProfile benchmark and see if it > helps the cycle time? Of course we'll get more noise, but I'm not sure what else > we can do here. I lowered the pageset_repeat to 1 for the large profiles, which causes the perf try-bots to pass.
On 2015/04/03 17:05:50, erikchen wrote: > https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... > File tools/perf/benchmarks/startup.py (right): > > https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... > tools/perf/benchmarks/startup.py:13: options = {'pageset_repeat': 5} > On 2015/04/03 13:59:43, sullivan wrote: > > Can we lower the pageset_repeat in your LargeProfile benchmark and see if it > > helps the cycle time? Of course we'll get more noise, but I'm not sure what > else > > we can do here. > > I lowered the pageset_repeat to 1 for the large profiles, which causes the perf > try-bots to pass. *on windows that is. They already passed on Mac/Linux. http://build.chromium.org/p/tryserver.chromium.perf/builders/win_x64_perf_bis...
On 2015/04/03 17:06:06, erikchen wrote: > On 2015/04/03 17:05:50, erikchen wrote: > > > https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... > > File tools/perf/benchmarks/startup.py (right): > > > > > https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... > > tools/perf/benchmarks/startup.py:13: options = {'pageset_repeat': 5} > > On 2015/04/03 13:59:43, sullivan wrote: > > > Can we lower the pageset_repeat in your LargeProfile benchmark and see if it > > > helps the cycle time? Of course we'll get more noise, but I'm not sure what > > else > > > we can do here. > > > > I lowered the pageset_repeat to 1 for the large profiles, which causes the > perf > > try-bots to pass. > > *on windows that is. They already passed on Mac/Linux. > http://build.chromium.org/p/tryserver.chromium.perf/builders/win_x64_perf_bis... Let's land this new benchmark with pageset_repeat = 1. We can bump the repeat count over time as we get a better sense for how long the test is taking, and how much noise we're seeing.
On 2015/04/03 17:07:56, erikchen wrote: > On 2015/04/03 17:06:06, erikchen wrote: > > On 2015/04/03 17:05:50, erikchen wrote: > > > > > > https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... > > > File tools/perf/benchmarks/startup.py (right): > > > > > > > > > https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... > > > tools/perf/benchmarks/startup.py:13: options = {'pageset_repeat': 5} > > > On 2015/04/03 13:59:43, sullivan wrote: > > > > Can we lower the pageset_repeat in your LargeProfile benchmark and see if > it > > > > helps the cycle time? Of course we'll get more noise, but I'm not sure > what > > > else > > > > we can do here. > > > > > > I lowered the pageset_repeat to 1 for the large profiles, which causes the > > perf > > > try-bots to pass. > > > > *on windows that is. They already passed on Mac/Linux. > > > http://build.chromium.org/p/tryserver.chromium.perf/builders/win_x64_perf_bis... > > Let's land this new benchmark with pageset_repeat = 1. We can bump the repeat > count over time as we get a better sense for how long the test is taking, and > how much noise we're seeing. If I understand correctly, reducing the repeat to 1 gets us under 2 minutes, so we have a lot of room to adjust here. I'm fine with landing this and bumping the repeat count after.
On 2015/04/03 17:09:25, sullivan wrote: > On 2015/04/03 17:07:56, erikchen wrote: > > On 2015/04/03 17:06:06, erikchen wrote: > > > On 2015/04/03 17:05:50, erikchen wrote: > > > > > > > > > > https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... > > > > File tools/perf/benchmarks/startup.py (right): > > > > > > > > > > > > > > https://codereview.chromium.org/1056623002/diff/40001/tools/perf/benchmarks/s... > > > > tools/perf/benchmarks/startup.py:13: options = {'pageset_repeat': 5} > > > > On 2015/04/03 13:59:43, sullivan wrote: > > > > > Can we lower the pageset_repeat in your LargeProfile benchmark and see > if > > it > > > > > helps the cycle time? Of course we'll get more noise, but I'm not sure > > what > > > > else > > > > > we can do here. > > > > > > > > I lowered the pageset_repeat to 1 for the large profiles, which causes the > > > perf > > > > try-bots to pass. > > > > > > *on windows that is. They already passed on Mac/Linux. > > > > > > http://build.chromium.org/p/tryserver.chromium.perf/builders/win_x64_perf_bis... > > > > Let's land this new benchmark with pageset_repeat = 1. We can bump the repeat > > count over time as we get a better sense for how long the test is taking, and > > how much noise we're seeing. > > If I understand correctly, reducing the repeat to 1 gets us under 2 minutes, so > we have a lot of room to adjust here. I'm fine with landing this and bumping the > repeat count after. lgtm
The CQ bit was checked by erikchen@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sullivan@chromium.org Link to the patchset: https://codereview.chromium.org/1056623002/#ps60001 (title: "Lower page_set repeat to 1.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1056623002/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/bc7b42b8c88eb24f92b62010d37d8c53d9b242aa Cr-Commit-Position: refs/heads/master@{#323777} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
