Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2851583002:
cc: Update rasterize and record to capture more work in recording. (Closed)
Descriptioncc: Update rasterize and record to capture more work in recording.
This patch ensures that we capture all relevant work that we do during
recording. This includes both the analysis (which is currently captured)
and discardable image generation (which is currently missed).
This is done via using RecordingSource, which does all of this work
during regular Chrome operations.
NOTE TO PERF SHERIFFS
=====================
This patch is expected to regress rasterize and record benchmark.
Some metrics like paint_time are expected to regress ~100%. This
is due to the fact that the current benchmark is not capturing
all of the work that the code normally does. The patch here
addresses this shortcoming by adding all of the work that needs
to be measured into the benchmark.
R=chrishtr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2851583002
Cr-Commit-Position: refs/heads/master@{#468041}
Committed: https://chromium.googlesource.com/chromium/src/+/78a9fddbe9db7c851cfefac5fd107aa11fcfc3ed
Patch Set 1 #
Total comments: 2
Patch Set 2 : rebase #Messages
Total messages: 24 (15 generated)
Description was changed from ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. R=chrishtr@chromium.org ========== to ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Please take a look. https://codereview.chromium.org/2851583002/diff/1/cc/benchmarks/rasterize_and... File cc/benchmarks/rasterize_and_record_benchmark.cc (right): https://codereview.chromium.org/2851583002/diff/1/cc/benchmarks/rasterize_and... cc/benchmarks/rasterize_and_record_benchmark.cc:143: recording_source.SetGenerateDiscardableImagesMetadata(true); This part is going away in a patch that is currently in flight. Once that lands, I'll remove the line and run the patch through ct first. The rest of the changes would stay the same.
Description was changed from ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by chrishtr@chromium.org
lgtm
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
khushalsagar@chromium.org changed reviewers: + khushalsagar@chromium.org
https://codereview.chromium.org/2851583002/diff/1/cc/benchmarks/rasterize_and... File cc/benchmarks/rasterize_and_record_benchmark.cc (right): https://codereview.chromium.org/2851583002/diff/1/cc/benchmarks/rasterize_and... cc/benchmarks/rasterize_and_record_benchmark.cc:143: recording_source.SetGenerateDiscardableImagesMetadata(true); On 2017/04/27 17:41:45, vmpstr wrote: > This part is going away in a patch that is currently in flight. Once that lands, > I'll remove the line and run the patch through ct first. The rest of the changes > would stay the same. The patch that removes this has landed. :P
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by vmpstr@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...
On 2017/04/27 17:56:33, Khushal wrote: > https://codereview.chromium.org/2851583002/diff/1/cc/benchmarks/rasterize_and... > File cc/benchmarks/rasterize_and_record_benchmark.cc (right): > > https://codereview.chromium.org/2851583002/diff/1/cc/benchmarks/rasterize_and... > cc/benchmarks/rasterize_and_record_benchmark.cc:143: > recording_source.SetGenerateDiscardableImagesMetadata(true); > On 2017/04/27 17:41:45, vmpstr wrote: > > This part is going away in a patch that is currently in flight. Once that > lands, > > I'll remove the line and run the patch through ct first. The rest of the > changes > > would stay the same. > > The patch that removes this has landed. :P Indeed. I've rebased and running ct now.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)
Description was changed from ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. NOTE TO PERF SHERIFFS ===================== This patch is expected to regress rasterize and record benchmark. Some metrics like paint_time are expected to regress ~100%. This is due to the fact that the current benchmark is not capturing all of the work that the code normally does. The patch here addresses this shortcoming by adding all of the work that needs to be measured into the benchmark. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Description was changed from ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. NOTE TO PERF SHERIFFS ===================== This patch is expected to regress rasterize and record benchmark. Some metrics like paint_time are expected to regress ~100%. This is due to the fact that the current benchmark is not capturing all of the work that the code normally does. The patch here addresses this shortcoming by adding all of the work that needs to be measured into the benchmark. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. NOTE TO PERF SHERIFFS ===================== This patch is expected to regress rasterize and record benchmark. Some metrics like paint_time are expected to regress ~100%. This is due to the fact that the current benchmark is not capturing all of the work that the code normally does. The patch here addresses this shortcoming by adding all of the work that needs to be measured into the benchmark. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by vmpstr@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/2851583002/#ps20001 (title: "rebase")
On 2017/04/27 19:03:07, vmpstr wrote: > On 2017/04/27 17:56:33, Khushal wrote: > > > https://codereview.chromium.org/2851583002/diff/1/cc/benchmarks/rasterize_and... > > File cc/benchmarks/rasterize_and_record_benchmark.cc (right): > > > > > https://codereview.chromium.org/2851583002/diff/1/cc/benchmarks/rasterize_and... > > cc/benchmarks/rasterize_and_record_benchmark.cc:143: > > recording_source.SetGenerateDiscardableImagesMetadata(true); > > On 2017/04/27 17:41:45, vmpstr wrote: > > > This part is going away in a patch that is currently in flight. Once that > > lands, > > > I'll remove the line and run the patch through ct first. The rest of the > > changes > > > would stay the same. > > > > The patch that removes this has landed. :P > > Indeed. I've rebased and running ct now. CT shows expected large regression, similar to what Khushal has seen when migrating gathering images to a different spot.
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": 1493397894719250,
"parent_rev": "fdd88ba132b6e7794715aee4d419b02fe7fa37b4", "commit_rev":
"78a9fddbe9db7c851cfefac5fd107aa11fcfc3ed"}
Message was sent while issue was closed.
Description was changed from ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. NOTE TO PERF SHERIFFS ===================== This patch is expected to regress rasterize and record benchmark. Some metrics like paint_time are expected to regress ~100%. This is due to the fact that the current benchmark is not capturing all of the work that the code normally does. The patch here addresses this shortcoming by adding all of the work that needs to be measured into the benchmark. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: Update rasterize and record to capture more work in recording. This patch ensures that we capture all relevant work that we do during recording. This includes both the analysis (which is currently captured) and discardable image generation (which is currently missed). This is done via using RecordingSource, which does all of this work during regular Chrome operations. NOTE TO PERF SHERIFFS ===================== This patch is expected to regress rasterize and record benchmark. Some metrics like paint_time are expected to regress ~100%. This is due to the fact that the current benchmark is not capturing all of the work that the code normally does. The patch here addresses this shortcoming by adding all of the work that needs to be measured into the benchmark. R=chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2851583002 Cr-Commit-Position: refs/heads/master@{#468041} Committed: https://chromium.googlesource.com/chromium/src/+/78a9fddbe9db7c851cfefac5fd10... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/78a9fddbe9db7c851cfefac5fd10... |
