Open
Bug 893113
Opened 11 years ago
Updated 1 year ago
Slow innerHTML performance with img
Categories
(Core :: DOM: HTML Parser, defect, P5)
Tracking
()
NEW
People
(Reporter: u474930, Unassigned)
References
(Depends on 1 open bug)
Details
Attachments
(1 file)
6.67 KB,
text/html
|
Details |
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.
Updated•11 years ago
|
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
Comment 1•11 years ago
|
||
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)
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)
Comment 3•11 years ago
|
||
> 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.
Comment 5•11 years ago
|
||
> 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?
Comment 6•10 years ago
|
||
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.
Comment 7•10 years ago
|
||
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).
Comment 8•10 years ago
|
||
> 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)
Comment 9•10 years ago
|
||
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.
Comment 10•10 years ago
|
||
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)
Comment 11•10 years ago
|
||
> 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.
Comment 12•10 years ago
|
||
Let's take this up in the W3C bug, since it's about what the spec has to say.
Comment 13•10 years ago
|
||
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.
Comment 14•9 years ago
|
||
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.
Comment 15•9 years ago
|
||
> 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....
Comment 16•9 years ago
|
||
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.
Comment 17•9 years ago
|
||
> 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.
Comment 18•9 years ago
|
||
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>.
Comment 19•9 years ago
|
||
> 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.
Updated•6 years ago
|
Priority: -- → P5
Updated•3 years ago
|
Component: DOM: Core & HTML → DOM: HTML Parser
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•