Issue metadata
Sign in to add a comment
|
Stop relying on the profiler option from Telemetry for the PGO builds. |
||||||||||||||||||||||
Issue description(this description assumes that you're familiar with the PGO 3-step build process, if not then check https://msdn.microsoft.com/en-us/library/e7k32f4k.aspx) Currently the profiling step of the PGO builds rely on the '--profiler' option/API offered by Telemetry[1] to collect the profiling data. The Telemetry team is planning to stop supporting this option in the feature because it's hard for them to maintain it (it's really dependent on Chrome's architecture) and they know that it won't work for their newer/more complex benchmarks. The aspect of this API that we care about in a PGO build is the callback that we get before terminating the renderer processes[2] (see the 'CollectProfile' function in [1]). It allows us to call a small utility program (pgomgr.exe, see [3]) that extract all the profiling information from the running renderer processes and save them into some .pgc files. Ideally we should add a build flag to Chrome that specify that we're doing an instrumented build for profiling, with this flag we could add a callback to a small utility application that run PGOSweep before terminating a process, this might be quite invasive because there's several way to kill a process in Chrome, so it'll require hooks at various place of the codebase. Or we could just do nothing and push for the switch to Clang, which will make all this work obsolete. [1] https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/internal/platform/profiler/win_pgo_profiler.py [2] This isn't 100% accurate, this callback gets called when we're about to close all the tabs started by this benchmark, this doesn't consider all the processes that can be created by a tab, Telemetry has no way to know if a given tab has spawned some sub-processes and AFAIK this is the main reason why this option is going away. [3] https://msdn.microsoft.com/en-us/library/2kw46d8w.aspx
,
May 4 2017
,
May 23 2017
Do you have any concrete timeline for this project?
,
May 23 2017
Not yet, I'm working on improving the build time first.
,
Jun 6 2017
Has anything changed in Telemetry recently? The profiling step has been really flaky recently, e.g. https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FChromium_Win_PGO_Builder%2F8996%2F%2B%2Frecipes%2Fsteps%2FProfiling_benchmarks.%2F0%2Fstdout : All the benchmarks fail.
,
Dec 11 2017
Lowering the priority of the PGO bugs as we've switched the official build to Clang. I'll close these bugs once we decide that we won't support the VS build anymore.
,
Dec 13 2017
,
Jan 13 2018
The NextAction date has arrived: 2018-01-13
,
Jan 16 2018
sebmarchand@, now that we use Win Clang build, can we remove this now?
,
Jan 16 2018
We could but as we want to keep the MSVC (/PGO) build working for at least 2 releases after switching to Clang I'd prefer to keep this bug open for now (as well as the other PGO ones). It's in my list of bugs to close once the switch to Clang is complete.
,
Jan 17 2018
SG. I will revisit this in 30 days
,
Jan 17 2018
,
Jan 17 2018
Actually 1 release ~= 6 weeks, so it would be 12 weeks from now
,
Apr 17 2018
The MSVC build ins't supported anymore, so it's fine to WontFix this bug as this profiler isn't needed anymore? (and we could remove it from Telemetry entirely).
,
Apr 17 2018
Won't fix away.
,
Apr 17 2018
,
Apr 18 2018
sebmarchand@: can you please remove PGO profiler from Telemetry before closing this bug?
,
Apr 18 2018
Sure, I'm trying to upload a CL that removes the telemetry/telemetry/internal/platform/profiler/win_pgo_profiler.py file and I'm getting the following message when running git cl upload: Credentials for the following hosts are required: github-review.com github.com These are read from /usr/local/google/home/sebmarchand/.gitcookies (or legacy /usr/local/google/home/sebmarchand/.netrc) You can (re)generate your credentials by visiting https://github-review.com/new-password github-review.com doesn't exist, what is the correct way to set my credentials?
,
Apr 18 2018
+Aaron: can you help with #18?
,
Apr 18 2018
I am making the CL instead: https://chromium-review.googlesource.com/c/catapult/+/1017009
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/a4264f97124c119d9ce6c7eab56ea2a5441589ef commit a4264f97124c119d9ce6c7eab56ea2a5441589ef Author: Nghia Nguyen <nednguyen@google.com> Date: Wed Apr 18 16:29:29 2018 Remove PGO profiler Bug: chromium:713328 Change-Id: I6108c318951bb16120ba1b90970d9f5910db1284 TBR=perezju@chromium.org Change-Id: I6108c318951bb16120ba1b90970d9f5910db1284 Reviewed-on: https://chromium-review.googlesource.com/1017009 Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Ned Nguyen <nednguyen@google.com> [delete] https://crrev.com/a903f96d720c4a4524567a7a25901705eb079f77/telemetry/telemetry/internal/platform/profiler/win_pgo_profiler.py
,
Apr 18 2018
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3cf4a083db5ccfada14f383916dbb6bcd9157134 commit 3cf4a083db5ccfada14f383916dbb6bcd9157134 Author: catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Wed Apr 18 19:00:41 2018 Roll src/third_party/catapult/ a903f96d7..a4264f971 (1 commit) https://chromium.googlesource.com/catapult.git/+log/a903f96d720c..a4264f97124c $ git log a903f96d7..a4264f971 --date=short --no-merges --format='%ad %ae %s' 2018-04-18 nednguyen Remove PGO profiler Created with: roll-dep src/third_party/catapult BUG= chromium:713328 The AutoRoll server is located here: https://catapult-roll.skia.org Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. TBR=sullivan@chromium.org Change-Id: I2865ddcff394972b702a750fc2fc22c54611adbf Reviewed-on: https://chromium-review.googlesource.com/1017223 Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#551769} [modify] https://crrev.com/3cf4a083db5ccfada14f383916dbb6bcd9157134/DEPS
,
Apr 20 2018
The NextAction date has arrived: 2018-04-20
,
Jan 16
,
Jan 16
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by nedngu...@google.com
, Apr 19 2017