1029157 - Consider removing sCachedComputedDOMStyle

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Andrew McCreight [:mccr8]]

sCachedComputedDOMStyle is being used to store a single dtor'ed block of memory for a nsComputedDOMStyle, and is pretty scary.  In the glorious world of jemalloc, I can't imagine it helps performance.  We should try to confirm that, and remove it, assuming it does not regress performance.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Eliminate sCachedComputedDOMStyle. Lightly tested.

I'm not sure exactly what perf testing should be done.

Comment 2

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

Not sure I trust jemalloc to be super fast, but the cache was added _long_ ago in Bug 155525, 
so many things have changed since. Like, SnowWhite kills CDS objects asynchronously.
So we may have a cached object right after SnowWhite has run, but once it has been use, the
cache won't be there until Snowwhite kills another CDS.

Comment 3

[Nicholas Nethercote [inactive]]

FWIW, I've removed several custom allocators dating back to pre-jemalloc days, and none of the removals affected performance. I'd be optimistic: remove it, land, and see if you get any regression emails :)

Comment 4

[Andrew McCreight [:mccr8]]

Do you have any thoughts on what sort of test I should run, Boris?  As Olli points out, the cache hit rate is probably not very high nowadays anyways.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, this is caching the memory, not an object?  Just kill it, and sorry you ended up having to think about this.  :(  I can't think of cases where it would be useful: in a tight loop we'd not end up with a live thing until GC anyway.

Comment 6

[Andrew McCreight [:mccr8]]

Yeah, it calls the dtor on the object, then sticks it in the global.  When you create a new one, it calls the ctor on the block.  So it is just saving the malloc/free call, not any initialization.

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: Eliminate sCachedComputedDOMStyle.

try run: https://tbpl.mozilla.org/?tree=Try&rev=ff1ece740d55

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8446025 [details] [diff] [review]
Eliminate sCachedComputedDOMStyle.

r=me

Comment 9

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/68dddeca55d2

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/68dddeca55d2