New issue
Advanced search Search tips

Issue 632394 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

Add experiment group to ParseHTMLOnMainThread that coalesces chunks

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

Issue description

Often, we send a tiny chunk to the main thread which spins off a long running task like script execution, while if we had waited a little bit, we would have gotten a bunch more preloads before the long running task.

We should add a group to the the threaded experiment group to coalesce these chunks, like the non-threaded experiment does (implicitly).
 
Project Member

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

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

commit 803fb39346006820c009f206dc05dd62b12e82ee
Author: csharrison <csharrison@chromium.org>
Date: Fri Jul 29 01:00:24 2016

ParseHTMLOnMainThread: notify main thread of chunks less frequently

This patch adds an experiment variant to the
ParseHTMLOnMainThread experiment threaded group. It adds
a group that uses the threaded parser, but coalesces token
chunks in the background parser, notifying the main thread
less often. This can often result in issuing preloads faster
due to not eagerly parsing/scripting when tokens are about to
come in.

Note: this experiment group actually aligns the threaded behavior
closer to the non-threaded behavior. The non-threaded experiment
group also runs the entirety of pumpTokenizer when it is tokenizing,
whereas the threaded parser usually starts parsing at the first call to
notifyPendingTokenizedChunks (if it is idle). This is usually very early
on in pumpTokenizer, because we split up chunks very fine grained.
This experiment group tries not to start the main thread parsing until
we've tokenized a bit more.

TBR=kinuko@chromium.org

BUG= 632394 

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

[modify] https://crrev.com/803fb39346006820c009f206dc05dd62b12e82ee/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/803fb39346006820c009f206dc05dd62b12e82ee/third_party/WebKit/Source/core/frame/Settings.in
[modify] https://crrev.com/803fb39346006820c009f206dc05dd62b12e82ee/third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.cpp
[modify] https://crrev.com/803fb39346006820c009f206dc05dd62b12e82ee/third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.h
[modify] https://crrev.com/803fb39346006820c009f206dc05dd62b12e82ee/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp

Labels: Merge-Request-53
Labels: -Merge-Request-53 Merge-Review-53
Why can't we experiment on M54 dev instead?  Why does this need to come back to M53?
Cc: kouhei@chromium.org
We can experiment on M54 Dev if necessary, but the rest of the experiment is ready to experiment in M53.

This change isn't too complicated, and would let us experiment faster, but I'm happy to defer to testing this change in Dev if the convenience isn't worth it.
Project Member

Comment 5 by sheriffbot@chromium.org, Jul 30 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Fixed (was: Started)
Labels: -Merge-Review-53
Yeah, let's hold off for M54 please.

Sign in to add a comment