Issue metadata
Sign in to add a comment
|
WebView perf tests (rendering.mobile) broke when Chrome changed policy for proxy bypass of localhost URLs |
||||||||||||||||||||||
Issue descriptionLots of rendering.mobile benchmarks seem to have become a bit confused and report weird improvements. For a full list, see https://chromeperf.appspot.com/group_report?rev=606122.
,
Nov 19
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14adb0f3e40000
,
Nov 19
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/12a50e48140000
,
Nov 19
😿 Pinpoint job stopped with an error. https://pinpoint-dot-chromeperf.appspot.com/job/14adb0f3e40000 All of the runs failed. The most common error (10/20 runs) was: HTTPException: HTTP status code 400: {"error": {"message": "CIPD package path is required. Use \".\" to install to run dir."}}
,
Nov 19
😿 Pinpoint job stopped with an error. https://pinpoint-dot-chromeperf.appspot.com/job/12a50e48140000 All of the runs failed. The most common error (10/20 runs) was: HTTPException: HTTP status code 400: {"error": {"message": "CIPD package path is required. Use \".\" to install to run dir."}}
,
Nov 19
,
Nov 22
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/11cd5be0140000
,
Nov 22
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/107b50ffe40000
,
Nov 22
📍 Found significant differences after each of 3 commits. https://pinpoint-dot-chromeperf.appspot.com/job/107b50ffe40000 Implicitly bypass localhost when proxying requests. by eroman@chromium.org https://chromium.googlesource.com/chromium/src/+/da790f920bbc169a6805a4fb83b4c2ab09532d91 thread_raster_cpu_time_per_frame: 0.3527 → No values Build internal android targets in telemetry_chrome_test. by jbudorick@chromium.org https://chromium.googlesource.com/chromium/src/+/e0c7bc0e869c70995d8b53d9a22e68294f07c7f2 thread_raster_cpu_time_per_frame: No values → No values Revert "Build internal android targets in telemetry_chrome_test." by jbudorick@chromium.org https://chromium.googlesource.com/chromium/src/+/c7539ef6291836eff22077b38ae55bb62f536b99 thread_raster_cpu_time_per_frame: No values → No values Understanding performance regressions: http://g.co/ChromePerformanceRegressions Benchmark documentation link: https://bit.ly/rendering-benchmarks
,
Nov 22
Could you paste the full browser command-line being used for the affected rendering.mobile test? My change (da790f920bbc169a6805a4fb83b4c2ab09532d91) had the effect of turning off proxying of http://localhost/ and http://127.0.0.1/ by default for non-ChromeOS (ChromeOS was already doing that). To keep tests running as they were -- since some telemetry tests rely on proxying localhost for traffic shaping, and failure to proxy could impact their runtime -- I added the command line switch "--proxy-bypass-list=<-loopback>" [1]. I need to add this in all the places where --proxy-server=XXX was being passed, and it seemed to be centralized to just one place. ChromeOS required some different surgery as it never proxied localhost to begin with, and had some entangled dependencies as a result. My change could be causing differences if either: (1) The test launcher is some other script I missed, and is passing --proxy-server without a corresponding --proxy-bypass-list=<-loopback> (2) The test launcher is using a means other than --proxy-server to specify the proxy server. For instance if it were relying on OS level proxy settings, a Chrome extension, or PAC script to configure things. I should be able to confirm or rule out (1) and (2) given the command line flag Chrome is launched with. Thanks! [1] https://chromium.googlesource.com/catapult.git/+/HEAD/telemetry/telemetry/internal/backends/chrome/chrome_startup_args.py#102
,
Nov 22
I don't think any of those changes are the culprit here. Seems like there's some infra problem with the pinpoint jobs.
,
Nov 22
,
Nov 22
📍 Found significant differences after each of 3 commits. https://pinpoint-dot-chromeperf.appspot.com/job/11cd5be0140000 Telemetry: finally move rendering to TBMv2 by chiniforooshan@chromium.org https://chromium.googlesource.com/chromium/src/+/05af02c8afe6a0fdc3d5e32ff88632fa15f5c567 avg_surface_fps: 2.142 → No values Build internal android targets in telemetry_chrome_test. by jbudorick@chromium.org https://chromium.googlesource.com/chromium/src/+/e0c7bc0e869c70995d8b53d9a22e68294f07c7f2 avg_surface_fps: No values → No values Revert "Build internal android targets in telemetry_chrome_test." by jbudorick@chromium.org https://chromium.googlesource.com/chromium/src/+/c7539ef6291836eff22077b38ae55bb62f536b99 avg_surface_fps: No values → No values Understanding performance regressions: http://g.co/ChromePerformanceRegressions Benchmark documentation link: https://bit.ly/rendering-benchmarks
,
Nov 23
,
Nov 23
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/10656884140000
,
Nov 24
📍 Found significant differences after each of 3 commits. https://pinpoint-dot-chromeperf.appspot.com/job/10656884140000 Implicitly bypass localhost when proxying requests. by eroman@chromium.org https://chromium.googlesource.com/chromium/src/+/da790f920bbc169a6805a4fb83b4c2ab09532d91 frame_times: 129.9 → No values Build internal android targets in telemetry_chrome_test. by jbudorick@chromium.org https://chromium.googlesource.com/chromium/src/+/e0c7bc0e869c70995d8b53d9a22e68294f07c7f2 frame_times: No values → No values Revert "Build internal android targets in telemetry_chrome_test." by jbudorick@chromium.org https://chromium.googlesource.com/chromium/src/+/c7539ef6291836eff22077b38ae55bb62f536b99 frame_times: No values → No values Understanding performance regressions: http://g.co/ChromePerformanceRegressions Benchmark documentation link: https://bit.ly/rendering-benchmarks
,
Nov 26
Hmm, since eroman@'s commit came up again in pinpoint, it might be worth looking into this further. eroman: You can view the (extremely long) command line here, if you search for "Browser command line": https://chrome-swarming.appspot.com/task?id=415b71fe9c26b310&refresh=10&show_raw=1 In particular, it includes: --proxy-server=socks://localhost:48257 and --proxy-bypass-list=<-loopback>.
,
Nov 26
Thanks! I took a look and that command line is correct. There shouldn't be any functional differences from the Chromium proxy change. Given this is flip-flopping without any conclusive regression range, could this be either a bot runtime issue, or a probabilistically enabled experiment? How do the perfbots interact with Finch-enabled trials? For instance the "NetworkService" feature is enabled at 50% on Canary (but off by default). If something like that were being applied to perfbot runs it could account for performance non-determinism.
,
Nov 27
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/15eebbf0140000
,
Nov 27
+perezju for the finch/feature question I started another pinpoint job [1] on a patch [2] that reverts a number of recent proxy changes, to see if that resolves the "no data" issue on TOT. [1] https://pinpoint-dot-chromeperf.appspot.com/job/15eebbf0140000 [2] https://chromium-review.googlesource.com/c/chromium/src/+/1350925
,
Nov 27
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/15eebbf0140000
,
Nov 27
The results of that pinpoint job show: - the frame_time metric (and a number of others) are not reported on tip-of-tree - after reverting the proxy-related changes, they are reported again. Looks more and more like the disappearance is related to the proxy changes (and not just due to infra flakes), most likely the one pinpoint bisected in #13 and #16.
,
Nov 27
Sorry, meant to refer to #9 and #16, not #13.
,
Nov 27
Correct, it really looks like the proxy change r606112 caused *all* of the renderer.mobile metrics to vanish in the WebView perf bots. Note on the dashboard the metrics "improve" to zero. The pinpoint job in #13 appears to have caught another issue that also happened a bit earlier in the same range, at r605718, where only *some* of the metrics disappeared. That one is being followed on issue 905165.
,
Nov 27
,
Nov 27
,
Nov 27
I am having a hard time reading and interpreting the perfbot links, so may need some hand-holding :) Is it only the WebView tests that are having problems, or any other platforms? I believe WebView is quite different in how it handles command line flags (writes it to a file and then interprets that), so one possibility is that it is not understanding/applying the --proxy-bypass-list=<-loopback> switch which is necessary to preserve previous behavior after my proxy changes. There is also an API for WebView users to set their own bypasses, which might be overriding our flag. As far as expedient fixes: Unfortunately reverting my proxy policy changes will be tricky as the policy change was made to solve a security bug. I can try to roll-back parts of the policy but not the entire thing. If this is disruptive and we need to green things right away I can try to ifdef some things out specifically for WebView. Could you send the following CLs to the poinpoint trybots? (and/or explain to me how to do that; haven't had time to learn all this infrastructure yet): https://chromium-review.googlesource.com/c/chromium/src/+/1352469 https://chromium-review.googlesource.com/c/chromium/src/+/1351947 One of those CLs does a smaller policy change over the bigger revert you tested. The other one aims to see if the webview headless mode is failing to set the --proxy-bypass-list flag (will crash on failure). Any tips on running locally appreciated, as I won't be able to give this my full attention today. Thanks!
,
Nov 28
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/1614a37be40000
,
Nov 28
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/139e899fe40000
,
Nov 28
Started two jobs for the patches. You can do this yourself by navigating to https://pinpoint-dot-chromeperf.appspot.com, logging in with chromium or google account, and pressing the + button on the bottom right. There, you can provide the URL to the change to test. For the other params, you can have a look at the pinpoint jobs in #28/#29 :) See here for docs on how to run the benchmarks locally: https://chromium.googlesource.com/catapult/+/HEAD/telemetry/docs/run_benchmarks_locally.md
,
Nov 28
Oh, and I believe it's only the WebView bots that are affected.
,
Nov 28
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/139e899fe40000
,
Nov 28
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/1614a37be40000
,
Nov 28
#30: Thanks eseckler! I can also confirm the failure goes away when changing policy for bypass on manual proxy settings. Is there a way to see the LOG(ERROR) messagesl
,
Nov 28
#34: No need to answer that question - I found the LOG(ERROR) - they just weren't showing up for the CHECK() details.
,
Nov 29
,
Nov 29
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4bc3f6d6ec942c97293e303f469ccdc4bf551fc2 commit 4bc3f6d6ec942c97293e303f469ccdc4bf551fc2 Author: Eric Roman <eroman@chromium.org> Date: Fri Nov 30 02:02:46 2018 Recognize --proxy-bypass-list from WebView. Bug: 906540 Change-Id: Ic0cae00166b1d3ffb85eb3dcae6f9df22a6252d8 Reviewed-on: https://chromium-review.googlesource.com/c/1356006 Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org> Commit-Queue: Eric Roman <eroman@chromium.org> Cr-Commit-Position: refs/heads/master@{#612514} [modify] https://crrev.com/4bc3f6d6ec942c97293e303f469ccdc4bf551fc2/android_webview/browser/net/aw_url_request_context_getter.cc
,
Nov 30
,
Dec 3
,
Dec 3
Issue 906678 has been merged into this issue.
,
Dec 3
,
Dec 3
Issue 911130 has been merged into this issue.
,
Dec 4
,
Dec 5
Issue 911578 has been merged into this issue.
,
Dec 5
,
Dec 5
Issue 911132 has been merged into this issue.
,
Dec 10
,
Dec 17
,
Jan 12
,
Jan 12
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Nov 19