Open Bug 1821730 Opened 2 years ago Updated 2 years ago

Sorting is slower in runBatchedUpdates in React-TodoMVC than in V8

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

People

(Reporter: mstange, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [sp3])

In the profiles below, we spend 5 times as much time calling Array.prototype.sort during runBatchedUpdates compared to V8.

There aren't a ton of samples in these profiles unfortunately, but you can see how Spidermonkey is gradually tiering up self-hosted sorting JS code whereas V8 just calls a precompiled Builtin.

Spidermonkey: https://share.firefox.dev/3mJyvyT
V8: https://share.firefox.dev/3ZDgjWa

To reproduce, run index.js in https://github.com/mozilla/Speedometer/tree/57830930398f53986e931b45bbf1340d890eb882/resources/todomvc/architecture-examples/react in the shell.

Note that we have a C++ implementation of sort, but we can't use it here because it doesn't support user-defined comparators unless they are exactly function(x,y) { return x - y; }.

Copying in some relevant discussion from the matrix channel:

iain: Do you know what happens to the relative performance if you increase the number of iterations? The maximum number of ticks we can lose to blinterp/baseline performance on selfhosted code is bounded, because eventually we'll tier up to Ion; if sorting isn't hot enough to tier up, it's also maybe not hot enough to matter
mstange: I do not know
mstange: but I fear that much of the remaining difference on this benchmark has similar characteristics: once we've tiered up to Ion we're competitive, but during the tiering up is where we lose
iain: Interesting
mstange: I haven't verified this but it's possible that we're worse on all the functions which don't run much
mstange: well, it requires a bit more investigation
mstange: for now I'd say there are lower hanging fruit than the sorting

This might be another piece of evidence in favour of overhauling how we implement builtins / selfhosted code, but I don't think it's enough to justify doing it right now.

Severity: -- → S3
Priority: -- → P3
Whiteboard: [sp3]
You need to log in before you can comment on or make changes to this bug.