Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2557483002:
Add blink_perf.paint_spinvalidation and blink_perf.svg_spinvalidation suites (Closed)
DescriptionAdd blink_perf.paint_spinvalidation and blink_perf.svg_spinvalidation suites
BUG=646176
CQ_INCLUDE_TRYBOTS=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/28340254faacae55d9c14a9ac94efd57d6f79748
Cr-Commit-Position: refs/heads/master@{#436820}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Rename, comments. #
Dependent Patchsets: Messages
Total messages: 15 (7 generated)
Description was changed from ========== Add blink_perf.paint_spinvalidation and blink_perf.svg_spinvalidation suites BUG=646176 ========== to ========== Add blink_perf.paint_spinvalidation and blink_perf.svg_spinvalidation suites BUG=646176 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ==========
wangxianzhu@chromium.org changed reviewers: + pdr@chromium.org
LGTM https://codereview.chromium.org/2557483002/diff/1/tools/perf/benchmarks/blink... File tools/perf/benchmarks/blink_perf.py (right): https://codereview.chromium.org/2557483002/diff/1/tools/perf/benchmarks/blink... tools/perf/benchmarks/blink_perf.py:122: class _BlinkPerfMeasurementSPInvalidation(_BlinkPerfMeasurement): It's really long but WDYT of changing this to: _BlinkPerfMeasurementSlimmingPaintInvalidation I think this will help reduce confusion if other people read this code and are not familiar with our 'SP' names. https://codereview.chromium.org/2557483002/diff/1/tools/perf/benchmarks/blink... tools/perf/benchmarks/blink_perf.py:278: class BlinkPerfPaintSPInvalidation(BlinkPerfPaint): Can you add a link to either of the design docs and a note that this can be removed when slimming-paint-invalidation ships (here and below)? I think this can be done with a python class comment: """The blink_perf.paint_slimmingpaintinvalidation tests measure paint time with the new paint invalidation system (see: https://goo.gl/eQczQW). These tests should be removed when slimming paint invalidation ships. """
wangxianzhu@chromium.org changed reviewers: + zhenw@chromium.org
+zhenw@ for owner's approval. https://codereview.chromium.org/2557483002/diff/1/tools/perf/benchmarks/blink... File tools/perf/benchmarks/blink_perf.py (right): https://codereview.chromium.org/2557483002/diff/1/tools/perf/benchmarks/blink... tools/perf/benchmarks/blink_perf.py:122: class _BlinkPerfMeasurementSPInvalidation(_BlinkPerfMeasurement): On 2016/12/06 06:47:28, pdr. wrote: > It's really long but WDYT of changing this to: > _BlinkPerfMeasurementSlimmingPaintInvalidation > > I think this will help reduce confusion if other people read this code and are > not familiar with our 'SP' names. Done. https://codereview.chromium.org/2557483002/diff/1/tools/perf/benchmarks/blink... tools/perf/benchmarks/blink_perf.py:278: class BlinkPerfPaintSPInvalidation(BlinkPerfPaint): On 2016/12/06 06:47:28, pdr. wrote: > Can you add a link to either of the design docs and a note that this can be > removed when slimming-paint-invalidation ships (here and below)? > > I think this can be done with a python class comment: > """The blink_perf.paint_slimmingpaintinvalidation tests measure paint time > with the new paint invalidation system (see: https://goo.gl/eQczQW). These > tests should be removed when slimming paint invalidation ships. > """ I put the following doc in _BlinkPerfMeasurementSlimmingPaintInvalidation: """Measures blink perf with the new paint invalidation system (see: https://goo.gl/eQczQW). The benchmarks using this measurement should be removed when slimming paint invalidation ships."""
lgtm
The CQ bit was checked by wangxianzhu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org Link to the patchset: https://codereview.chromium.org/2557483002/#ps20001 (title: "Rename, comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481061699958370,
"parent_rev": "e22ebbfcc08135ad48335a68a5dd32dd0251d7ea", "commit_rev":
"1b91105f409662b02eb8ded1da00b12841276b41"}
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add blink_perf.paint_spinvalidation and blink_perf.svg_spinvalidation suites BUG=646176 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ========== to ========== Add blink_perf.paint_spinvalidation and blink_perf.svg_spinvalidation suites BUG=646176 CQ_INCLUDE_TRYBOTS=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/28340254faacae55d9c14a9ac94efd57d6f79748 Cr-Commit-Position: refs/heads/master@{#436820} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/28340254faacae55d9c14a9ac94efd57d6f79748 Cr-Commit-Position: refs/heads/master@{#436820}
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2616343006/ by wangxianzhu@chromium.org. The reason for reverting is: We've enabled SlimmingPaintInvalidation for experimental which also enabled it for perf tests, so no longer need the separate suites.. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
