BattOr Telemetry tests take much longer to run on Windows than non-BattOr tests |
||||
Issue descriptionTo reproduce, attach a BattOr to a Windows computer and run a Telemetry benchmark: tools/perf/run_benchmark battor.power_cases Observe how long the page stays up on the screen after the red light returns to a blinking yellow one on the BattOr (indicating that downloading the trace is complete). Something funny has to be happening here. aschulman@ and I suspect that it has to do with how slow streaming to STDOUT is on Windows laptops, and the fact that Telemetry's Python battor wrapper receives the trace through STDOUT. This can be observed by running the battor_agent directly from the command prompt and noticing how slowly the samples stream out relative to Linux and Mac. Could we maybe write to a temp file by using StopTracing's output file option, and then read directly out of that temp file for Telemetry?
,
Jun 28 2016
The trace is not gathered directly from STDOUT. It is already saving it to a file and later retrieving it. https://cs.chromium.org/chromium/src/third_party/catapult/common/battor/battor/battor_wrapper.py?q=battor_wrapper+StopTracing&sq=package:chromium&l=133&dr=CSs It is currently splitting lines then reforming them into one string, getting rid of this could decrease runtime.
,
Jun 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bbd6c910168b53f0e8da94a399aa451a38e436e6 commit bbd6c910168b53f0e8da94a399aa451a38e436e6 Author: catapult-deps-roller <catapult-deps-roller@chromium.org> Date: Wed Jun 29 22:29:04 2016 Roll src/third_party/catapult/ a45087135..b8081c5e8 (5 commits). https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/a4508713566b..b8081c5e8c38 $ git log a45087135..b8081c5e8 --date=short --no-merges --format='%ad %ae %s' BUG=624164, 622290 TBR=catapult-sheriff@chromium.org Review-Url: https://codereview.chromium.org/2107113003 Cr-Commit-Position: refs/heads/master@{#402959} [modify] https://crrev.com/bbd6c910168b53f0e8da94a399aa451a38e436e6/DEPS
,
Jun 30 2016
Aaron did some profiling of the code. It seems to spend a lot of time in trace2html doing json.dump().
,
Jun 30 2016
Actually I only looked at run_benchmark, it may also be taking some time in trace2html doing json.dump(). In run_benchmark, it is taking 25/58 seconds in GetTempFileHandle() which calls DumpTraceToFile which calls json.dump().
,
Jun 30 2016
Hmh, I thought it no longer does that after https://github.com/catapult-project/catapult/commit/184187257422cb5901d928ae75b3caa6b9b1ae42?
,
Jun 30 2016
Yeah, I was wrong about where the dump occurs. Its somewhere in run_benchmark, not trace2html (as Aaron said in his comment). https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/value/trace.py?q=DumpTraceToFile&sq=package:chromium&l=47
,
Jun 30 2016
Hmhh, at this point, I think we all know that json implementation is terrible in python. The fix here is to tweak the whole stack of getting trace from chrome, processing trace, dumping trace to file to be using only string instead of json-parsed object.
,
Jul 11 2016
Randy: any update on this?
,
Jul 14 2016
Also excited about this: Randy, any update?
,
Jul 14 2016
None yet :/ I just landed the other big non-battor project I was working on though, so this is getting 100% of my attention now.
,
Jul 14 2016
Great. Thanks for the update!
,
Jul 22 2016
I have just committed a change which is likely to mitigate this issue: https://codereview.chromium.org/2174543002/
,
Jul 25 2016
Should have tagged this CL with this bug: https://codereview.chromium.org/2160953003/
,
Jul 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f5bc3d9485a29e4bc94af36ac65d95e1ca57603 commit 1f5bc3d9485a29e4bc94af36ac65d95e1ca57603 Author: catapult-deps-roller <catapult-deps-roller@chromium.org> Date: Mon Jul 25 20:13:21 2016 Roll src/third_party/catapult/ 9224f7903..189c4816b (1 commit). https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/9224f7903b9c..189c4816b033 $ git log 9224f7903..189c4816b --date=short --no-merges --format='%ad %ae %s' BUG=624164 TBR=catapult-sheriff@chromium.org Review-Url: https://codereview.chromium.org/2179103002 Cr-Commit-Position: refs/heads/master@{#407557} [modify] https://crrev.com/1f5bc3d9485a29e4bc94af36ac65d95e1ca57603/DEPS
,
Jul 26 2016
Times taken from battor.system_health_laoding_desktop: https://uberchromegw.corp.google.com/i/chromium.perf.fyi/builders/Win%20Power%20High-DPI%20Perf Time before Me and Alex's CLs : 25m04s Time after both landed : 19m05s Time after reverting Alex's CL: 21m41s So, speedup by my CL was ~ 3 minutes 23 seconds from these sample runs. Speedup by Alex's CL is about 2 minutes 36 seconds. Still keeping this bug open since Alex's CL is reverted.
,
Aug 2
,
Jan 16
,
Jan 16
|
||||
►
Sign in to add a comment |
||||
Comment 1 by charliea@chromium.org
, Jun 28 2016