Closed Bug 1386443 Opened 7 years ago Closed 7 years ago

stylo: Consider using FxHash instead of FnvHash for the stylist and invalidation stuff.

Categories

(Core :: CSS Parsing and Computation, enhancement, P4)

enhancement

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

According to Glenn this made a big difference in WebRender, and probably the hottest hashmaps in servo are in the Stylist, so we should at least profile with it instead.
IRC conversation with links, fwiw (since I need to go to sleep now): 00:23 <@gw> If anyone is using Sip/Fnv hash in hot paths of Servo - you may like to try FxHash - see https://github.com/servo/webrender/pull/1538 for some numbers showing the performance wins in WR. 00:23 <crowbot> PR #1538: Switch hash table and maps to use FxHash instead of Fnv. - https://github.com/servo/webrender/pull/1538 00:26 <emilio> gw: that may also be nice for the slow rebuild stuff that bholley and bz are investigating, thanks! 00:27 <@gw> emilio: no worries, it definitely makes a big difference in WR!
Priority: -- → P4
I just tested it with the testcase from bug 1382927, but with 4k iterations, and I get a 4x+ speedup (8s vs. 1.7s), so I'll work on land it.
Assignee: nobody → emilio+bugs
> I just tested it with the testcase from bug 1382927, but with 4k iterations, and I get a 4x+ speedup I just tried applying this patch and testing on that testcase, and I see numbers pretty much the same as without this patch, fwiw.
(In reply to Boris Zbarsky [:bz] from comment #4) > > I just tested it with the testcase from bug 1382927, but with 4k iterations, and I get a 4x+ speedup > > I just tried applying this patch and testing on that testcase, and I see > numbers pretty much the same as without this patch, fwiw. So I can't repro my numbers in my personal laptop, so I'm not sure what was happening to the nightly I was comparing against (for which I got ~19/~24s consistently...) So I'd disregard that measuring. I'll try to get a profile of the nightly once I'm back at that computer.
Here's a profile of a run of the testcase that took over 22s on my usual profiling profile (with no addons other than the Gecko profiler): https://perfht.ml/2vvtTk0 However rm -rf'ing that directory makes it non reproducible again, so I'm not sure what was up with that profile at all.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #6) > Here's a profile of a run of the testcase that took over 22s on my usual > profiling profile (with no addons other than the Gecko profiler): > > https://perfht.ml/2vvtTk0 > > However rm -rf'ing that directory makes it non reproducible again, so I'm > not sure what was up with that profile at all. Huh, so I found the reason for this, and it's that I had the |layers.acceleration.force-enabled| pref set to true from profiling bug 1372665... Why that pref makes things 4 to 5x slower, I don't know...
That's pretty bizarre. I would not expect that pref to affect this testcase at all... :(
(In reply to Boris Zbarsky [:bz] (vacation Aug 14-27) from comment #8) > That's pretty bizarre. I would not expect that pref to affect this testcase > at all... :( FWIW, since I pasted it on IRC but not here: https://crisal.io/tmp/layers-accel-force-enabled.ogv I changed the hasher to use the precomputed hash anyway. The only FnvHashMaps remaining are the stylesheet invalidation stuff, the namespaces hashmaps, and the media query invalidation stuff. Since none of them are that hot, and we don't have that much strong evidence to justify this, I'm closing this as incomplete. We can measure it if we think it's worth again later.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
(In reply to Emilio Cobos Álvarez [:emilio] from comment #9) > FWIW, since I pasted it on IRC but not here: > https://crisal.io/tmp/layers-accel-force-enabled.ogv My results of layers.acceleration.force-enabled with "true", "true with webrender+webrendest+layersfree+blob-image" and "false" are all between 2370 and 2440. Nightly 57 x64 20170824100243 @ Debian Testing (KDE Plasma/Xorg) (Unrelated, but amazing: bholley's benchmark in https://news.ycombinator.com/item?id=15076968 Macbook Pro: Stylo_parallel=48.35ms, Stylo_sequential=156.72ms, Gecko=575.29ms, Chrome_Canary=196.53ms, Safari_11_Beta=223.50ms vs. My Octacore Debian: Stylo_parallel=74.50ms, Stylo_sequential=254.17ms, Gecko=293.27ms, Chrome_Dev=257.73ms)
You need to log in before you can comment on or make changes to this bug.