1364813 - Devirtualize nsIFrame::IsFrameOfType()

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Mats Palmgren (inactive)]

It *might* be a good idea to flatten this hierarchy and store the flags
for each concrete frame class in an array indexed by mClass.
I haven't looked at the implementation details yet though...

Comment 1

[(no longer active)]

IsFrameOfType shows up as 3% of bug 1376536...  Perhaps this is worth doing.

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to :Ehsan Akhgari from comment #1)
> IsFrameOfType shows up as 3% of bug 1376536...  Perhaps this is worth doing.

I don't think this is connected to bug 1376536 anymore - in a profile I captured there yesterday, IsFrameOfType only shows up as 1ms (https://perfht.ml/2zEBRHk) vs. a total of 213ms of samples that have "reflow" in the backtrace ( https://perfht.ml/2zDnwdW )  So: it's less than 0.5% of our reflow time, and doesn't seem significant enough that optimizing it away would make any sort of dent in that bug.

 --> Removing the dependency, in part because that bug's about to be fixed by some work done in a different helper.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

This is probably worth doing, and other engines seem to report wins in speedometer while doing the equivalent work.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I still need to do some debugging for some orange, but preliminary results seem positive. https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=b3295bdae3baea3902c80443a9904019c8e7e630&originalSignature=4569403&newSignature=4569403&framework=13&application=firefox&originalRevision=0052c4c8d9789acd5dddbafbeb2596ecd78b8b33&page=1

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1364813 - Remove IsFrameOfType, use non-virtual checks. r=jwatt

Extend the per-frame-class bit we have to devirtualize IsLeaf to also
devirtualize IsFrameOfType.

This was done by going through all the frame classes, trying to preserve
behavior.

The only quirky thing is that I had to add two more trivial frame
classes, nsAudioFrame for audio elements, and
nsFloatingFirstLetterFrame. That's because these frame classes were
returning different answers at runtime, but they do this only on
conditions that trigger frame reconstruction (floating, and being an
audio element, respectively).

Comment 7

[Jonathan Watt [:jwatt]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

This is probably worth doing, and other engines seem to report wins in speedometer while doing the equivalent work.

Thanks for doing this. Fingers crossed!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef11a45be803
Remove IsFrameOfType, use non-virtual checks. r=jwatt

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ef11a45be803