Closed Bug 1696261 Opened 11 months ago Closed 10 months ago

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

Categories

(Core :: Networking, defect, P3)

Firefox 88
defect

Tracking

()

RESOLVED FIXED
89 Branch
Tracking Status
firefox89 --- fixed

People

(Reporter: marakesh7568, Assigned: valentin)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file)

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?

Component: Untriaged → Networking
Product: Firefox → Core

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.

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

Severity: -- → S4
Flags: needinfo?(valentin.gosu)
Priority: -- → P3

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.

Assignee: nobody → valentin.gosu
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(valentin.gosu)
Whiteboard: [necko-triaged]
  • 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.

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.

Flags: needinfo?(valentin.gosu)
Flags: needinfo?(kershaw)
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(kershaw)
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0c8f8a58b61f
Optimize nsDataHandler and nsSimpleURI::SetSpec to do fewer passes r=necko-reviewers,kershaw
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
You need to log in before you can comment on or make changes to this bug.