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

Issue 627575 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

The document.write evaluator is in standard preloads critical path.

Project Member Reported by csharrison@chromium.org, Jul 12 2016

Issue description

The logic in HTMLDocumentParser evaluates document.write scripts first, then deals with standard preloads. This ordering should be reversed, as the evaluator can add extra delay to fetching the standard preloads.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9cb7a71a7369a77b2d0c400b2a7e436f489bc2c0

commit 9cb7a71a7369a77b2d0c400b2a7e436f489bc2c0
Author: csharrison <csharrison@chromium.org>
Date: Wed Jul 13 04:45:18 2016

Move document.write evaluation after standard preloads

This patch moves document.write evaluation to take place after all the
preloads the HTMLDocumentParser knows about are fetched.

This shaves off preload delay when the evaluator is busy evaluating or
initializing the context.

BUG= 627575 

Review-Url: https://codereview.chromium.org/2146673002
Cr-Commit-Position: refs/heads/master@{#405026}

[modify] https://crrev.com/9cb7a71a7369a77b2d0c400b2a7e436f489bc2c0/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp

Reading the motivation for this bug, I was wondering: does the evaluator still add delay in this new ordering (e.g. to the next parsing step)?
  If so, do we have data on how much latency is added? 
  Would it be useful to do a dummy experiment where the evaluator doesn't trigger any additional preload to know its baseline cost?
Cc: bmcquade@chromium.org
Labels: Merge-Request-53
Yes we know exactly how much latency is added (usually 0-1ms for evaluating and ~5ms for initializing the context). This is in PreloadScanner.* histograms.

Your suggestion to add another experiment group is a good idea. +bmcquade, WDYT?

Also requesting a merge on this so it goes out asap.
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 13 2016

Labels: merge-merged-2795
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9cb7a71a7369a77b2d0c400b2a7e436f489bc2c0

commit 9cb7a71a7369a77b2d0c400b2a7e436f489bc2c0
Author: csharrison <csharrison@chromium.org>
Date: Wed Jul 13 04:45:18 2016

Move document.write evaluation after standard preloads

This patch moves document.write evaluation to take place after all the
preloads the HTMLDocumentParser knows about are fetched.

This shaves off preload delay when the evaluator is busy evaluating or
initializing the context.

BUG= 627575 

Review-Url: https://codereview.chromium.org/2146673002
Cr-Commit-Position: refs/heads/master@{#405026}

[modify] https://crrev.com/9cb7a71a7369a77b2d0c400b2a7e436f489bc2c0/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp

Comment 5 by dimu@google.com, Jul 14 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 14 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39e2ca00a9b1c57902509ad191848a9cd180b7c8

commit 39e2ca00a9b1c57902509ad191848a9cd180b7c8
Author: csharrison <csharrison@chromium.org>
Date: Thu Jul 14 18:20:33 2016

Move document.write evaluation after standard preloads

This patch moves document.write evaluation to take place after all the
preloads the HTMLDocumentParser knows about are fetched.

This shaves off preload delay when the evaluator is busy evaluating or
initializing the context.

BUG= 627575 

Review-Url: https://codereview.chromium.org/2146673002
Cr-Commit-Position: refs/heads/master@{#405026}
(cherry picked from commit 9cb7a71a7369a77b2d0c400b2a7e436f489bc2c0)

TBR=kouhei@chromium.org
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2148363002
Cr-Commit-Position: refs/branch-heads/2785@{#120}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/39e2ca00a9b1c57902509ad191848a9cd180b7c8/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp

Status: Fixed (was: Started)

Sign in to add a comment