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

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect, P3)

Description

[Markus Stange [:mstange]]

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.

Comment 1

[Iain Ireland [:iain]]

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.

Comment 2

[Iain Ireland [:iain]]

I don't see any samples in runBatchedUpdates or Array.prototype.sort in the most recent profile of TodoMVC-React-Complex-DOM. I'm not sure whether this is because of Jan's work to improve sort performance, or because the code has changed between sp2 and sp3.

Marking this as WORKSFORME. If we have better examples of places where sorting still needs more work, we can track those in other bugs.