Closed
Bug 1362982
Opened 8 years ago
Closed 7 years ago
stylo: Stylo 5%-10% slower than Gecko for setting innerHTML
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: jseward, Unassigned)
References
Details
Using the testcase at https://bugzilla.mozilla.org/show_bug.cgi?id=1347525#c13
(letting it run through and reporting the best value (milliseconds))
for ("gecko") m-c, opt, non-debug, non-Stylo, -O2 vs
("stylo") m-c, opt, non-debug, Stylo, -O2, STYLO_THREADS=1
on Linux x86_64, Fedora 25, Intel(R) Xeon(R) CPU E3-1505M v5, 2.6 GHz.
(4 cores, 8 threads, 8 MB L3)
Results:
gecko: 55 53
stylo: 57 59
It looks as if Stylo is 5%-10% slower than Gecko for this test.
Mozconfigs and command line are below. It's unclear to me what the effect on
Rust optimisation is of setting --enable-optimize="-g -O2" in the Stylo case.
For Stylo, the Clang version was 3.9. gcc is 6.3.1 and rustc is 1.16.0.
-- command line ----------------------------------
STYLO_THREADS=1 DISPLAY=:2.0 ./gcc-O2-nondebug-stylo/dist/bin/firefox-bin -P dev -no-remote file:///home/sewardj/MOZ/STYLO/bug1347525testcase_looping2.html
-- mozconfig gecko -------------------------------
. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/gcc-O2-nondebug-plain
ac_add_options --enable-tests
ac_add_options --enable-optimize="-g -O2"
ac_add_options --enable-debug-symbols
ac_add_options --enable-profiling
ac_add_options --disable-crashreporter
mk_add_options MOZ_MAKE_FLAGS="-j8"
mk_add_options AUTOCLOBBER=1
-- mozconfig stylo -------------------------------
. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/gcc-O2-nondebug-stylo
ac_add_options --enable-tests
ac_add_options --enable-optimize="-g -O2"
ac_add_options --enable-debug-symbols
ac_add_options --enable-stylo
ac_add_options --enable-valgrind
ac_add_options --enable-profiling
ac_add_options --enable-elf-hack
ac_add_options --disable-crashreporter
mk_add_options MOZ_MAKE_FLAGS="-j8"
mk_add_options AUTOCLOBBER=1
Comment 1•8 years ago
|
||
I'm not getting as high regression, but there is some.
In stylo enabled build (layout.css.servo.enabled == true), nsNodeUtils::ContentAppended becomes a bit slower, mostly because nsPresShell::ContentAppended calls nsCSSFrameConstructor::ContentAppended which then calls nsCSSFrameConstructor::LazilyStyleNewChildRange. LazilyStyleNewChildRange calls are done only when isNewlyAddedContentForServo or some such is true.
This stuff just isn't there at all when layout.css.servo.enabled == false or non-gecko build.
Still looking these profiles a bit more.
Comment 2•8 years ago
|
||
On average the numbers seem to be higher (slower) with just stylo build, even with layout.css.servo.enabled == false. Still investigating.
Comment 3•8 years ago
|
||
I think this should be tested on non-linux platforms too. I don't right now have access to such machines.
Comment 4•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #2)
> On average the numbers seem to be higher (slower) with just stylo build,
> even with layout.css.servo.enabled == false. Still investigating.
If you find a perf regression that reproduces with stylo built but disabled, that's much more urgent - please file separately and I'll get someone to jump on it.
Summary: Stylo 5%-10% slower than Gecko for setting innerHTML → stylo: Stylo 5%-10% slower than Gecko for setting innerHTML
Comment 5•8 years ago
|
||
So far I haven't found anything concrete in the profiles. It may hint that use of memory caches is worse because of larger Element objects, or it may not.
Comment 6•8 years ago
|
||
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #4)
> (In reply to Olli Pettay [:smaug] from comment #2)
> > On average the numbers seem to be higher (slower) with just stylo build,
> > even with layout.css.servo.enabled == false. Still investigating.
>
> If you find a perf regression that reproduces with stylo built but disabled,
> that's much more urgent - please file separately and I'll get someone to
> jump on it.
That seems like something that the Stylo team should be investigating actively as part of investigating the performance implications of enabling Stylo. We'll file bugs if we notice something, but please don't assume someone else is doing this investigation. :-0
Comment 7•8 years ago
|
||
(In reply to :Ehsan Akhgari (super long backlog, slow to respond) from comment #6)
> (In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #4)
> > (In reply to Olli Pettay [:smaug] from comment #2)
> > > On average the numbers seem to be higher (slower) with just stylo build,
> > > even with layout.css.servo.enabled == false. Still investigating.
> >
> > If you find a perf regression that reproduces with stylo built but disabled,
> > that's much more urgent - please file separately and I'll get someone to
> > jump on it.
>
> That seems like something that the Stylo team should be investigating
> actively as part of investigating the performance implications of enabling
> Stylo. We'll file bugs if we notice something, but please don't assume
> someone else is doing this investigation. :-0
The point here is that smaug was reporting results that I would not expect, and don't plan to spend much time chasing unless we have STR (or at least a definite claim that there is actually a slowdown) or a regression on Talos. Building stylo by default (but preffing it off) is no different than any run-of-the-mill platform patch - it adds some fields and branches here and there, but nothing that I would expect to alter our basic performance characteristics. So while I'd certainly be very interested to hear about issues and would follow up, the risk profile of this change is just the same as landing any other platform feature preffed off.
Comment 8•8 years ago
|
||
I wouldn't be at all surprised if just adding 8 bytes to sizeof Element would regress performance.
That is a major change to the core of DOM after all.
Comment 9•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #8)
> I wouldn't be at all surprised if just adding 8 bytes to sizeof Element
> would regress performance.
> That is a major change to the core of DOM after all.
If we're concerned about this, I can rearrange some things to avoid growing Element at least on 64-bit. I'll file a bug.
Comment 10•8 years ago
|
||
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #9)
> (In reply to Olli Pettay [:smaug] from comment #8)
> > I wouldn't be at all surprised if just adding 8 bytes to sizeof Element
> > would regress performance.
> > That is a major change to the core of DOM after all.
>
> If we're concerned about this, I can rearrange some things to avoid growing
> Element at least on 64-bit. I'll file a bug.
Filed bug 1363375 about this.
Updated•7 years ago
|
Blocks: stylo-nightly
Comment 11•7 years ago
|
||
smaug, now that bug 1363375 landed, are you still seeing a regression from simply building stylo?
Flags: needinfo?(bugs)
Comment 12•7 years ago
|
||
I assume https://bugzilla.mozilla.org/show_bug.cgi?id=1362982#c1 hasn't disappeared, but let me re-profile
Comment 13•7 years ago
|
||
Times seem to vary quite a lot, but https://bug934607.bmoattachments.org/attachment.cgi?id=8872948 definitely does shows a regression, and comment 1 is still very valid.
Flags: needinfo?(bugs)
Comment 14•7 years ago
|
||
Ok - but the build-time regression has gone away?
Comment 15•7 years ago
|
||
The noise is quite bad, but at least servo build with the pref disable is never faster than non-servo build. I'd say in general it seems to be a bit slower. But worth to try on non-linux too.
Comment 16•7 years ago
|
||
I modified the testcase in bug 1347525 to track averages, and measured a vanilla local m-c build against a stylo build (with stylo enabled). This is on new OSX MBP. Measurements are:
With stylo:
* avg: 54.4, best: 40.95
* avg: 53.8, best: 42.8
* avg: 54.2, best: 46.6
Without stylo:
* avg: 54.3, best: 46.8
* avg: 53.3, best: 42.3
* avg: 52.7, best: 46
* avg: 54.1, best: 48.1
That does not look to me like an indicator that we should be spending time on this.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•