1360881 - stylo: We allocate too much during the cascade

Status: RESOLVED WORKSFORME

(Core :: CSS Parsing and Computation, enhancement, P4)

Description

[Bobby Holley (:bholley)]

In the profile in bug 1360878 comment 1, we spend 11ms in malloc/free, which is too much. There are a couple of sources for this, and I'll file dependent bugs.

Note that this is about sequential performance. In bug 1291355 Julian is investigating contention issues that (we think) makes malloc blow up when parallelized. So any malloc reduction here will dovetail with whatever we find out there.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

We should also refcount inherited-by-default types.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I think before considering implementing our own Arc, we should fix the dumb stuff we do. I've opened https://github.com/servo/servo/pull/16664 to do that, which should make the comparison actually useful.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Bobby, how does bug 1361392 block this? That's text shaping code, so shouldn't affect styling, only layout.

Comment 4

[Bobby Holley (:bholley)]

Oh, I just saw that it was allocation-related and blocked bug 1360878. So I guess the real question is why it blocks bug 1360878...

Comment 5

[Julian Seward [:jseward]]

I was unclear whether this was part of styling or not; evidently not.
So the blocking of bug 1360878 is wrong, IIUC.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Yeah, I suggested Julian to report the memory inefficiencies he found while investigating that test case blocking bug 1360878, and triage them later. My fault, should've removed that dependency. But the report was quite useful anyway :)

Comment 7

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57

Comment 8

[Bobby Holley (:bholley)]

We fixed a lot of stuff in this area, and things look pretty good here at this point.