ShadowNode immutability timing

[acoates-ms]

Please correct me if my understanding here is wrong:

When a rerender changes a prop on a component, the JS will call cloneNodeWithNewProps, creating a new unsealed shadownode.
Then once the render is complete completeRoot will be called. -- At this point the node is still unsealed.

In completeRoot assuming the scheduler isn't in a synchronous state, a background task will be triggered to commit the new node. Part of that commit will run layout and update the shadownode with new layout metrics. (Thus the node couldn't have been sealed yet). After layout the tree is sealed before setting it as the next root.

During the time the background layout is running, what tree is the JS running against? The node that JS got given in cloneNode is still not sealed. Would a rerender with an additional prop change during this time have to reclone the previously commited node? Or does it clone the currently unsealed node - which I would think wouldn't be thread safe at this point since the layout thread could be in the middle of modifying the node?

[andrestone]

I'm also curious about this. Aside from the layout work, there's also native side state being progressed.

By inspecting the code, it seems like it's possible that JS works on the uncommitted node.

[sammy-SC]

Hello @acoates-ms,

sorry for the slow reply, I'm on a leave and come to Github sparsely.

I'll try to answer your questions but you should know I'm not 100% familiar with JavaScript's implementation but will share my mental model, which may be incomplete.

The original design only had two threads, JavaScript thread and the main thread. We had to add additional background thread for layout to match old architecture's performance characteristics on Android. In the future we may look at removal of background thread as it would simplify the threading model.

During the time the background layout is running, what tree is the JS running against? The node that JS got given in cloneNode is still not sealed.

To clarify, Sealable -> this is just a helper class we use to detect if we try to mutate a shadow node incorrectly. Its implementation does nothing in optimised builds. This concept is not exposed to React.
During the time layout is running in background, JS is running against tree that it called completeRoot on. Once React calls completeRoot on a tree, it will make its way to the screen. React does not way until the tree is promoted and the state is represent appears on the screen.

Would a rerender with an additional prop change during this time have to reclone the previously commited node? Or does it clone the currently unsealed node - which I would think wouldn't be thread safe at this point since the layout thread could be in the middle of modifying the node?

If a prop changes, the unsealed node is cloned with new props and completeRoot has to be called. Again, React does not know whether a node is sealed or not, as far as it is concerned, the node has been committed. I haven't looked at the code but it does sound like that wouldn't be thread safe. Have you encountered any issues with it?

[acoates-ms]

I had Josh explain the thinking here offline.

Most code acts off the last committed tree, so things like calls to measure will always get the latest committed node, and those will have layout data set already. Layout data should only be read during/after commit, at which point it should be set. But the cloning can happen before that. -- So yes, during cloning the state of the layout metrics are unknown.

No, I haven't hit anything specific. At this point it's more a concern that despite the sealable code which looks like its trying to detect if mutations are happening incorrectly, the protection does not ensure that the node is sealed before additional clones are made. Really the sealable nodes would have to have two types of sealed. One that ensures that the majority of props are sealed, and its safe to clone again. Which should happen before returning from completeRoot. And a second state once layout is completed, to ensure layout doesn't get changed again.