New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 729747 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.2%-177.6% regression in system_health.memory_mobile at 476641:476894

Project Member Reported by m...@chromium.org, Jun 5 2017

Issue description

See the link to graphs below.
 

Comment 1 by m...@chromium.org, Jun 5 2017

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=729747

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgxsannAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgxueVrAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg5oykuwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpsPsuQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpsPsuQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpp-r4wgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpt_SugkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgxrDmqwgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpsaXtwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDghvTK-AkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpqzW-AgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgxr--pwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpqibtwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpvykmQsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpoeevQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgxv3w7gsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpqz7rQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpr23pQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpoegpAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgxtfg_wkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpu-jsAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgptHnoQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpqKv-wkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDghr221QgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgprulugkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpujyoQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgpp2PvQsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgptKttgsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgxqTDmwkM


Bot(s) for this bug's original alert(s):

android-nexus5X
android-one
chromium-rel-mac11
chromium-rel-mac11-air
chromium-rel-win10
chromium-rel-win7-dual
chromium-rel-win7-gpu-intel
chromium-rel-win7-x64-dual
linux-release
Cc: y...@yoav.ws
Owner: y...@yoav.ws

=== 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!
Cc: hansberry@chromium.org
 Issue 729752  has been merged into this issue.
Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, Jun 12 2017

Cc: alexclarke@chromium.org
 Issue 732313  has been merged into this issue.

Comment 6 by y...@yoav.ws, Jun 29 2017

Status: Started (was: Untriaged)
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

Comment 7 by y...@yoav.ws, Jun 29 2017

Reverting the relevant parts (LinkLoader::GetResourceTypeFromAsAttribute) locally shows no improvement. Any advice on how to further debug this?
Cc: primiano@chromium.org
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?

Comment 9 by m...@chromium.org, Jul 1 2017

Cc: -m...@chromium.org
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

Comment 11 by y...@yoav.ws, 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?

Comment 12 by y...@yoav.ws, Aug 23 2017

Cc: csharrison@chromium.org
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.
Yoav, your reasoning makes sense to me. Might be worth devrel reachout to these sites if it repros on their live, non WPR sites.

Comment 14 by y...@yoav.ws, Aug 23 2017

Cc: addyo@chromium.org
Status: WontFix (was: Started)
+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