New issue
Advanced search Search tips

Issue 657978 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Loading (big) URLs can be optimized in the loader stack.

Project Member Reported by csharrison@chromium.org, Oct 20 2016

Issue description

Off the top of my head:

- unnecessary allocation in KURL if the data url is already canonicalized
- unnecessary isolatedCopy in PreloadRequest. We never preload data urls!
- unecessary GURL copy in RenderFrameImpl::willSendRequest (only really used for extensions/chrome-search).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 25 2016

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 5 2016

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

commit 495bfff108b8c1b2f67bfbd0a7fffceb436b31f8
Author: csharrison <csharrison@chromium.org>
Date: Sat Nov 05 14:48:04 2016

[KURL] Avoid re-hashing input if is already canonicalized

KURL will re-hash the raw canonicalized output and check the
AtomicStringTable (addWithTranslator) for the string. This can be very
expensive for large URLs.

However, since many URLs are generated from existing AtomicStrings
(which already have their hashes computed), we can use a fast path
and just return the input if it was already canonicalized and hashed.

This optimization shaves 25% off the overhead of loading 1mb data
url images (if the data url is properly canonicalized). Data from
profiling results on Linux.

BUG=348655,657978

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

[modify] https://crrev.com/495bfff108b8c1b2f67bfbd0a7fffceb436b31f8/third_party/WebKit/Source/platform/weborigin/KURL.cpp
[modify] https://crrev.com/495bfff108b8c1b2f67bfbd0a7fffceb436b31f8/third_party/WebKit/Source/platform/weborigin/KURL.h

Summary: Loading (data) URLs can be optimized in the loader stack. (was: Loading data URLs can be optimized in the loader stack.)
Slightly reworded the title, because these optimizations apply to all URLs, they just apply *especially* to long URLs.

Another place I've seen some slowness is in StringUTF8Adaptor (used all over the place in Blink), especially in checking if the string contains only ascii. I wonder if we could lazily cache that in a bit in StringImpl?
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 1 2016

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

commit c64537201a63fc8436fcedd8edab32be3669405b
Author: csharrison <csharrison@chromium.org>
Date: Thu Dec 01 14:15:14 2016

[url] Avoid scanning for whitespace twice during ResolveRelative

For calls to url::ResolveRelative, if the input is not in fact relative,
we call RemoveURLWhitespace twice on the same buffer. This is not necessary.

On a test of loading 100 identical ~1mb data URL images, this patch speeds
up the renderer by ~8% on Linux.

BUG=657978

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

[modify] https://crrev.com/c64537201a63fc8436fcedd8edab32be3669405b/url/gurl_unittest.cc
[modify] https://crrev.com/c64537201a63fc8436fcedd8edab32be3669405b/url/url_util.cc

Summary: Loading (big) URLs can be optimized in the loader stack. (was: Loading (data) URLs can be optimized in the loader stack.)
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 6 2016

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

commit ad24ce2f18b3de5e33955675eb4710a6d722236f
Author: csharrison <csharrison@chromium.org>
Date: Tue Dec 06 14:37:20 2016

[url] Early return for finding query/ref parts in path parsing

For URLs with huge ref parts (~.5mb), this saves ~18% off of
url::ResolveRelative and ~10% off of KURL::init.

BUG=657978

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

[modify] https://crrev.com/ad24ce2f18b3de5e33955675eb4710a6d722236f/url/third_party/mozilla/url_parse.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 9 2017

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

commit 0b43ca02fdfd66fbd9a1a35dfd74be402f80e77d
Author: csharrison <csharrison@chromium.org>
Date: Mon Jan 09 14:01:21 2017

Avoid URL allocation for GetContentSettingFromRules fast path

This function is called all the time for both image and script loads from
Blink, resulting in extra WebURL -> GURL allocations. This is
particularly bad for images loaded via data URLs which can be big.

BUG=657978

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

[modify] https://crrev.com/0b43ca02fdfd66fbd9a1a35dfd74be402f80e77d/chrome/renderer/content_settings_observer.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 19 2017

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

commit 96b890e58809d310e0b70029913e0f72a00a4414
Author: csharrison <csharrison@chromium.org>
Date: Thu Jan 19 00:13:34 2017

[url] Reserve output buffer for various URL parsing paths

This patch moves buffer reservation to the internals of //url code,
whereas previously this was the caller's (e.g. GURL, KURL)
responsibility.

The reason for this change was that KURL was *not* doing any buffer reservation,
so a naive exponential buffer growth algorithm was being used. Rather than
expecting all callers to be bug free, this patch opts to always do the right thing.

For a ~1.5MB data URL, this yields a ~13% improvement of
DoCanonicalizePathComponent<char, unsigned>, which translates to a ~8%
improvement for the overall KURL::init.

BUG=657978

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

[modify] https://crrev.com/96b890e58809d310e0b70029913e0f72a00a4414/url/gurl.cc
[modify] https://crrev.com/96b890e58809d310e0b70029913e0f72a00a4414/url/url_canon.h
[modify] https://crrev.com/96b890e58809d310e0b70029913e0f72a00a4414/url/url_canon_relative.cc
[modify] https://crrev.com/96b890e58809d310e0b70029913e0f72a00a4414/url/url_util.cc

Sign in to add a comment