Compress SuspenseList code & avoid O(n^2) operations#2095
Conversation
|
This is great, however My current sketch is a WIP. and might change while doing nested SuspenseList. I'll take a deeper look as soon as I am confident that my current approach covers everything |
|
@prateekbh I updated this pull request 👍 |
| } | ||
| SuspenseList.prototype.render = function(props) { | ||
| this._next = null; | ||
| this._map = new Map(); |
There was a problem hiding this comment.
this._map =this._map || new Map();
Dont re-initialize the map here.
I haven't been able to find a way to write test for this but here the use case.
All the Suspensions are completed with revealOrder=forwards. And then one more enters on 2nd positions.
This might put completed suspensions(from third positions onwards) back in loading state(atleast earlier version of suspense did).
There was a problem hiding this comment.
Also make sure that in above use case we should not re-call the unsuspend method of children as sometimes the render is not a pure function in real world.
in my PR _isSuspensionCompleted takes care of this
|
I will try to find a way to add this test case but haven't been successful so far |
This pull request modifies the
SuspenseListimplementation in the following ways:Use an alternative strategy to store the callbacks and link them to vnodes. The original implementation used arrays, which is a very clear way to store the vnode order. This modified implementation uses a combination of a singly linked list to store the callbacks and a
Mapto map vnodes to linked list nodes. That way locating a linked list item for a vnode takes O(1) time (amortized, and this also depends on the JS engine implementation). The first element of the linked list can be consumed in O(1) time as well.It should be noted that I have not benchmarked anything, so all these changes may be inconsequential or even hurt performance in the real world.
Compress code with an alternative version of how SuspenseLists get notified that a child has been suspended, resolved and offering a way for the SuspenseList to delay the final unsuspension. It goes something like this:
_suspendedmethod - if such method exists. This is a way for a parent (e.g. SuspenseList) to get notified that one of its children/descendants suspended.unsuspendas a parameter. The resolved child descendant will not actually get unsuspended untilunsuspendgets called. (If the parent did not return a callback at point i. then the resolved vnode gets unsuspended immediately when it resolves.)SuspenseLists also act similarly to Suspense in that they also try to call the parent's
_suspendedmethod and implements the behavior in i., ii. and iii. This hierarchical mechanism to allow parent components to control the final unsuspension makes nested SuspenseLists possible without much special casing.In the end the compat.js.br file is 144 bytes smaller compared to the original implementation (2834 B vs 2690 B).
This PR should only be taken as just a collection of suggestions for optimizing the SuspenseList implementation size and worst case time compexity. @prateekbh's original implementation is great, works well and is very readable 👍