Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2094143005:
[perf] Enable non-verbose logging in memory system health benchmarks (Closed)
|
Created:
4 years, 5 months ago by petrcermak Modified:
4 years, 5 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[perf] Enable non-verbose logging in memory system health benchmarks
Rationale: To help triage flakes on the perf waterfall.
BUG=623058
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
Committed: https://crrev.com/a29030040dff42fe4dc0317d81f49bdaf80f1185
Cr-Commit-Position: refs/heads/master@{#402784}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Add TODOs #Patch Set 3 : System Health only #Messages
Total messages: 31 (12 generated)
Description was changed from ========== [perf] Enable non-verbose logging in memory-infra and system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 ========== to ========== [perf] Enable non-verbose logging in memory-infra and system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ==========
petrcermak@chromium.org changed reviewers: + nednguyen@google.com, perezju@chromium.org, primiano@chromium.org
PTAL. Thanks, Petr https://codereview.chromium.org/2094143005/diff/1/tools/perf/benchmarks/syste... File tools/perf/benchmarks/system_health.py (right): https://codereview.chromium.org/2094143005/diff/1/tools/perf/benchmarks/syste... tools/perf/benchmarks/system_health.py:113: (This extra blank line is intentional. According to the Python style guide, there should be 2 blank lines between top-level definitions: http://google.github.io/styleguide/pyguide.html?showone=Blank_Lines#Blank_Lines)
Description was changed from ========== [perf] Enable non-verbose logging in memory-infra and system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 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 ========== [perf] Enable non-verbose logging in memory-infra and memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ==========
Do you see any memory metrics move with this?
non-owner LGTM from my side. Provided that Ned is happy and the waterfall doesn't go crazy after this cl :)
maybe add a TODO to remove the logging (and switch to a log-on-retry logic) after the issues are solved? Alternatively, if memory numbers do not appear to be very much affected I would be OK with leaving this for longer.
On 2016/06/27 22:27:21, nednguyen wrote: > Do you see any memory metrics move with this? I don't expect that to happen, but I'll watch the dashboard and we should be able spot such changes very quickly in my opinion. I don't see any simple way to check this locally, so I suggest we land it and revert if there's a problem. WDYT? Thanks, Petr
On 2016/06/28 10:25:36, petrcermak wrote: > On 2016/06/27 22:27:21, nednguyen wrote: > > Do you see any memory metrics move with this? > > I don't expect that to happen, but I'll watch the dashboard and we should be > able spot such changes very quickly in my opinion. I don't see any simple way to > check this locally, so I suggest we land it and revert if there's a problem. > WDYT? > > Thanks, > Petr maybe ... $ tools/perf/run_benchmark try android-nexus5 memory.top_10_mobile_tbmv2 and see what happens?
On 2016/06/28 10:28:49, perezju wrote: > On 2016/06/28 10:25:36, petrcermak wrote: > > On 2016/06/27 22:27:21, nednguyen wrote: > > > Do you see any memory metrics move with this? > > > > I don't expect that to happen, but I'll watch the dashboard and we should be > > able spot such changes very quickly in my opinion. I don't see any simple way > to > > check this locally, so I suggest we land it and revert if there's a problem. > > WDYT? > > > > Thanks, > > Petr > > maybe ... > > $ tools/perf/run_benchmark try android-nexus5 memory.top_10_mobile_tbmv2 > > and see what happens? Thanks for the command :-) WITHOUT logging: https://codereview.chromium.org/2104873002/ WITH logging: https://codereview.chromium.org/2108593003/ I also added the TODOs as you suggested. Thanks, Petr
lgtm
On 2016/06/28 10:51:25, petrcermak wrote: > On 2016/06/28 10:28:49, perezju wrote: > > On 2016/06/28 10:25:36, petrcermak wrote: > > > On 2016/06/27 22:27:21, nednguyen wrote: > > > > Do you see any memory metrics move with this? > > > > > > I don't expect that to happen, but I'll watch the dashboard and we should be > > > able spot such changes very quickly in my opinion. I don't see any simple > way > > to > > > check this locally, so I suggest we land it and revert if there's a problem. > > > WDYT? > > > > > > Thanks, > > > Petr > > > > maybe ... > > > > $ tools/perf/run_benchmark try android-nexus5 memory.top_10_mobile_tbmv2 > > > > and see what happens? > > Thanks for the command :-) > > WITHOUT logging: https://codereview.chromium.org/2104873002/ > WITH logging: https://codereview.chromium.org/2108593003/ > > I also added the TODOs as you suggested. > > Thanks, > Petr The run WITHOUT logging succeeded: https://codereview.chromium.org/2104873002/ The run WITH logging failed (device_status_check), so I'm running another one: https://codereview.chromium.org/2101353002/ Petr
lgtm
The CQ bit was checked by petrcermak@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: winx64_10_perf_cq on tryserver.chromium.perf (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64_10_perf_c...)
Patchset #3 (id:40001) has been deleted
On 2016/06/28 19:24:23, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > winx64_10_perf_cq on tryserver.chromium.perf (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64_10_perf_c...) It seems that the logging does have a negative impact on memory measurements (see http://storage.googleapis.com/chromium-telemetry/html-results/results-2016-06...). Given this and after a discussion with Juan, I'll enable logging on SH benchmarks only. The rationale is that we are currently not tracking them (so it's not a disaster if they suddenly become noisy etc.). Petr
Description was changed from ========== [perf] Enable non-verbose logging in memory-infra and memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 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 ========== [perf] Enable non-verbose logging in memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ==========
Description was changed from ========== [perf] Enable non-verbose logging in memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 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 ========== [perf] Enable non-verbose logging in memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ==========
The CQ bit was checked by petrcermak@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from primiano@chromium.org, nednguyen@google.com Link to the patchset: https://codereview.chromium.org/2094143005/#ps60001 (title: "System Health only")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [perf] Enable non-verbose logging in memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ========== to ========== [perf] Enable non-verbose logging in memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [perf] Enable non-verbose logging in memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq ========== to ========== [perf] Enable non-verbose logging in memory system health benchmarks Rationale: To help triage flakes on the perf waterfall. BUG=623058 CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq Committed: https://crrev.com/a29030040dff42fe4dc0317d81f49bdaf80f1185 Cr-Commit-Position: refs/heads/master@{#402784} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/a29030040dff42fe4dc0317d81f49bdaf80f1185 Cr-Commit-Position: refs/heads/master@{#402784}
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:60001) has been created in https://codereview.chromium.org/2124333002/ by rbyers@chromium.org. The reason for reverting is: Seeing system_health.memory_desktop failures on Windows. petrcermak@ says this CL is the likely culprit and should be reverted. https://bugs.chromium.org/p/chromium/issues/detail?id=625172.
Message was sent while issue was closed.
On 2016/07/07 14:54:10, Rick Byers wrote: > A revert of this CL (patchset #3 id:60001) has been created in > https://codereview.chromium.org/2124333002/ by mailto:rbyers@chromium.org. > > The reason for reverting is: Seeing system_health.memory_desktop failures on > Windows. petrcermak@ says this CL is the likely culprit and should be reverted. > > https://bugs.chromium.org/p/chromium/issues/detail?id=625172. Petr, can you try just enable this for the smoke?
Message was sent while issue was closed.
On 2016/07/07 20:58:41, nednguyen (ooo til 7-11) wrote: > On 2016/07/07 14:54:10, Rick Byers wrote: > > A revert of this CL (patchset #3 id:60001) has been created in > > https://codereview.chromium.org/2124333002/ by mailto:rbyers@chromium.org. > > > > The reason for reverting is: Seeing system_health.memory_desktop failures on > > Windows. petrcermak@ says this CL is the likely culprit and should be > reverted. > > > > https://bugs.chromium.org/p/chromium/issues/detail?id=625172. > > Petr, can you try just enable this for the smoke? Sure (https://codereview.chromium.org/2133093002/), but I don't expect the tests to pass on Windows either. |
