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)
Core
CSS Parsing and Computation
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
Updated•7 years ago
|
Priority: -- → P1
Comment 1•7 years ago
|
||
This might be fixed by bug 1386602. Should reprofile when it lands.
Depends on: 1386602
Comment 3•7 years ago
|
||
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.
Comment 4•7 years ago
|
||
(setting P3, since we don't need to track this separately from bug 1386045 for now).
Reporter | ||
Comment 5•7 years ago
|
||
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.
Comment 6•7 years ago
|
||
Can you share a profile? I suspect this is the same thing I'm working on in bug 1386045.
Flags: needinfo?(cyu)
Reporter | ||
Comment 7•7 years ago
|
||
(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)
Comment 8•7 years ago
|
||
(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.
Comment 9•7 years ago
|
||
(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.
Comment 10•7 years ago
|
||
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.
Comment 11•7 years ago
|
||
(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.
Comment 12•7 years ago
|
||
Stylist flushing should be pretty zippy these days. Cervantes, can you remeasure?
Flags: needinfo?(cyu)
Reporter | ||
Comment 13•7 years ago
|
||
(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
Comment 15•7 years ago
|
||
(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.
Description
•