|
|
Chromium Code Reviews
DescriptionDevTools: do not report paint metrics under blink.user_timing category.
(blink.user_timing operates user land trace names and should not be mixed with other categories).
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Review-Url: https://codereview.chromium.org/2802313002
Cr-Commit-Position: refs/heads/master@{#463498}
Committed: https://chromium.googlesource.com/chromium/src/+/f18b4bfa08f94f62114ca49a3125fc4edbc06bf2
Patch Set 1 #
Total comments: 1
Patch Set 2 : rebaselined #
Messages
Total messages: 27 (15 generated)
Description was changed from ========== DevTools: do not report paint metrics under blink.user_timing category. (blink.user_timing operates user land trace names and should not be mixed with other categories). ========== to ========== DevTools: do not report paint metrics under blink.user_timing category. (blink.user_timing operates user land trace names and should not be mixed with other categories). CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
pfeldman@chromium.org changed reviewers: + caseq@chromium.org, fmeawad@chromium.org
https://codereview.chromium.org/2802313002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/paint/PaintTiming.cpp (right): https://codereview.chromium.org/2802313002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/paint/PaintTiming.cpp:154: TRACE_EVENT_INSTANT1("rail,devtools.timeline", "firstContentfulPaint", I only found this one mentioned in catapult, but I did not find the category check there, only the name one. So is this part safe to land then?
The CQ bit was checked by pfeldman@chromium.org to run a CQ dry run
lgtm
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
fmeawad@chromium.org changed reviewers: + nednguyen@google.com, tdresser@chromium.org
Tim and Ned, can you check if this is safe for the loading metric?
On 2017/04/07 19:42:02, fmeawad wrote: > Tim and Ned, can you check if this is safe for the loading metric? LGTM Pavel, can you do an A/B testing for loading metric benchmark using results2.html? benjhayden can help.
Description was changed from ========== DevTools: do not report paint metrics under blink.user_timing category. (blink.user_timing operates user land trace names and should not be mixed with other categories). CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== DevTools: do not report paint metrics under blink.user_timing category. (blink.user_timing operates user land trace names and should not be mixed with other categories). CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
On 2017/04/07 19:44:27, fmeawad wrote: > On 2017/04/07 19:42:02, fmeawad wrote: > > Tim and Ned, can you check if this is safe for the loading metric? > > LGTM > > Pavel, can you do an A/B testing for loading metric benchmark using > results2.html? benjhayden can help. Tell me what I need to run and I will!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/04/07 20:14:46, pfeldman wrote: > On 2017/04/07 19:44:27, fmeawad wrote: > > On 2017/04/07 19:42:02, fmeawad wrote: > > > Tim and Ned, can you check if this is safe for the loading metric? > > > > LGTM > > > > Pavel, can you do an A/B testing for loading metric benchmark using > > results2.html? benjhayden can help. > > Tell me what I need to run and I will! The fact that tests in https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/... is passing in CQ tells me that thing is probably fine. If you want to check whether this shift the metrics' value, you can build the release browser, then run: tools/perf/run_benchmark page_cycler_v2.typical_25 --browser=system --reset --results-label='without_change" tools/perf/run_benchmark page_cycler_v2.typical_25 --browser=release --results-label='with_change" After this, you can show us the tools/perf/results.html file & so we can determine whether this patch effects the loading metric.
I could not wait for it to finish - takes minutes and minutes of time, so I am landing it as is.
The CQ bit was checked by pfeldman@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was unchecked by pfeldman@chromium.org
The CQ bit was checked by pfeldman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fmeawad@chromium.org, caseq@chromium.org Link to the patchset: https://codereview.chromium.org/2802313002/#ps20001 (title: "rebaselined")
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": 1491870090337190,
"parent_rev": "5e94029ae66f4c36265570d712852896f28d3ad2", "commit_rev":
"f18b4bfa08f94f62114ca49a3125fc4edbc06bf2"}
Message was sent while issue was closed.
Description was changed from ========== DevTools: do not report paint metrics under blink.user_timing category. (blink.user_timing operates user land trace names and should not be mixed with other categories). CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== DevTools: do not report paint metrics under blink.user_timing category. (blink.user_timing operates user land trace names and should not be mixed with other categories). CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2802313002 Cr-Commit-Position: refs/heads/master@{#463498} Committed: https://chromium.googlesource.com/chromium/src/+/f18b4bfa08f94f62114ca49a3125... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/f18b4bfa08f94f62114ca49a3125... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
