Pres shell arena allocation methods should be inlined and non-virtual

RESOLVED FIXED in mozilla15

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

({perf})

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
We should inline AllocateFrame etc, at least under _IMPL_NS_LAYOUT 
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsIPresShell.h#226
(Assignee)

Comment 1

5 years ago
Created attachment 620465 [details] [diff] [review]
fix

Do we need to expose non-_IMPL_NS_LAYOUT versions at all?

https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=67684568dbcf
Attachment #620465 - Flags: review?(bzbarsky)
I don't think we do.
(Assignee)

Comment 3

5 years ago
Created attachment 620524 [details] [diff] [review]
fix, v2

OK, great.  So, I tried to wrap these in #ifdef _IMPL_NS_LAYOUT but it
didn't work out... I had to wrap nsPresContext::AllocateFromShell too
and that led to compile errors because they are used in inline methods
in nsStyleStruct.h, which gets pulled in from parser/html and layout/ipc
etc (through nsIFrame.h, nsDisplayList.h), so I don't really see a good
solution to solve these dependencies... the cost seems to outweigh the
benefit of the #ifdef

https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=4512f68bc191
Attachment #620465 - Attachment is obsolete: true
Attachment #620465 - Flags: review?(bzbarsky)
Attachment #620524 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

5 years ago
bz, review ping?
Comment on attachment 620524 [details] [diff] [review]
fix, v2

Hmm.  The review request mail went AWOL...

r=me
Attachment #620524 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d682dd9d93ea
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/d682dd9d93ea
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.