Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(338)

Issue 394923003: [telemetry] Add IPPET power monitor. (Closed)

Created:
6 years, 5 months ago by dtu
Modified:
6 years, 4 months ago
Reviewers:
tonyg
CC:
chromium-reviews, telemetry+watch_chromium.org
Project:
chromium
Visibility:
Public.

Description

[telemetry] Add IPPET power monitor. This power monitor lets our automated testers track power usage on Windows on Intel Sandy Bridge or later. BUG=336558, 153139 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=286541

Patch Set 1 #

Total comments: 17

Patch Set 2 : Comments, use util.path, add installer. #

Patch Set 3 : Add unit tests. #

Total comments: 5

Patch Set 4 : Check WaitForSingleObject return code. #

Patch Set 5 : Check quit output. #

Patch Set 6 : Increase timeout. #

Patch Set 7 : Close PyHANDLE #

Patch Set 8 : Check IPPET exit code and print temp directory on failure. #

Patch Set 9 : Increase timeout again to 20s. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+314 lines, -9 lines) Patch
A tools/telemetry/bin/win/ippet.zip.sha1 View 1 1 chunk +1 line, -0 lines 0 comments Download
M tools/telemetry/telemetry/core/platform/android_platform_backend.py View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download
M tools/telemetry/telemetry/core/platform/mac_platform_backend.py View 1 2 chunks +5 lines, -5 lines 0 comments Download
A tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py View 1 2 3 4 5 6 7 8 1 chunk +236 lines, -0 lines 0 comments Download
A tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor_unittest.py View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
M tools/telemetry/telemetry/core/platform/win_platform_backend.py View 1 2 3 4 5 6 3 chunks +38 lines, -0 lines 0 comments Download

Messages

Total messages: 21 (0 generated)
tonyg
lgtm https://codereview.chromium.org/394923003/diff/1/tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py File tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py (right): https://codereview.chromium.org/394923003/diff/1/tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py#newcode25 tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py:25: 'C:\\', 'Program Files (x86)', 'Intel', I think we ...
6 years, 5 months ago (2014-07-21 21:26:04 UTC) #1
dtu
https://chromiumcodereview.appspot.com/394923003/diff/1/tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py File tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py (right): https://chromiumcodereview.appspot.com/394923003/diff/1/tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py#newcode25 tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py:25: 'C:\\', 'Program Files (x86)', 'Intel', On 2014/07/21 21:26:03, tonyg ...
6 years, 5 months ago (2014-07-24 01:13:53 UTC) #2
tonyg
A couple of suggestions to get started on, still looking at the rest... https://chromiumcodereview.appspot.com/394923003/diff/100001/tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py File ...
6 years, 5 months ago (2014-07-24 01:32:04 UTC) #3
tonyg
lgtm Please consider the comments and land when you're ready. Thanks again! https://chromiumcodereview.appspot.com/394923003/diff/100001/tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py File tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py ...
6 years, 5 months ago (2014-07-24 01:38:29 UTC) #4
dtu
https://chromiumcodereview.appspot.com/394923003/diff/100001/tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py File tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py (right): https://chromiumcodereview.appspot.com/394923003/diff/100001/tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py#newcode196 tools/telemetry/telemetry/core/platform/power_monitor/ippet_power_monitor.py:196: re.match(r'Process\(Google Chrome [0-9]+\)', parts[3])): On 2014/07/24 01:38:28, tonyg wrote: ...
6 years, 5 months ago (2014-07-24 23:13:15 UTC) #5
dtu
On 2014/07/24 01:38:29, tonyg wrote: > lgtm > > Please consider the comments and land ...
6 years, 5 months ago (2014-07-25 00:07:28 UTC) #6
dtu
The CQ bit was checked by dtu@chromium.org
6 years, 5 months ago (2014-07-25 00:07:33 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dtu@chromium.org/394923003/100001
6 years, 5 months ago (2014-07-25 00:11:54 UTC) #8
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: chromium_presubmit on tryserver.chromium ...
6 years, 5 months ago (2014-07-25 04:33:13 UTC) #9
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 5 months ago (2014-07-25 04:36:57 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/builds/82305)
6 years, 5 months ago (2014-07-25 04:36:58 UTC) #11
tonyg
The CQ bit was checked by tonyg@chromium.org
6 years, 5 months ago (2014-07-25 13:56:36 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dtu@chromium.org/394923003/100001
6 years, 5 months ago (2014-07-25 13:57:01 UTC) #13
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_x64_rel on tryserver.chromium ...
6 years, 5 months ago (2014-07-25 18:07:00 UTC) #14
tonyg
On 2014/07/25 18:07:00, I haz the power (commit-bot) wrote: > FYI, CQ is re-trying this ...
6 years, 5 months ago (2014-07-25 18:23:40 UTC) #15
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 5 months ago (2014-07-25 20:09:30 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/builds/34730)
6 years, 5 months ago (2014-07-25 20:09:31 UTC) #17
dtu
The CQ bit was checked by dtu@chromium.org
6 years, 4 months ago (2014-07-30 16:28:42 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dtu@chromium.org/394923003/210001
6 years, 4 months ago (2014-07-30 16:29:54 UTC) #19
commit-bot: I haz the power
Change committed as 286541
6 years, 4 months ago (2014-07-30 17:01:15 UTC) #20
Sami
6 years, 4 months ago (2014-08-06 13:14:11 UTC) #21
Message was sent while issue was closed.
A revert of this CL has been created in
https://codereview.chromium.org/443973002/ by skyostil@chromium.org.

The reason for reverting is: The win8 perf
(http://build.chromium.org/p/chromium.perf/builders/Win%208%20Perf%20%281%29)
bot became pretty flaky right after this power monitoring patch landed, so I'd
like to pull it out. Some of the failures are clearly IPPET-related, but others
are silent timeouts.

Sample failures:

- AssertionError: Called StartMonitoringPower() twice.
http://build.chromium.org/p/chromium.perf/builders/Win%208%20Perf%20%281%29/b...

- IppetError: Timed out waiting for IPPET to close.
http://build.chromium.org/p/chromium.perf/builders/Win%208%20Perf%20%281%29/b...

- TimeoutException: Timed out while waiting 5s for IppetServerIsUp.
http://build.chromium.org/p/chromium.perf/builders/Win%208%20Perf%20%281%29/b...

- Generic timeout
http://build.chromium.org/p/chromium.perf/builders/Win%208%20Perf%20%281%29/b....

Powered by Google App Engine
This is Rietveld 408576698