Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1016503002:
Add a slimming paint variant of the rasterize and record micro benchmark (Closed)
DescriptionAdd a slimming paint variant of the rasterize and record micro benchmark
This patch adds a new benchmark for slimming paint:
rasterize_and_record_micro.top_25_slimming_paint_smooth
I verified this test passes locally. Perf cl tryjobs:
linux - https://codereview.chromium.org/1010103002
nexus5 - https://codereview.chromium.org/1013653003
nexus4 - https://codereview.chromium.org/1012893003
BUG=464910
Committed: https://crrev.com/d1b914bd1347b4b125f3cca75227a7a7c5b768be
Cr-Commit-Position: refs/heads/master@{#321142}
Patch Set 1 #
Total comments: 1
Patch Set 2 : No reference build #Messages
Total messages: 15 (3 generated)
pdr@chromium.org changed reviewers: + chrishtr@chromium.org, sullivan@chromium.org
Slight change in plan--if we can only run one benchmark, rasterize and record is the one. This patch replaces https://codereview.chromium.org/989773002 and should get us most of the numbers we need for tracking slimming paint as we get closer to launching. @Annie, would you please take a look?
lgtm
https://codereview.chromium.org/1016503002/diff/1/tools/perf/benchmarks/raste... File tools/perf/benchmarks/rasterize_and_record_micro.py (right): https://codereview.chromium.org/1016503002/diff/1/tools/perf/benchmarks/raste... tools/perf/benchmarks/rasterize_and_record_micro.py:59: @benchmark.Disabled('mac', 'win') Do you also want: @benchmark.Disabled('reference')? The reference build is currently M37. I'm guessing that slimming paint isn't really ready on the reference builds until they get to M43 or so?
On 2015/03/16 at 21:26:02, sullivan wrote: > https://codereview.chromium.org/1016503002/diff/1/tools/perf/benchmarks/raste... > File tools/perf/benchmarks/rasterize_and_record_micro.py (right): > > https://codereview.chromium.org/1016503002/diff/1/tools/perf/benchmarks/raste... > tools/perf/benchmarks/rasterize_and_record_micro.py:59: @benchmark.Disabled('mac', 'win') > Do you also want: > > @benchmark.Disabled('reference')? > > The reference build is currently M37. I'm guessing that slimming paint isn't really ready on the reference builds until they get to M43 or so? Good idea, done. I've also contacted the perf sheriff about the linux bisect bot never succeeding "without patch".
On 2015/03/16 21:31:45, pdr wrote: > On 2015/03/16 at 21:26:02, sullivan wrote: > > > https://codereview.chromium.org/1016503002/diff/1/tools/perf/benchmarks/raste... > > File tools/perf/benchmarks/rasterize_and_record_micro.py (right): > > > > > https://codereview.chromium.org/1016503002/diff/1/tools/perf/benchmarks/raste... > > tools/perf/benchmarks/rasterize_and_record_micro.py:59: > @benchmark.Disabled('mac', 'win') > > Do you also want: > > > > @benchmark.Disabled('reference')? > > > > The reference build is currently M37. I'm guessing that slimming paint isn't > really ready on the reference builds until they get to M43 or so? > > Good idea, done. > > I've also contacted the perf sheriff about the linux bisect bot never succeeding > "without patch". lgtm as long as the bisects pass. (note on the linux perf bisect: the step that actually ran the test passed, so that one is fine)
On 2015/03/16 at 21:39:48, sullivan wrote: > lgtm as long as the bisects pass. (note on the linux perf bisect: the step that actually ran the test passed, so that one is fine) Thanks! The trybots didn't pass--they caught a real android-only error. Fixed in https://src.chromium.org/viewvc/blink?view=rev&revision=192018. Another set of trybot runs. Go, go gadget: motoe - https://codereview.chromium.org/997183008 nexus4 - https://codereview.chromium.org/1013563004 nexus5 - https://codereview.chromium.org/1015823002 nexus10 - https://codereview.chromium.org/1018693002
On 2015/03/17 at 18:02:14, pdr wrote: > On 2015/03/16 at 21:39:48, sullivan wrote: > > lgtm as long as the bisects pass. (note on the linux perf bisect: the step that actually ran the test passed, so that one is fine) > > Thanks! > > The trybots didn't pass--they caught a real android-only error. Fixed in https://src.chromium.org/viewvc/blink?view=rev&revision=192018. > > Another set of trybot runs. Go, go gadget: > motoe - https://codereview.chromium.org/997183008 > nexus4 - https://codereview.chromium.org/1013563004 > nexus5 - https://codereview.chromium.org/1015823002 > nexus10 - https://codereview.chromium.org/1018693002 These all ended up failing in exactly the same way because blink didn't roll all day. Our perf testing infrastructure has really improved lately; there were no flakes / weird errors / etc in any of these 4 android runs. Kudos perf team! I just unblocked and landed the blink roll, so we have another set of tries: motoe - https://codereview.chromium.org/1012133003 nexus4 - https://codereview.chromium.org/1014943003 nexus5 - https://codereview.chromium.org/1002393006 nexus10 - https://codereview.chromium.org/1016873003
On 2015/03/18 05:52:41, pdr wrote: > On 2015/03/17 at 18:02:14, pdr wrote: > > On 2015/03/16 at 21:39:48, sullivan wrote: > > > lgtm as long as the bisects pass. (note on the linux perf bisect: the step > that actually ran the test passed, so that one is fine) > > > > Thanks! > > > > The trybots didn't pass--they caught a real android-only error. Fixed in > https://src.chromium.org/viewvc/blink?view=rev&revision=192018. > > > > Another set of trybot runs. Go, go gadget: > > motoe - https://codereview.chromium.org/997183008 > > nexus4 - https://codereview.chromium.org/1013563004 > > nexus5 - https://codereview.chromium.org/1015823002 > > nexus10 - https://codereview.chromium.org/1018693002 > > These all ended up failing in exactly the same way because blink didn't roll all > day. Our perf testing infrastructure has really improved lately; there were no > flakes / weird errors / etc in any of these 4 android runs. Kudos perf team! Prasadv has been working really hard on this! > I just unblocked and landed the blink roll, so we have another set of tries: > motoe - https://codereview.chromium.org/1012133003 > nexus4 - https://codereview.chromium.org/1014943003 > nexus5 - https://codereview.chromium.org/1002393006 > nexus10 - https://codereview.chromium.org/1016873003 I took a look at these. It's passing on the motoe, nexus4, nexus5, but the "Running Without Patch" step is red and fails because there's no test to run. I filed http://crbug.com/468291 about this. The nexus10 bot seems to be offline. I'm okay with submitting this with the 3 passing trybots.
On 2015/03/18 at 13:10:57, sullivan wrote: > On 2015/03/18 05:52:41, pdr wrote: > > On 2015/03/17 at 18:02:14, pdr wrote: > > > On 2015/03/16 at 21:39:48, sullivan wrote: > > > > lgtm as long as the bisects pass. (note on the linux perf bisect: the step > > that actually ran the test passed, so that one is fine) > > > > > > Thanks! > > > > > > The trybots didn't pass--they caught a real android-only error. Fixed in > > https://src.chromium.org/viewvc/blink?view=rev&revision=192018. > > > > > > Another set of trybot runs. Go, go gadget: > > > motoe - https://codereview.chromium.org/997183008 > > > nexus4 - https://codereview.chromium.org/1013563004 > > > nexus5 - https://codereview.chromium.org/1015823002 > > > nexus10 - https://codereview.chromium.org/1018693002 > > > > These all ended up failing in exactly the same way because blink didn't roll all > > day. Our perf testing infrastructure has really improved lately; there were no > > flakes / weird errors / etc in any of these 4 android runs. Kudos perf team! > > Prasadv has been working really hard on this! > > > I just unblocked and landed the blink roll, so we have another set of tries: > > motoe - https://codereview.chromium.org/1012133003 > > nexus4 - https://codereview.chromium.org/1014943003 > > nexus5 - https://codereview.chromium.org/1002393006 > > nexus10 - https://codereview.chromium.org/1016873003 > > I took a look at these. It's passing on the motoe, nexus4, nexus5, but the "Running Without Patch" step is red and fails because there's no test to run. I filed http://crbug.com/468291 about this. The nexus10 bot seems to be offline. I'm okay with submitting this with the 3 passing trybots. Thanks for your help. Off to the cq
The CQ bit was checked by pdr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1016503002/#ps20001 (title: "No reference build")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1016503002/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/d1b914bd1347b4b125f3cca75227a7a7c5b768be Cr-Commit-Position: refs/heads/master@{#321142} |
