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

Issue 608078 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Shared resources (i.e. URLs that load files from ui/webui or third_party/polymer) should not hit the disk for mime type

Project Member Reported by dbeam@chromium.org, Apr 29 2016

Issue description

When I load a resource like chrome://resources/js/cr.js or chrome://resources/polymer/polymer.html, it should not hit the disk just looking for the MIME type for this URL.
 

Comment 1 by dbeam@chromium.org, Apr 29 2016

hrome://resources/html/polymer.html**

basically anything loaded by content/browser/webui/shared_resources_data_source.cc

Comment 2 by dbeam@chromium.org, Apr 30 2016

Cc: jam@chromium.org

Comment 3 by groby@google.com, Apr 30 2016

To give jam some context:

It's not actually hitting the disk, it just _might_. 

What happens is URLDataManagerBackend::StartRequest queries the data source for its message loop via MessageLoopForRequestPath.

SharedResourceDataSource returns the UI thread. This triggers a separate path in StartRequest, which posts GetMimeTypeOnUI to the UI thread. GetMimeTypeOnUI then calls GetMimeType on the source, which calls SharedResourcesDataSource::GetMimeType.

That (see comment in function,  bug #59849 ) _can_ go to disk on posix systems. 

So, first proposal is for SharedResourceDataSource::GetMimeType to simply resolve the known mime types by extension. https://codereview.chromium.org/1932753006/ - first patchset.

Second proposal, which was also implemented in that CL, was to simply not have a MessageLoop attached to SharedResourcesDataSource - which means everything stays on the IO thread.

Which saves us three PostTasks between threads for each access to chrome://resources
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 30 2016

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

commit 8678179613c967ec865b8ee7d1abd9427bd27233
Author: dbeam <dbeam@chromium.org>
Date: Sat Apr 30 03:26:27 2016

Figure out the MIME type of shared WebUI resources more efficiently

Previously, GetMimeType() was posted to run on UI thread and potentially
hit disk on Posix systems.

This CL fixes both by adding a hardcoded list (based on `find` scraping)
and requesting GetMimeType() be run from the originating thread (IO)
rather than posting to UI.

R=groby@chromium.org
BUG= 608078 

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

[modify] https://crrev.com/8678179613c967ec865b8ee7d1abd9427bd27233/content/browser/webui/shared_resources_data_source.cc
[modify] https://crrev.com/8678179613c967ec865b8ee7d1abd9427bd27233/content/browser/webui/shared_resources_data_source.h

Comment 5 by dbeam@chromium.org, May 2 2016

Status: Fixed (was: Started)

Comment 6 by groby@chromium.org, May 2 2016

Running a very informal test with my full profile (having accrued over 5 years), this change drops pure load time from 600ms to 500ms, so ~15% improvement.

#6: "pure load time" of what?

Comment 8 by groby@chromium.org, May 3 2016

of chrome://md-settings - i.e. the time until we start executing JS. 

It _used_ to be clearly visible in the previous version of Canary. All "ParseHTML" events were squished together at the beginning 600ms. New Canary is at 2300ms load time, and ParseHTML events are splattered over the first 1100ms now :(

I see similar timelines in 51.0.2704.29 and 52.0.2722.0: 300ms of mostly Parse HTML but also evaluating the Polymer JS files, then 300ms of evaluating other scripts, then a giant Parse HTML for chrome://md-settings itself which primarily includes the DOMContentLoaded event.
52.0.2722.0 as well, but a very scattered timeline (see attached image)
Screen Shot 2016-05-02 at 7.49.15 PM.png
48.9 KB View Download
Wow, very different from what I see.
foo.png
46.0 KB View Download

Comment 12 by groby@google.com, May 3 2016

That's what I usually see, as well - and the initial chunk (of blue humps) got shorter after the patch was applied. That's the part I was referring to :)

Sign in to add a comment