Issue metadata
Sign in to add a comment
|
1.2%-177.6% regression in system_health.memory_mobile at 476641:476894 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jun 5 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8977591707591474464
,
Jun 6 2017
=== Auto-CCing suspected CL author yoav@yoav.ws === Hi yoav@yoav.ws, the bisect results pointed to your CL, please take a look at the results. === BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : yoav Commit : 1f18863cc9b2a5fe463671f3eb60de2f95bbf283 Date : Sat Jun 03 00:20:44 2017 Subject: [preload] Mandatory `as` value and related spec alignments Bisect Details Configuration: win_x64_perf_bisect Benchmark : loading.desktop Metric : cpuTimeToFirstMeaningfulPaint_avg/pcv1-warm/TheOnion Change : 150.43% | 310.753555556 -> 778.232 Revision Result N chromium@476747 310.754 +- 461.471 9 good chromium@476816 287.602 +- 136.945 6 good chromium@476833 299.607 +- 201.879 6 good chromium@476838 296.55 +- 191.695 6 good chromium@476839 332.715 +- 264.837 6 good chromium@476840 763.925 +- 176.147 6 bad <-- chromium@476842 781.92 +- 189.386 6 bad chromium@476850 777.997 +- 303.387 6 bad chromium@476884 778.232 +- 220.476 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=TheOnion loading.desktop Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8977591707591474464 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5808386345533440 | O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq | X | for more information addressing perf regression bugs. For feedback, | / \ | file a bug with component Speed>Bisection. Thank you!
,
Jun 6 2017
,
Jun 12 2017
,
Jun 29 2017
I'm looking into this. I see how these benchmarks can be related to the change, as they include preload links with no `as` value. I'll dig further
,
Jun 29 2017
Reverting the relevant parts (LinkLoader::GetResourceTypeFromAsAttribute) locally shows no improvement. Any advice on how to further debug this?
,
Jun 29 2017
Thanks for looking into this. I had a look at your patch and nothing obvious pops out at me as being the cause. The bisect seems pretty clear that it is however :( Perhaps Memory-infra team can help you dig into the regression?
,
Jul 1 2017
,
Aug 18 2017
yoav: some of these regressions are pretty big (both memory and page load time). Did you have a chance to investigate? The docs for memory regressions are here: https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md
,
Aug 23 2017
So, I cannot re-run those exact benchmarks locally (on a Mac, no "release_x64"). Trying to run them as "release" and hoping that viable. One thing that *could* be the cause is the fact that now these tests show a console warning for trying to preload a resource with no `as` attribute. Does that make sense?
,
Aug 23 2017
OK, I'm able to recreate locally, and it seems like that change really regressed the site in question. The onion were (and still are) using preload without setting the right `as` attribute value for the downloaded resources.
```
<link rel="preload" href="//b.scorecardresearch.com/beacon.js" crossorigin />
<link rel="preload" href="//js-sec.indexww.com/ht/p/183957-129026818948246.js" crossorigin />
<link rel="preload" href="https://assets2.onionstatic.com/onion/static/css/main.8ea1896401d6.css" type="text/css" crossorigin />
```
That means that before this change:
* The resource was preloaded when the renderer's HTMLPreloadScanner hit the <link> tag token, and stored in MemoryCache with the wrong resource type. However, since the resources are cacheable, they were stored in the HTTP cache.
* When the renderer tried to fetch the actual JS/CSS resource (when parsing the corresponding tag), it did *not* find a MemoryCache match (wrong resource type), and continued on to fetch the resource from the HTTP cache.
Right now, because a preload is not triggered for these resources, they are not preloaded into the HTTP cache and the above scenario is replaced by:
* Preload is never triggered.
* The renderer finds the actual element that needs the resource and then fetches it from the server.
So the actual first meaningful paint time is longer *for the cold start case*, which means the CPU and memory consumption up until that point is larger for that case.
Let me know if this all makes sense. If so, I'll close this issue, since this is WAI.
,
Aug 23 2017
Yoav, your reasoning makes sense to me. Might be worth devrel reachout to these sites if it repros on their live, non WPR sites.
,
Aug 23 2017
+addyo At least for the onion, there are still 2 preloaded resources with no `as` value on their homepage right now... WontFixing as WAI. Feel free to reopen. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by m...@chromium.org
, Jun 5 2017