Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Synchronously Aborting an In Progress Lock #17

Closed
sebmarkbage opened this issue Sep 13, 2018 · 3 comments
Closed

Synchronously Aborting an In Progress Lock #17

sebmarkbage opened this issue Sep 13, 2018 · 3 comments

Comments

@sebmarkbage
Copy link

sebmarkbage commented Sep 13, 2018

There is a difference between this API and the mechanism that we have in React and our native frameworks.

In those frameworks we have the ability to effectively synchronously "rollback" any changes that were made during the lock. Instead we apply different changes and synchronously perform a new layout. In some variants we also synchronously wait for the layout to finish or compute a new layout for both changes synchronously.

The key is that once you have started to perform an asynchronous layout, you can opt-out of the asynchronous nature at any point. For example, if you had a higher priority interaction that you wanted to perform on that tree. The main use case for this is coordination and interop though. If something else around you changes, such as resize of the container when the content in the middle requires multi-pass layout to be performed. Or if some preexisting code around the tree needs to operate synchronously and you want to coordinate displays. E.g. if you don't want to show blank space while scrolling or something like that.

For newly inserted subtrees, this is not much of an issue because we can work around this by just synchronously inserting a new root. So for React, I was only planning on using this feature for that use case.

However, ideally we could also use it for updated trees. If we acquired a display lock for a subtree to perform an update and then something around it needs to update synchronously and this thing needs to coordinate with that tree. Then there will be visible inconsistencies that immediately pop out as highly visible.

I think there are three strategies we can try to use to address this:

  • Immutable/persistent/copy-on-write DOM trees (which is what we use in the frameworks above). I don't think this is a reasonable small step for a DOM API.
  • Rollback of DOM changes. This is a much more invasive change since it'll mean a way to define which parts are rolled back and which are not. Although it could be a powerful feature.
  • Expose a synchronous commit API that can be called any time after having acquired the lock. If called after the asynchronous commit has started, this forces the layout to be completed in a synchronous way. This can be implemented by blocking on a different thread or by recomputing the layout.

I see this mostly as an escape hatch rather than a primary usage of the API.

It may seem heavy handed to block the main thread on synchronous layout but the alternative is that we're more conservative with our use of this API which means that this and a lot more layout is synchronous anyway. (The tradeoff is that visible discrepancies or not being able to use some APIs are worse than main thread jank.)

@chrishtr
Copy link
Collaborator

I think bullet point 3 (adding a synchronous commit API) makes sense and is not hard to add.

Example:

lock.commit();
lock.forceUnlock(); // this causes the above commit to be synchronous

.. (script yields)
Promise resolution callback for lock.commit (in a microtask at end of script task containing forceUnlock)

.. wait for vsync
rAF callbacks & rendering update

For rollback, it's not feasible because Display Locking does not use a double-buffered concept of DOM and so does not keep history. In mode 2 (#12) the developer can easily rollback by simply removing
the async root node.

I also don't think immutable/copy-on-write DOM is feasible to implement right now because it's a lot of work.

@vmpstr
Copy link
Collaborator

vmpstr commented Oct 4, 2018

To align the API better with the discussion in #15 we have a few options for forcing a sync commit (modulo naming):
1.

element.acquireDisplayLock((context) => {
   ...
   context.makeSynchronous();
});
element.acquireDisplayLock((context) => {
  ...
});
...
element.forceSynchronousDisplayLock();

The difference is whether we'd want this to be a method on the element or on the context that is passed to the acquireDisplayLock callback.

Also, this raises a couple of more questions about the desired behavior of this synchronous commit:

  • What happens with possible continuations via context.schedule()? Do we keep processing them synchronously after script called for synchronous commit?
  • Should we have a separate "abort any scheduled thing that hasn't run yet"?

Without rollback, it's unclear what the desired behavior would be. We can certainly add a bunch of options to really customize the behavior, but I think targeting the common case makes sense.

@sebmarkbage do you have a sense of what would be the API that would be most elegant and useful?

@chrishtr
Copy link
Collaborator

chrishtr commented Mar 8, 2019

The latest API has a commit() method that is separate from update() and updateAndCommit(). Closing.

@chrishtr chrishtr closed this as completed Mar 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants