1285977 - [DOM] Elliott's JS parser is slower in Firefox than in Blink/Webkit

Status: NEW

(Core :: DOM: Core & HTML, defect, P3)

Description

[Olli Pettay [:smaug][[e-mail]]]

+++ This bug was initially created as a clone of Bug #1285930 +++

https://bugs.chromium.org/p/chromium/issues/detail?id=595492


This bug is about the DOM parts of the issue. The original bug can be about JS.
From DOM code especially setAttribute shows up in 'Real Bindings' - especially HTMLImageElement's SetAttr

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is is the src attribute being set on the <img>?

Setting that does a whole bunch of sync work in Gecko; see bug 893916 (and the discussion in bug 893113).  But note that this should affect innerHTML just as much as it does setting things via the DOM APIs...

Comment 2

[Boris Zbarsky [:bzbarsky]]

OK, did a profile of this myself, for the "Real Bindings" part, using the testcase from bug 1285930.  Looks like this:

18% jitcode
14% appendChild, with time spent in various places but
    at least 1/3 realloc of the child array.
 5% GC
17% createTextNode.  Lots of malloc and wrapper allocation, of course.
17% createElement.  Gain, lots of malloc, no surprise.
27% ElementBinding::setAttribute, breaking down as:
    2% Allocating a stringbuffer (for the copy for long attr values, I guess)
    4% under HTMLImageElement::SetAttr, including all the LoadImage stuff.  And only 1.6% of
       this is under imgLoader.  So comment 1 is not that relevant, imo.
    3% Atomizing (presumably the attr name).  The atom table locks are showing up
       here for about 1.5%.
   15% under Element::SetAttr.  This has:
     3% ParseAtomArray (more atomization here!)
     2% nsAttrValue::SetTo doing string buffer alloc via nsAttrValue::GetStringBuffer
     1% ~nsAttrValue
     2% growing the attr and child array.

My conclusion is that what we need most for this testcase is a faster allocator.  :(

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Boris Zbarsky [:bz] from comment #2)
> My conclusion is that what we need most for this testcase is a faster
> allocator.  :(

I've noticed malloc/free in DOM code show up a lot on other benchmarks. In SpiderMonkey we got rid of many perf sensitive malloc stuff by using the nursery, GC heap, LifoAllocs, etc instead. Do other engines also use malloc for these DOM things?

Comment 4

[Boris Zbarsky [:bzbarsky]]

I think they do, but they may use a different malloc implementation than we do...

Comment 5

[Andrew Overholt [:overholt]]

P2 ~= "we want to fix this in the next few months". Is that reasonable? WHo's going to work on this?

Comment 6

[Andrew Overholt [:overholt]]

(In reply to Boris Zbarsky [:bz] (TPAC) from comment #2)
> My conclusion is that what we need most for this testcase is a faster
> allocator.  :(

I don't think we're going to get to that in the near future. Marking as 'backlog' until we have a plan.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

We're migrating Firefox to use L20n for localization - that's very heavy on inserting TextNodes into DOM, so this bug may affect Firefox UI performance soon.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

It would be better to file separate bugs on whatever issues you see with L20n. They may have very little to do with this one.