Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Schedule supports two priorities #12707

Closed

Commits on Apr 24, 2018

  1. Add support for multiple callbacks in ReactScheduler

    **what is the change?:**
    We want to support calling ReactScheduler multiple times with different
    callbacks, even if the initial callback hasn't been called yet.
    
    There are two possible ways ReactScheduler can handle multiple
    callbacks, and in one case we know that callbackA depends on callbackB
    having been called already. For example;
    callbackA -> updates SelectionState in a textArea
    callbackB -> processes the deletion of the text currently selected.
    
    We want to ensure that callbackA happens before callbackB. For now we
    will flush callbackB as soon as callbackA is added.
    
    In the next commit we'll split this into two methods, which support two
    different behaviors here. We will support the usual behavior, which
    would defer both callbackA and callbackB.
    
    One issue with this is that we now create a new object to pass to the
    callback for every use of the scheduler, while before we reused the same
    object and mutated the 'didExpire' before passing it to each new
    callback. With multiple callbacks, I think this leads to a risk of
    mutating the object which is being used by multiple callbacks.
    
    **why make this change?:**
    We want to use this scheduling helper to coordinate between React and
    non-React scripts.
    
    **test plan:**
    Added and ran a unit test.
    flarnie committed Apr 24, 2018
    Configuration menu
    Copy the full SHA
    9488108 View commit details
    Browse the repository at this point in the history

Commits on Apr 25, 2018

  1. Configuration menu
    Copy the full SHA
    5364c9a View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    9a1155d View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    48791d1 View commit details
    Browse the repository at this point in the history

Commits on Apr 27, 2018

  1. Fix case of scheduled callbacks which schedule other callbacks

    **what is the change?:**
    We added support for serial scheduled callbacks to schedule more
    callbacks, and maintained the order of first-in first-called.
    
    **why make this change?:**
    This is sort of a corner case, but it's totally possible and we do
    something similar in React.
    
    We wouldn't do this with serial callbacks, but with deferred callbacks
    we do a pattern like this:
    ```
       +                                             +
       |   +--------------------+ +----------------+ | +--------------------------------+ +-------------------------+
       |   |                    | |                | | |                                | |                         |
       |   | main thread blocked| |callback A runs | | | main thread blocked again      | |callback A runs again, finishes
       |   +--------------------+ +-----+----------+ | +--------------------------------+ ++------------------------+
       v                                ^            v                                     ^
    schedule +------------------------->+            no time left!+----------------------->+
    callbackA                                        reschedule   |
                                                     callbackA    +
                                                     to do more
                                                     work later.
    
    ```
    
    **test plan:**
    Wrote some fun new tests and ran them~
    Also ran existing React unit tests. As of this PR they are still
    directly using this module.
    
    Rename ReactScheduler.rIC -> scheduleSerialCallback
    
    **what is the change?:**
    We renamed this method.
    
    **why make this change?:**
    1) This is no longer just a polyfill for requestIdleCallback. It's
       becoming something of it's own.
    2) We are about to introduce a second variant of behavior for handling
       scheduling of callbacks, so want to have names which make sense for
       differentiating the two priorities.
    
    **test plan:**
    Ran tests
    flarnie committed Apr 27, 2018
    Configuration menu
    Copy the full SHA
    4d68007 View commit details
    Browse the repository at this point in the history
  2. Rename cIC -> cancelSerialCallback and add tests for this method

    **what is the change?:**
    Rename a method and add tests
    
    **why make this change?:**
    This will set things up for adding a new method which cancels callbacks
    which have 'deferred' priority.
    
    **test plan:**
    Run the updated tests.
    flarnie committed Apr 27, 2018
    Configuration menu
    Copy the full SHA
    c0f4f18 View commit details
    Browse the repository at this point in the history
  3. Wrap 'scheduleSerialCallback' test in a 'describe' block

    **what is the change?:**
    see title
    
    **why make this change?:**
    For more readable and organized test and test output.
    I put this in it's own commit because the diff looks gnarly, but we're
    just moving things around. No actual changes.
    
    **test plan:**
    Run the tests
    flarnie committed Apr 27, 2018
    Configuration menu
    Copy the full SHA
    b0a966e View commit details
    Browse the repository at this point in the history
  4. RFC: Add methods for scheduling

    **what is the change?:**
    Add support for multiple priorities.
    
    - We are storing pending deferred callbacks in an array for simplicity,
    which means that we have to do a linear search of the array repeatedly
    to find callbacks which are past their timeout.
    It would be more efficient to use a min heap to store them and then
    quickly find the expired ones, but I'm not sure it's worth the extra
    complexity unless we know that we'll be dealing with a high number of
    pending deferred callbacks at once.
    
    If there are a high number of pending deferred callbacks, we have other
    problems to consider.
    
    Also the implementation of cancel is missing and I haven't added support
    for returning ids which can be used to cancel. Not sure we need this
    yet, but will have to add it eventually.
    
    **why make this change?:**
    To get feedback on this approach as early as possible.
    
    **test plan:**
    Tests are a challenge.
    
    To write a unit test I'll need to add support for injecting some of the
    browser APIs as dependencies, and basically allow us to create a
    noopScheduler.
    
    We also should have a real browser fixture to test this. Going to work
    on that in another branch.
    
    **issue:**
    Internal task T28128480
    flarnie committed Apr 27, 2018
    Configuration menu
    Copy the full SHA
    a13a126 View commit details
    Browse the repository at this point in the history