Closed Bug 1367711 Opened 3 years ago Closed 3 years ago

Fast path(s) for reflowing placeholder frames?

Categories

(Core :: Layout: Positioned, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mats, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(1 file)

nsPlaceholderFrame::Reflow is pretty much a NOP:
http://searchfox.org/mozilla-central/rev/35b37316149108d53a02fb8498e250ea8a22ab5d/layout/generic/nsPlaceholderFrame.cpp#108
I wonder if it would be possible to "fast path" that in some ways, 
e.g. cheating on setting up its reflow state (ReflowInput).
Bug 1359341 has a testcase (attachment 8869972 [details]) that generates a lot of placeholder reflows.
This sounded like a sufficiently worthwhile idea that I thought I'd try an experiment to see how much we could gain for minimal effort. This patch seems to pass all tests on tryserver, and on my local opt build it reduces the average time reported by the testcase from 0.735s to 0.634s.
Attachment #8872083 - Flags: review?(mats)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8872083 [details] [diff] [review]
Bail out early when initializing ReflowInput for a placeholder frame, which has a trivial Reflow method

I suggest adding a comment in nsPlaceholderFrame::Reflow about this.

It would good if dbaron also has time to take a look at this...
Attachment #8872083 - Flags: review?(mats)
Attachment #8872083 - Flags: review+
Attachment #8872083 - Flags: feedback?(dbaron)
Comment on attachment 8872083 [details] [diff] [review]
Bail out early when initializing ReflowInput for a placeholder frame, which has a trivial Reflow method

sounds fine to me, although it would perhaps be good to investigate making the rest of the work faster...
Attachment #8872083 - Flags: feedback?(dbaron) → feedback+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4ac809a2e76161c335b9a8dbac2a4a243f33952
Bug 1367711 - Bail out early when initializing ReflowInput for a placeholder frame, which has a trivial Reflow method. r=mats
https://hg.mozilla.org/mozilla-central/rev/e4ac809a2e76
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.