608759 - Stop calling ContentEnumFunc through a function pointer

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

EnumerateAllRules takes a function pointer, but it's always to the same function.

At this point I've got a patch to fix that in two different trees (independently written each time as part of other work).

Once we ship fx2 and I have time to remerge stuff to this change, I'll just do it.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I think if you actually want the compiler to inline ContentEnumFunc, you should put it before its first use.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 510050 [details] [diff] [review]
Fix

So two things.  First, that's doable but a bit of a hassle.  In particular, ContentEnumFunc needs to come after the class decls of both TreeMatchContext and NodeMatchContext, and needs to know about the signatures of SelectorMatches and SelectorMatchesTree.  I could move the former and forward-declare the latter, I guess...  The other option is to move the ContentEnumFunc consumers down lower in the file.  They're class methods, so that's safe enough.  It's a bunch of code movement, though.  Let me know if you prefer one of those options?

Second, at some point I verified that at least gcc on Mac did inline it, iirc.  And I'm not quite sure how much I care about the inlining...

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 510050 [details] [diff] [review]
Fix

r=dbaron

Comment 5

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/projects/birch/rev/feee7408eacd

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

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