568430 - Perf of insertBefore testcase much much slower than test with appendChild

Status: RESOLVED WORKSFORME

(Core :: Disability Access APIs, defect)

Description

[Marco Zehe (:MarcoZ)]

Brought to my attention by Shaver yesterday.

STR:
1. Open above URL.
2. Click the SECOND test button. With accessibility enabled, this takes just a couple hundred MS on my machine. This example uses the appendChild method to append children to the DOM.
3. Now, press the FIRST of the test buttons, the one that uses insertBefore. This froze my browser for several minutes (I had NVDA running). For Shaver, presumably on his tablet, this froze his browser for about 30 seconds.

My suspicion is that the insertBefore has to continually resize the array and fires reorder events en masse (at least 80000 reorder events possibly), whereas appending to the end is much faster and the events that are fired aren't that many.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Is this a regression? I assume that we've been behaving this way forever? If so, this is probably not a blocker, but something we'd really like to fix.

If it is a regression, please renominate.

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> Is this a regression? I assume that we've been behaving this way forever? If
> so, this is probably not a blocker, but something we'd really like to fix.

It really should be blocking even if it's not a regression because if the browser is unusable then it should block release.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

If this makes "the browser is unusable", how come we haven't heard about this until just a few weeks ago, after many releases of firefox.

There's always going to be edgecase testcases out there that bring us to our knees. We should of course work on fixing as many of them as possible, but unless this is something that affects many websites then I don't think we should hold a release for it.

So that said, does this affects any real-world websites that we know about.

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

We only diagnosed it recently.  The underlying issue, being the fact that we coalesce accessibility events since 2005, affects *every* web site that does DHTML, if the a11y stuff is activated (tablet drivers, anti-virus, etc.).  Most people being affected by it probably just think that Firefox is slow.  (When I force-disabled the a11y stuff on my machine a little while back it was like getting a new browser.)

Sites get more complex all the time, which means that O(n^3 * n!) algorithms will hurt us where they didn't always.  Similarly, operating system contexts change, as with tablet drivers causing a11y to be triggered (unavoidably, I think).

I feel pretty strongly that we should not ship Firefox 4 with coalescing that affects browser performance in a way that we can measure.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ok. I need help judging risk though, and someone that has the time and knowledge to work on it.

Alexander, could you take this? How complex to you expect the patch to be, i.e. would it be something that we are fine with landing later in the game, or do we need to fix this before beta1?

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

This will be fixed by the bold coalescing plans, I believe.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

bug#?

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Done and done :)

Comment 9

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> Ok. I need help judging risk though, and someone that has the time and
> knowledge to work on it.
> 
> Alexander, could you take this? How complex to you expect the patch to be, i.e.
> would it be something that we are fine with landing later in the game, or do we
> need to fix this before beta1?

It's definitely on my track and as Mike said it must be a part of bold plan of coalescence improvements. I'm not sure now how complex is patch expected, I think I can come with estimation in few weeks.

Comment 10

[alexander :surkov (:asurkov)]

Actually appendChild and insertBefore tests (http://hacks.mozilla.org/2010/05/better-performance-with-lazy-frame-construction/) inserts/appends different amount of children: appendChild appends 2000, insertBefore - 80000. I made these numbers equal: appendChild is slower than insertBefore (with and without a11y). What is this bug about?

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

You could close as dupe or fixed. Unassigning myself.