Closed Bug 1377713 Opened 5 years ago Closed 5 years ago

GetScrollPortSizeExcludingHeadersAndFooters does too much malloc/free

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: perf, Whiteboard: [qf])

Attachments

(1 file)

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 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?
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.
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.
Right, but there's no cost in adding a bit of margin here,
so that it performs well also for pages that are "unusual".
Attachment #8882846 - Flags: review?(tnikkel) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ee475ea4dfb
Use an AutoTArray in GetScrollPortSizeExcludingHeadersAndFooters to avoid malloc/free in most cases. r=tn
https://hg.mozilla.org/mozilla-central/rev/9ee475ea4dfb
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.