Open Bug 893113 Opened 11 years ago Updated 1 year ago

Slow innerHTML performance with img

Categories

(Core :: DOM: HTML Parser, defect, P5)

22 Branch
x86
macOS
defect

Tracking

()

People

(Reporter: u474930, Unassigned)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release)
Build ID: 20130618035212

Steps to reproduce:

When assigning a large quantity of text with innerHTML, the speed of the assignment is generally relative to the size of the string, which is expected. However assignment is ~significantly~ slower in Firefox (compared to other browsers) if even the simplest text contains <img>s. Tested in Windows and Mac.

Performance data and test at : http://jsperf.com/slow-innerhtml-performance-with-img

This has a real-world impact on the templating system we use.


Actual results:

In Firefox, the third test ("Simple text with img") is significantly slower compared to other browsers (the first two tests, without images, run about the same speed and seem correct).


Expected results:

I'd expect the third test in Firefox to be somewhat equivalent to other browsers.
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
Assigning text with an <img> starts the image load.  That image load has to start sometime; if not sync under the innerHTML setter then async soon after.  It's possible other browsers are just starting it async so you're not measuring that in those browsers.

Is your templating system doing the exact thing the benchmark does, where it sets the innerHTML over and over and over again to the same string with <img> tags?  If it does the set only once, then the time needed wouldn't change even if the image load starts async...
Flags: needinfo?(paul)
Depends on: 893916
My single-page app composes new views by reading strings out of <script> tags (template storage hack) and then using innerHTML to build a document fragment. Once I have the string, it's similar to how the jsperf test works.

I've also run tests using cloneNode() to try and bypass innerHTML, figuring it would be faster, but in Firefox it has identical slowness issues vs. other browsers. I suspect the underpinnings of innerHTML and cloneNode are the same in regards to embedded <img> elements.

It sounds reasonable that other browsers are doing some of the <img> processing asynchronously. However because those browsers return from innerHTML faster, they feel more responsive, even if total processing time does work out to about the same in the end, as you suggest.

FWIW, real world for my project specifically, the most complex view that I compose has about 75 calls to .innerHTML (or cloneNode), many of those with <img>s. In Firefox it takes an extra 500 to 750 ms vs. Chrome/Safari. Not the end of the world, but it adds up. During testing when I took all <img>s out of my templates, Firefox often performed faster than other browsers.
Flags: needinfo?(paul)
> Once I have the string, it's similar to how the jsperf test works.

Well, the jsperf test sets innerHTML to the string over and over.  Does the template do that?

> I suspect the underpinnings of innerHTML and cloneNode are the same in regards to
> embedded <img> elements.

Indeed.

> In Firefox it takes an extra 500 to 750 ms vs. Chrome/Safari

Yeah, that's no good.  How many <img> tags are involved in total, if I might ask?
Status: UNCONFIRMED → NEW
Ever confirmed: true
> Well, the jsperf test sets innerHTML to the string over and over.  Does the
> template do that?

Yes. One string (template) might represent the structure of a span with date/time placeholder, later populated by a controller. One might represent a profile picture, also populated by a controller. Etc.

> How many <img> tags are involved in total, if I might ask?

In the worst case I cited (500-750ms delay over Chrome/Safari) there are ~250 images. This is on a page which shows 20 profile summaries, where each summary has 10 or so images. Quite a few of those images are display:none (toggled button states). Many, obviously, are also the same image referenced multiple times.
> Yes.

One the same element?

Kicking off an image load should really not take 2-3ms... I wonder what's going on there.  The jsperf test is likewise showing closer to 6800 image loads per second, not the 300-500 that the 500-750ms delay suggests.

I'll do some profiling to check what's going on here, but could you check whether you have any addons like adblock, say, installed?
On Nightly

Simple text without img
5,380 ±0.28% fastest

Complex text without img
705 ±1.24% 87% slower

Simple text with img
136 ±8.09% 98% slower

AdBlock Plus and NoScript really interferes on the last test... with both enabled the result drops to 20.
Blink's experience with starting <img> load async from innerHTML was that it regressed perf, so the spec now requires sync loading for parser-created <img> and async for other <img>s (since scripts should be able to set src and srcset in any order without getting double downloads it needs to await a stable state).
> so the spec now requires sync loading for parser-created <img>

What does that mean, exactly?  Just sync check for an image-cached version, I presume, no?  You're not seriously requiring the whole load to be sync, I assume.

> and async for other <img>s 

You need sync check (from the point of view of scripts) for an image-cached version for scripted sets of src for compat.  Web pages do things like set src and immediately check the size and expect it to be the new size.  See bug 721659 for example.

Past that part, how can you require any aspect of image loading to be "sync" exactly?

> since scripts should be able to set src and srcset in any order without getting double
> downloads it needs to await a stable state

That's not web-compatible; see above.
Flags: needinfo?(zcorpan)
Also, as a smaller issue, I'd like to understand why we're changing the spec based on internal webkit implementation details here.  It seems to me like whether image loads (past the image cache lookup) start sync or async is not observable from the web page itself without cooperation from the server, right?  Though I guess with service workers it could be....  But with service workers you get all sorts of overspecification possibilities here.

Basically, UAs should be free to do the load as lazily as they'd like (including "on user request"), within current web compat constraints, and if the spec prevents that then the spec needs to be fixed.
The spec always sync checks for a cached image for <img src> since that's required for compat. I'm aware of that constraint.

No, the whole load is not sync, obviously. :-) But the browser is allowed to start the load immediately without awaiting a stable state (or start the load later if it likes).

As far as I know it should be web-compatible to start the actual download after awaiting a stable state in the scripted case.

The spec also allows you to load images on user request.

When the image starts loading is observable by checking .complete and, if you start loading the "wrong" image too early, you can end up with "double downloads" which Web developers notice and don't want.
Flags: needinfo?(zcorpan)
> The spec always sync checks for a cached image for <img src> since that's required for
> compat.

OK, good.  :)

> When the image starts loading is observable by checking .complete

How so?

Or more precisely how are we defining "starts loading"?  We could change .complete without actually starting a fetch, just while waiting to start it.  How could script tell?

> As far as I know it should be web-compatible to start the actual download after awaiting
> a stable state in the scripted case.

OK, so why do we want to make the "parser" case different?  That seems like extra complexity to work around what sounds like a Blink implementation limitation, so far.
Let's take this up in the W3C bug, since it's about what the spec has to say.
And in particular, because the spec doesn't prevent us from doing async the bits I want to do async here, and can't even try to do that.
Attached file innerHTML span vs img
Attached is a simple test program, it tests both multiple calls to innerHTML and a single call with a large string containing multiple tags.

On GNU/Linux, I can confirm that innerHTML is significantly slower with <img> tags than with other tags.

With AdBlock enabled innerHTML with an <img> tag with a valid src attribute is around 4-5 times slower than with AdBlock disabled or uninstalled. I think that's to be expected, though.

Interestingly, though, is that this test runs faster in every way in Firefox than in Chrome. Especially, the <img> with an empty src and <img> with a valid src. However, innerHTML with <img> is also slower in Chrome.
> innerHTML is significantly slower with <img> tags than with other tags

Did you try it with <script>?  <object>?  Other interesting tags that _do_ something, unlike <span>? ;)

Seriously, comparing <img> to <span> here is pointless: <img> is required by spec to do a bunch of extra work that <span> never needs to do.  This bug is about the <img> case in Gecko being slower than other browsers.  I can confirm this is no longer the case wrt Chrome: we seem to be about 10x faster than them now.  But we're about 6x slower than Safari....
You'll notice in my test the created <div> I'm innerHTML'ing the images into is never appended to any element in the document. So maybe some work is being done for the <img> tags before they are even appended to the document, things like, maybe, width and height calculations or other rendering preparations, checking the cache, setting up the display of the alt attribute, etc. And when it comes to AdBlock, checking the URL against the block list.

I feel none of that kind of work should be done before the element is appended to the document. That could be where the bug lies, though. But than again, it's slow in Chrome, too.

Maybe this isn't a bug.
> Did you try it with <script>?  <object>?  Other interesting tags that _do_ something, unlike <span>? ;)

Yes. I've tried with <script> (with and without code), <object>, <canvas>, <textarea>, etc. The most it slowed down by was a mere 10 ms. There is something seriously wrong with <img> if it's 100s of ms slower.
What's more, other element's with the src attribute aren't as slow as <img>.

For example: <img src="."> is very slow, while <iframe src="."></iframe> is not, nor is <embed>.
> So maybe some work is being done for the <img> tags before they are even appended to the
> document

The image load starts as soon as src is set for an <img>, even if the <img> is not in the document.  This is required for web compat and by the spec.  Consider the |var img = new Image(); img.src = ...| pattern that's commonly used.

> I feel none of that kind of work should be done before the element is appended to the
> document.

That would be nice, but as far as I know is not web-compatible.

> I've tried with <script> 

Just realized that this is innerHTML, which doesn't run <script>, ever.  So sadly that's not a useful comparison...

> while <iframe src="."></iframe> is not

<iframe> is NOT loaded unless in a document.  

> nor is <embed>.

Likewise.

Unfortunately, you're comparing apples and oranges.

This is not to say that <img> handling shouldn't be faster; it should.  It will never be as fast as other tags, due to web compat constraints.
Priority: -- → P5
Component: DOM: Core & HTML → DOM: HTML Parser
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: