Closed Bug 1386580 Opened 7 years ago Closed 7 years ago

stylo: StyleSetHandle::AppendFontFaceRules() is much slower than Gecko's implementation

Categories

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

defect

Tracking

()

RESOLVED DUPLICATE of bug 1386045

People

(Reporter: cyu, Unassigned)

References

Details

This is found when I wan testing bug 1366960 with and without stylo enabled. Without stylo, AppendFontFaceRules() takes 30 ms: https://perfht.ml/2w5q41Z With stylo, it takes 89 ms: https://perfht.ml/2w5ZjdY
Priority: -- → P1
This might be fixed by bug 1386602. Should reprofile when it lands.
Depends on: 1386602
This landed. Have a chance to look at it again?
Flags: needinfo?(cyu)
Actually, I think we should probably wait until bug 1386045 is resolved, since emilio and cameron are doing lots of optimizations of stylist flushes, which is the actual work happening under AppendFontFaceRules.
(setting P3, since we don't need to track this separately from bug 1386045 for now).
Depends on: 1386045
Flags: needinfo?(cyu)
Priority: P1 → P3
Blocks: 1366960
For the record, I reprofiled with today's build, stylo's implementation of nsIDocument::FlushUserFontSet() is still a lot slower than gecko (64 ms vs 25 ms) on my workstation.
Can you share a profile? I suspect this is the same thing I'm working on in bug 1386045.
Flags: needinfo?(cyu)
(In reply to Emilio Cobos Álvarez [:emilio] from comment #6) > Can you share a profile? I suspect this is the same thing I'm working on in > bug 1386045. Gecko profile with stylo on: https://perfht.ml/2vCX9UV (55 ms) In comparison, Gecko profile without stylo: https://perfht.ml/2vCXwie (22 ms)
Flags: needinfo?(cyu)
(In reply to Cervantes Yu [:cyu] [:cervantes] from comment #7) > (In reply to Emilio Cobos Álvarez [:emilio] from comment #6) > > Can you share a profile? I suspect this is the same thing I'm working on in > > bug 1386045. > > Gecko profile with stylo on: https://perfht.ml/2vCX9UV (55 ms) > In comparison, Gecko profile without stylo: https://perfht.ml/2vCXwie (22 ms) Thanks! Yeah, so FlushUserFontSet is 28ms vs 16ms, and those extra 13ms are the ones we're building the invalidation map with. My work for bug 1386045 should help here as well.
(In reply to Cervantes Yu [:cyu] [:cervantes] from comment #7) > (In reply to Emilio Cobos Álvarez [:emilio] from comment #6) > > Can you share a profile? I suspect this is the same thing I'm working on in > > bug 1386045. > > Gecko profile with stylo on: https://perfht.ml/2vCX9UV (55 ms) > In comparison, Gecko profile without stylo: https://perfht.ml/2vCXwie (22 ms) Oh, also, note that a bit of time gets spent in style::invalidation::stylesheets::StylesheetInvalidationSet::process_invalidations_in_subtree<style::gecko::wrapper::GeckoElement>. That's time that Gecko doesn't spend, but helps stylo afterwards (it basically scans the DOM looking for elements affected by the stylesheets that have been added, to avoid restyling the whole document afterwards). So even though that makes this function slightly slower, it saves time overall.
So all the time here is actually in UpdateStylistIfNeeded, which means we are just slow on updating stylist, and FlushUserFontSet happens to be the first one who tries to update stylist.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #10) > So all the time here is actually in UpdateStylistIfNeeded, which means we > are just slow on updating stylist, and FlushUserFontSet happens to be the > first one who tries to update stylist. Yeah. This is basically just something to remeasure when all emilio's stylist fixups land.
Stylist flushing should be pretty zippy these days. Cervantes, can you remeasure?
Flags: needinfo?(cyu)
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #12) > Stylist flushing should be pretty zippy these days. Cervantes, can you > remeasure? Stylo is now much faster than Gecko in this case. I redo the test on nightly 2017-09-07 build. For the time spent in nsIDocument::FlushUserFontSet(): Stylo spent 1 to 4 ms in several tests. In comparison, Gecko spent 15 to 33 ms. I think we can close this bug as FIXED. \o/
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(cyu)
Resolution: --- → FIXED
Hoorray :)
Resolution: FIXED → DUPLICATE
(In reply to Cervantes Yu [:cyu] [:cervantes] from comment #13) > Stylo spent 1 to 4 ms in several tests. > In comparison, Gecko spent 15 to 33 ms. ROCKS! ;-)
You need to log in before you can comment on or make changes to this bug.