208727 - consider removing Declaration::mOrder

Status: RESOLVED WONTFIX

(Core :: CSS Parsing and Computation, defect)

Description

[David Baron :dbaron:]

On bug 107270 and bug 125246 there was some discussion of whether we should
bother with nsCSSDeclaration::mOrder.  The CSSOM doesn't require it, and it's
mainly just bloat.  However, Daniel has mentioned that he would like to have it
around for editing (perhaps optionally?).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Could we at least not maintain this array during DOM manipulation of style, when
it's not really meaningful anyway?  Doing that is about 1% of total time on
DHTML testcases with lots of objects being animated (for comparison, all of
reflow is about 6%).

Comment 2

[Daniel Glazman (:glazou) (NOT MONITORING BMO ANY MORE)]

Preserving style declaration orders matters in an editor, since we serialize the
STYLE attribute from the CSSOM. So if foo.style.backgroundColor="red" preserves
the order instead of putting "background-color: red" at the end, that's fine:-)

Comment 3

[Boris Zbarsky [:bzbarsky]]

I filed bug 288574 on that thought.

Comment 4

[[e-mail]]

it'd be great if we can remove this, since we create about 8000 of these things starting the browser and loading gmail. and the nsCSSDeclaration::ValueAppended() codepath gets hit about 50,000 times.

filed bug 410357 to use nsAutoTArray and avoid most of the allocations.

Comment 5

[Nicholas Nethercote [inactive]]

dbaron, bz: is mOrder still needed?

Comment 6

[David Baron :dbaron:]

I think it's definitely still needed for anyone building editing tools on top of Gecko.

Comment 7

[Nicholas Nethercote [inactive]]

Some data:  after starting Firefox and loading Gmail I get almost 9500 Declarations.  The sizes of the array storage have this distribution:

( 1)   6706 (70.8%, 70.8%): size=0
( 2)   2003 (21.2%, 92.0%): size=40
( 3)    683 ( 7.2%, 99.2%): size=72
( 4)     71 ( 0.7%,100.0%): size=136
( 5)      3 ( 0.0%,100.0%): size=56
( 6)      1 ( 0.0%,100.0%): size=88

That's 139KB for the array storage.  If we add the actual nsAutoTArrays themselves (within the Declaration object), that's probably another 16 bytes per array, which would be another 150KB or so.  Non-trivial!

Comment 8

[Daniel Glazman (:glazou) (NOT MONITORING BMO ANY MORE)]

(In reply to David Baron [:dbaron] from comment #6)

> I think it's definitely still needed for anyone building editing tools on
> top of Gecko.

Yes. The side-effects of a removal on all online editing tools relying more
and more on CSS became unpredictable. It was a bit different years ago when this
bug was filed.