1377713 - GetScrollPortSizeExcludingHeadersAndFooters does too much malloc/free

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Mats Palmgren (inactive)]

I'm seeing malloc/free churn from this (reversed) stack:

...
mozilla::ScrollFrameHelper::ComputeScrollMetadata(mozilla::layers::Layer*, nsIFrame*, mozilla::ContainerLayerParameters const&, mozilla::DisplayItemClip const*) const
GetScrollFrameFromContent(nsIContent*)
nsHTMLScrollFrame::GetPageScrollAmount() const
mozilla::ScrollFrameHelper::GetPageScrollAmount() const
void nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::ShiftData<nsTArrayInfallibleAllocator>(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long)
free

Comment 1

[Mats Palmgren (inactive)]

Attachment: Use an AutoTArray in GetScrollPortSizeExcludingHeadersAndFooters to avoid malloc/free in most cases

Comment 2

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8882846 [details] [diff] [review]
Use an AutoTArray in GetScrollPortSizeExcludingHeadersAndFooters to avoid malloc/free in most cases

50 seems really high for this. Where does that come from?

Comment 3

[Mats Palmgren (inactive)]

50 * 8 is just 400 bytes (plus a few for the array header I guess),
but that doesn't seem high to me for a temporary stack allocation.
It's a non-nested helper function AFAICT, so there's no risk of
having a deep stack of nested calls here.

Comment 4

[Timothy Nikkel (:tnikkel)]

I was thinking of it more from the perspective of how many fixed frames we expect to see on pages. I think it's usually in the single digits.

Comment 5

[Mats Palmgren (inactive)]

Right, but there's no cost in adding a bit of margin here,
so that it performs well also for pages that are "unusual".

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ee475ea4dfb
Use an AutoTArray in GetScrollPortSizeExcludingHeadersAndFooters to avoid malloc/free in most cases. r=tn

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9ee475ea4dfb