1696261 - Assigning a large data: URI to an image is unusually slow

Status: RESOLVED FIXED

(Core :: Networking, defect, P3)

Description

[marakesh7568]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:88.0) Gecko/20100101 Firefox/88.0

Steps to reproduce:

On the site blaseball.com, log in and navigate to the "shop" tab.
(Alternately, click the "add <img>" or "add <image>" buttons on the test page https://tar-outrageous-goldenrod.glitch.me/)

Actual results:

34 SVG <image> elements with xlink:href="data:..." URIs totaling about 11MB are created.
Loading these takes around 1200ms, and appears to spend a lot of time converting between utf-8 and utf-16, and removing whitespace: https://profiler.firefox.com/public/q5wbda6wrdp0v6vxwkpvkjgbd17xc2cpgr6dzgg/flame-graph/?globalTrackOrder=0-1-2&localTrackOrderByPid=819-2-3-0-1~951-0-1~&thread=3&transforms=ff-905~ff-1027&v=5

Expected results:

Ultimately this is a site issue, but it seems unusually slow. In Safari and Chrome it takes around 400ms and 200ms respectively.

I've reported it to the site, but wanted to note it here as well.
My best guess is that it's caused by a webpack asset inlining misconfiguration, so other sites might have the same misconfiguration, and it could have a small benefit on well-configured sites as well?

Comment 1

[Tom S. (please needinfo tschuster)]

Great testcase, thanks.

The code under nsDataHandler::CreateNewURI indeed seem to be performance bottleneck. Even just the StripWhitespace call shows up high on the glitch testcase, even though there doesn't seem to be any whitespace in the string. Most time is spend in net_FilterAndEscapeURI.

Comment 2

[Kershaw Chang [:kershaw]]

Valentin, could you take a look?
It seems this is not a common issue, so set this to P3 for now.

Comment 3

[Valentin Gosu [:valentin]]

There are definitely some quick optimizations we can make here to improve things.
The problem is that we do too many scans through the string when parsing the URL.
Here we could only do one.

And instead of having separate allocations for the query and hash, we can probably just pass in substrings into the calls to SetQuery and SetRef.

Comment 4

[Valentin Gosu [:valentin]]

Attachment: Bug 1696261 - Optimize nsDataHandler and nsSimpleURI::SetSpec to do fewer passes r=#necko

• Rename nsAciiMask.h details:: namespace to asciimask_details so it
  doesn't clash with ipc/chromium/src/base/task.h
• Add SetSpecAndFilterWhitespace simple URI constructor that filters
  whitespace instead of just CR/LF.
• Only do one scan of the string in nsSimpleURI::SetPathQueryRefInternal
  in order to find the end of the path, query & ref.

There are probably more optimizations possible.
In my testing these get me a 1.5x-2x speedup.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:valentin, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c8f8a58b61f
Optimize nsDataHandler and nsSimpleURI::SetSpec to do fewer passes r=necko-reviewers,kershaw

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/0c8f8a58b61f