Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2049593002:
[PCv2] Add tracing category needed for computing ttfmp (Closed)
|
Created:
4 years, 6 months ago by kouhei (in TOK) Modified:
4 years, 6 months ago CC:
chromium-reviews, telemetry-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[PCv2] Add tracing category needed for computing ttfmp
The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled.
This CL enables recording of the category in PCv2.
https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metrics/system_health/first_paint_metric.html
BUG=None
Committed: https://crrev.com/f9bc2493524f028607a8755c5bcfd8f51ba17998
Cr-Commit-Position: refs/heads/master@{#399186}
Patch Set 1 #
Total comments: 2
Patch Set 2 : add devtools.timeline cat #Patch Set 3 : use AddDisabledByDefault #Messages
Total messages: 32 (13 generated)
Description was changed from ========== [PCv2] Add tracing category needed for computing ttfmp The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled. This CL enables recording of the category in PCv2. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... BUG=None ========== to ========== [PCv2] Add tracing category needed for computing ttfmp The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled. This CL enables recording of the category in PCv2. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... BUG=None CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ==========
kouhei@chromium.org changed reviewers: + ksakamoto@chromium.org, nednguyen@google.com
https://codereview.chromium.org/2049593002/diff/1/tools/perf/benchmarks/page_... File tools/perf/benchmarks/page_cycler_v2.py (right): https://codereview.chromium.org/2049593002/diff/1/tools/perf/benchmarks/page_... tools/perf/benchmarks/page_cycler_v2.py:26: cat_filter.AddIncludedCategory('disabled-by-default-blink.debug.layout') We also need devtools.timeline category for the paint events.
https://codereview.chromium.org/2049593002/diff/1/tools/perf/benchmarks/page_... File tools/perf/benchmarks/page_cycler_v2.py (right): https://codereview.chromium.org/2049593002/diff/1/tools/perf/benchmarks/page_... tools/perf/benchmarks/page_cycler_v2.py:26: cat_filter.AddIncludedCategory('disabled-by-default-blink.debug.layout') On 2016/06/08 01:56:50, Kunihiko Sakamoto wrote: > We also need devtools.timeline category for the paint events. Done.
lgtm
On 2016/06/08 02:00:42, Kunihiko Sakamoto wrote: > lgtm lg2me but I think we should delay landing this on https://bugs.chromium.org/p/chromium/issues/detail?id=618174 because fixing that bug may also shift the metrics number a bit. Separate the two changes that can cause the metrics to move up/down will make it easier to analyze which causes what
On 2016/06/08 03:12:11, nednguyen wrote: > On 2016/06/08 02:00:42, Kunihiko Sakamoto wrote: > > lgtm > > lg2me but I think we should delay landing this on > https://bugs.chromium.org/p/chromium/issues/detail?id=618174 because fixing that > bug may also shift the metrics number a bit. Separate the two changes that can > cause the metrics to move up/down will make it easier to analyze which causes > what sgtm
On 2016/06/08 03:29:40, kouhei wrote: > On 2016/06/08 03:12:11, nednguyen wrote: > > On 2016/06/08 02:00:42, Kunihiko Sakamoto wrote: > > > lgtm > > > > lg2me but I think we should delay landing this on > > https://bugs.chromium.org/p/chromium/issues/detail?id=618174 because fixing > that > > bug may also shift the metrics number a bit. Separate the two changes that can > > cause the metrics to move up/down will make it easier to analyze which causes > > what > > sgtm lgtm but please wait 1 or 2 days before checking cq
The CQ bit was checked by kouhei@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2049593002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by kouhei@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com, ksakamoto@chromium.org Link to the patchset: https://codereview.chromium.org/2049593002/#ps40001 (title: "use AddDisabledByDefault")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2049593002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_retina_perf_cq on tryserver.chromium.perf (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_...)
The CQ bit was checked by kouhei@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2049593002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_retina_perf_cq on tryserver.chromium.perf (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_...)
On 2016/06/10 15:07:38, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > mac_retina_perf_cq on tryserver.chromium.perf (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_...) Looks like page_cycler_v2.typical_25 is passing in https://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf.... I remove the CQ_EXTRA_TRYBOT
Description was changed from ========== [PCv2] Add tracing category needed for computing ttfmp The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled. This CL enables recording of the category in PCv2. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... BUG=None CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ========== to ========== [PCv2] Add tracing category needed for computing ttfmp The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled. This CL enables recording of the category in PCv2. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... BUG=None ==========
The CQ bit was checked by nednguyen@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2049593002/40001
Message was sent while issue was closed.
Description was changed from ========== [PCv2] Add tracing category needed for computing ttfmp The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled. This CL enables recording of the category in PCv2. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... BUG=None ========== to ========== [PCv2] Add tracing category needed for computing ttfmp The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled. This CL enables recording of the category in PCv2. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... BUG=None ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== [PCv2] Add tracing category needed for computing ttfmp The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled. This CL enables recording of the category in PCv2. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... BUG=None ========== to ========== [PCv2] Add tracing category needed for computing ttfmp The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled. This CL enables recording of the category in PCv2. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... BUG=None Committed: https://crrev.com/f9bc2493524f028607a8755c5bcfd8f51ba17998 Cr-Commit-Position: refs/heads/master@{#399186} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/f9bc2493524f028607a8755c5bcfd8f51ba17998 Cr-Commit-Position: refs/heads/master@{#399186}
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2057413002/ by fmeawad@chromium.org. The reason for reverting is: Speculative revert, suspecting crashing the reference test page_cycler_v2.typical_25.reference on mac. BUG=619254. |
