|
|
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[PCv1] Avoid using dot in metrics name
Before this CL, the PCv1 emitted value names contained '.'.
The Chrome Performance Dashboard treats '.' in the value name specially,
thus messed up the graph views.
This CL workarounds the issue by renaming the value names to not contain
any '.'.
BUG=chromium:616342
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
Committed: https://crrev.com/046819a28863bf8300f3a377c024033c126cc712
Cr-Commit-Position: refs/heads/master@{#397369}
Patch Set 1 #Patch Set 2 : update unittests #Patch Set 3 : keep old #
Total comments: 2
Patch Set 4 : fix typo #Messages
Total messages: 38 (17 generated)
Description was changed from ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=catapult:#2357 ========== to ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=catapult:#2357 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: + nednguyen@google.com
ptal
The CQ bit was checked by kouhei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2029483002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2029483002/1
lgtm
Description was changed from ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=catapult:#2357 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 ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=catapult:#2357 ==========
On 2016/06/01 01:21:10, nednguyen wrote: > lgtm Actually please hold on landing this. I think you shouldn't need to prepend the 'cold' vs 'warm' to the metric name
kouhei@chromium.org changed reviewers: + sullivan@chromium.org
Annie: Would you also take a look?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2016/06/01 01:32:27, kouhei (catching up) wrote: > Annie: Would you also take a look? Can someone write a bug explaining what they expect to happen vs what actually happens with the '.' here?
Description was changed from ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=catapult:#2357 ========== to ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=chromium:616342 ==========
On 2016/06/01 03:00:33, sullivan wrote: > On 2016/06/01 01:32:27, kouhei (catching up) wrote: > > Annie: Would you also take a look? > > Can someone write a bug explaining what they expect to happen vs what actually > happens with the '.' here? I updated the bug: crbug.com/616342
Description was changed from ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=chromium:616342 ========== to ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=chromium:616342 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ==========
The CQ bit was checked by kouhei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2029483002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2029483002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/06/01 12:50:01, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. Kouhei, thinking back about this, I think we should not rename these metrics but instead keep the old one & add the new one with "-" instead of ".". We let the 2 old & 2 new metrics run in parallel for a while then removing the old one so that if there is regression happens, we won't confuse the bisect.
On 2016/06/01 16:11:28, nednguyen wrote: > On 2016/06/01 12:50:01, commit-bot: I haz the power wrote: > > Dry run: This issue passed the CQ dry run. > > Kouhei, thinking back about this, I think we should not rename these metrics but > instead keep the old one & add the new one with "-" instead of ".". > > We let the 2 old & 2 new metrics run in parallel for a while then removing the > old one so that if there is regression happens, we won't confuse the bisect. Once you also add the old metrics in the change, feel free to just land this.
lgtm https://codereview.chromium.org/2029483002/diff/40001/tools/perf/measurements... File tools/perf/measurements/page_cycler.py (right): https://codereview.chromium.org/2029483002/diff/40001/tools/perf/measurements... tools/perf/measurements/page_cycler.py:13: page_ typo? :P
https://codereview.chromium.org/2029483002/diff/40001/tools/perf/measurements... File tools/perf/measurements/page_cycler.py (right): https://codereview.chromium.org/2029483002/diff/40001/tools/perf/measurements... tools/perf/measurements/page_cycler.py:13: page_ On 2016/06/02 01:56:06, nednguyen wrote: > typo? :P typo. Ack. Will fix and land
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 Link to the patchset: https://codereview.chromium.org/2029483002/#ps60001 (title: "fix typo")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2029483002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2029483002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
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/2029483002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2029483002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
The CQ bit was checked by nednguyen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2029483002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2029483002/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=chromium:616342 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 ========== [PCv1] Avoid using dot in metrics name Before this CL, the PCv1 emitted value names contained '.'. The Chrome Performance Dashboard treats '.' in the value name specially, thus messed up the graph views. This CL workarounds the issue by renaming the value names to not contain any '.'. BUG=chromium:616342 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq Committed: https://crrev.com/046819a28863bf8300f3a377c024033c126cc712 Cr-Commit-Position: refs/heads/master@{#397369} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/046819a28863bf8300f3a377c024033c126cc712 Cr-Commit-Position: refs/heads/master@{#397369} |