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

Refactor progressed work model #9695

Closed
wants to merge 62 commits into from
Closed

Commits on Jun 6, 2017

  1. Configuration menu
    Copy the full SHA
    624d5b5 View commit details
    Browse the repository at this point in the history
  2. Hidden children

    Make sure we resume on progressed hidden children. I split these out
    into their own functions so that it doesn't distract from the normal,
    non-hidden code path. Will also make it easier to reuse later if/when
    we apply the offscreen children feature to other types of components.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    21eb29f View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    83ff51c View commit details
    Browse the repository at this point in the history
  4. Re-implement begin phase for classes

    There was some unnecessary abstraction in ReactFiberClassComponent that
    was making it difficult to maintain and, ironically, causing some
    duplication. I've moved most of that module into the begin phase,
    and consolidated the separate mount, resume mount, and update functions
    into a single function that's easier to reason about.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    3c4718f View commit details
    Browse the repository at this point in the history
  5. Better naming for resuming/resetting work

    A resume is where we continue working on an already existing work-in-
    progress. A reset is where we create a new work-in-progress by cloning
    values from current.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    c45954d View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    020e7a2 View commit details
    Browse the repository at this point in the history
  7. Before processing an update queue, clone from current if necessary

    It's fine for the work-in-progress and current fiber to have the same
    update queue, until we begin work on that fiber. At that point, we need
    to clone the current queue to create a work-in-progress queue. Works
    pretty much just like fibers.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    b14433a View commit details
    Browse the repository at this point in the history
  8. Fix arguments passed to componentWillUpdate

    I'd accidentally reversed which ones were passed to the method and
    which ones were set on the instance. D'oh.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    c05987c View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    17fa02e View commit details
    Browse the repository at this point in the history
  10. Avoid "previous current" work-in-progress scenario

    It's possible for a work-in-progress fiber to be the most progressed
    work (last fiber whose children were reconciled) but be older than
    the current fiber. This scenario, where the work-in-progress fiber
    is really the "previous current," happens after a bailout. To avoid,
    in addition to keeping track of the most progressed fiber, we also
    keep track of the most recent fiber to enter the begin phase,
    regardless of whether it bailed out. Then we only resume on the work-
    in-progress if its the most recent fiber.
    
    It'd be nice to come up with a heuristic here that didn't require an
    additional fiber field.
    
    Also, the naming of "progressedWork" versus "newestWork" is not great.
    Maybe "lastUpdatedWork" and "lastUpdatedOrBailedOutWork"? We can
    bikeshed later.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    7d64b2e View commit details
    Browse the repository at this point in the history
  11. Pass pending props as an argument to createWorkInProgress

    Helps ensure we don't accidentally reuse props from the current tree,
    or from an invalid work-in-progress ("previous current").
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    75e8c04 View commit details
    Browse the repository at this point in the history
  12. Fix wrong priority being reset in bailoutOnHiddenChildren

    The pendingWorkPriority should be reset, not the progressedPriority.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    0a62806 View commit details
    Browse the repository at this point in the history
  13. Don't bubble work priority from the current tree

    Only bubble work priority from the work-in-progress tree. Otherwise you
    could fall into an infinite loop because the current tree's priority
    is never reset.
    
    However, update priority should always be bubbled up from the children,
    even after a bailout, because there might be low priority updates.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    b7d7ad7 View commit details
    Browse the repository at this point in the history
  14. Don't stash work-in-progress as progressed work if it's no longer valid

    When resetting a work-in-progress to current, if the progressed priority
    is the render priority, then the progressed work must be invalid.
    Otherwise we would have resumed instead of resetting. So don't stash it,
    just throw it out.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    c400969 View commit details
    Browse the repository at this point in the history
  15. Allocate a single progressed work object per fiber pair

    There's only ever one progressed work object per fiber pair. Reuse the
    same object every time instead of allocating. The first time a high-
    priority update interrupts a low one, we'll still have to allocate a
    ProgressedWork object per fiber. But not for subsequent interruptions.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    c0f6cf9 View commit details
    Browse the repository at this point in the history
  16. Configuration menu
    Copy the full SHA
    62b64c7 View commit details
    Browse the repository at this point in the history
  17. Don't schedule an update effect when shouldComponentUpdate returns false

    ...unless there was already an update scheduled. Need to compare the
    memoized props to current, not the next props.
    
    TODO: this will fail if we bailout with shouldComponentUpdate twice
    before committing, because the props and state are updated after the
    first bailout. Only observable in async mode. Need to find a better
    heuristic. We can't read from the effectTag because it may have been
    reset during reconciliation.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    9e3768a View commit details
    Browse the repository at this point in the history
  18. Deprioritize setState inside hidden subtrees

    Don't bubble up priority inside deprioritized trees.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    b0dc149 View commit details
    Browse the repository at this point in the history
  19. Set current owner before calling render method

    Also, throw an error if an element with a string ref does not have an
    error. Fiber was throwing, but not until the commit phase, and with
    a bad error message.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    a191962 View commit details
    Browse the repository at this point in the history
  20. Begin failed work at the same priority that the error was thrown

    Mistake caused by poor naming. Renamed priorityLevel -> minPriorityLevel
    to avoid confusion with nextPriorityLevel.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    299ff8f View commit details
    Browse the repository at this point in the history
  21. Configuration menu
    Copy the full SHA
    19a33f3 View commit details
    Browse the repository at this point in the history
  22. Make the fiber root an effect list

    This lets us reuse the logic in transferEffectsToParent. Requires adding
    two fields to the FiberRoot type, but I figure those are
    fairly inexpensive. Otherwise I'll revert and duplicate that
    code, instead.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    6c5af09 View commit details
    Browse the repository at this point in the history
  23. Check if object is extensible instead of comparing to emptyObject

    Don't know why this stopped working. Is it related to bundling? If so, I
    wouldn't expect it to happen in a Jest environment.
    
    Take another look at this before landing.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    0175d3a View commit details
    Browse the repository at this point in the history
  24. Set progressed work on both fibers

    Missed this one.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    303641b View commit details
    Browse the repository at this point in the history
  25. Configuration menu
    Copy the full SHA
    b2d3f7a View commit details
    Browse the repository at this point in the history
  26. Configuration menu
    Copy the full SHA
    f4fa99a View commit details
    Browse the repository at this point in the history
  27. Configuration menu
    Copy the full SHA
    5e7c2fc View commit details
    Browse the repository at this point in the history
  28. Configuration menu
    Copy the full SHA
    b5c980b View commit details
    Browse the repository at this point in the history
  29. Schedule class component effects after lifecycles are called

    ...in case the lifecycles contain updates
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    1b8c460 View commit details
    Browse the repository at this point in the history
  30. Configuration menu
    Copy the full SHA
    b85724b View commit details
    Browse the repository at this point in the history
  31. Extract body of reconcile function into a separate function

    We can reuse it for coroutines, which reconcile children stored on
    the stateNode.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    6971d79 View commit details
    Browse the repository at this point in the history
  32. Move pure begin phase functions out of constructor

    Pretty much anything that isn't component-specific can be moved out.
    Then we can reuse them in the complete phase.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    4424615 View commit details
    Browse the repository at this point in the history
  33. Configuration menu
    Copy the full SHA
    20caa40 View commit details
    Browse the repository at this point in the history
  34. Configuration menu
    Copy the full SHA
    5a34910 View commit details
    Browse the repository at this point in the history
  35. Configuration menu
    Copy the full SHA
    bf2d145 View commit details
    Browse the repository at this point in the history
  36. Configuration menu
    Copy the full SHA
    7aefa2d View commit details
    Browse the repository at this point in the history
  37. Configuration menu
    Copy the full SHA
    0e54237 View commit details
    Browse the repository at this point in the history
  38. Configuration menu
    Copy the full SHA
    1d04ebe View commit details
    Browse the repository at this point in the history
  39. Run test script

    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    4174480 View commit details
    Browse the repository at this point in the history
  40. Run prettier

    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    9940c8a View commit details
    Browse the repository at this point in the history
  41. Fix lint

    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    53d9e0f View commit details
    Browse the repository at this point in the history
  42. Add tests from facebook#8716

    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    7571b0c View commit details
    Browse the repository at this point in the history
  43. Add invariant

    I would expect that whenever we resume on progressed work, the child
    set is a work-in-progress set.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    b9160ba View commit details
    Browse the repository at this point in the history
  44. shouldReuseContent should return false for text nodes

    Without this guard, the function throws
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    a1a2e9e View commit details
    Browse the repository at this point in the history
  45. Fix Flow arity errors

    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    758aa3c View commit details
    Browse the repository at this point in the history
  46. Update newestWork at end of beginWork

    ...rather than in reconcile and begin. Easier to keep track of.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    9df490d View commit details
    Browse the repository at this point in the history
  47. Configuration menu
    Copy the full SHA
    c76dacc View commit details
    Browse the repository at this point in the history
  48. Remove invariant

    Progressed priority could be less than work priority if there's a
    setState deep in the tree that's less than the current render priority
    but greater than the priority at which the fiber last reconciled.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    4577349 View commit details
    Browse the repository at this point in the history
  49. Configuration menu
    Copy the full SHA
    6527bb3 View commit details
    Browse the repository at this point in the history
  50. Use a fiber to stash progressed work

    ProgressedWork is a subset of Fiber. We use a separate Flow type to
    prevent access of extra properties, but we use a whole Fiber for
    monomorphism.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    5e20db9 View commit details
    Browse the repository at this point in the history
  51. Configuration menu
    Copy the full SHA
    bf44b5a View commit details
    Browse the repository at this point in the history
  52. Priority bubbling

    I had this wrong in an earlier commit. Performing work in the
    work-in-progress tree should never add work to the current tree. Only
    state updates do. So in the complete phase, we should bubble priority
    up from the children even if they are current, so that we don't drop
    pending updates.
    
    Also realized there's no reason to check if a work-in-progress is
    valid during a bailout. By the time we get to that function, we already
    determined if the work-in-progress is valid during the
    resume-or-reset step.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    aa77ecf View commit details
    Browse the repository at this point in the history
  53. Simulate Sierpinski triangle demo

    New test suite simulates the triangle demo. It renders a recursive
    tree and flushes it unit by unit, resetting the unit of work pointer
    to the root after each triangle is rendered. It also simulates the hover
    effect by updating a target node with high priority.
    
    The test is written in such a way that we can add fuzz testing on top
    of it later. For now, I've "manually" fuzz tested by trying out
    arbitrary values.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    7af9eae View commit details
    Browse the repository at this point in the history
  54. Change color of dots on each tick in triangle demo

    Makes bugs more noticeable.
    acdlite committed Jun 6, 2017
    Configuration menu
    Copy the full SHA
    2b38dc3 View commit details
    Browse the repository at this point in the history

Commits on Jun 7, 2017

  1. Add pendingWorkPriority to ProgressedWork

    Avoided doing this originally because I didn't want to add a field to
    the fork, and figured we could infer this in other ways. But the fork
    is a full Fiber for monomorphism purposes, anyway, so we already
    have it.
    acdlite committed Jun 7, 2017
    Configuration menu
    Copy the full SHA
    6e94072 View commit details
    Browse the repository at this point in the history
  2. Triangle tester: Simulate multiple hovers in a single step

    This allows for multiple nodes to be active at a time, which isn't
    possible in the demo because to hover over on node you must leave
    the other.
    acdlite committed Jun 7, 2017
    Configuration menu
    Copy the full SHA
    ec911db View commit details
    Browse the repository at this point in the history
  3. Add ReactNoop.yieldBeforeNextUnitOfWork() API

    Most incremental tests rely on yielding at a specific point. Using an
    explicit API, rather than relying on fake expiration times that rely
    on implementation details, should make our tests more resilient and
    reduce false positives.
    acdlite committed Jun 7, 2017
    Configuration menu
    Copy the full SHA
    9cec000 View commit details
    Browse the repository at this point in the history

Commits on Jun 8, 2017

  1. Configuration menu
    Copy the full SHA
    e1209b2 View commit details
    Browse the repository at this point in the history

Commits on Jun 9, 2017

  1. When comparing insertion positions, only compare non-null values

    If `insertAfter` is null, the insertion position is at the start of the
    list. Similarly, if the `insertBefore` is null, the insertion position
    is at the end of the list.
    
    The starting/end position of one list is not necessarily the same as the
    starting/end position of another. So, only compare non-null values.
    acdlite committed Jun 9, 2017
    Configuration menu
    Copy the full SHA
    3a9069d View commit details
    Browse the repository at this point in the history
  2. Improve Triangle simulator

    Previous version couldn't increment the counter more than once before
    flushing, which meant it couldn't reproduce certain errors that occur
    when running the triangle demo in Firefox, where starvation is worse.
    
    The new version uses a Redux-like approach, with actions.
    acdlite committed Jun 9, 2017
    Configuration menu
    Copy the full SHA
    2b72ba2 View commit details
    Browse the repository at this point in the history

Commits on Jun 10, 2017

  1. Fix argument order

    This actually wasn't causing an error because the arguments were
    flipped twice: the first time to the wrong order, and the second time
    back to the correct order :D
    acdlite committed Jun 10, 2017
    Configuration menu
    Copy the full SHA
    0959584 View commit details
    Browse the repository at this point in the history
  2. Introduce new failing test

    This bug illustrates that our `progressedPriority` model is wrong. When
    deciding whether to resume work, we should compare the priority at which
    the parent reconciled its children, not the priority at which the fiber
    itself reconciled.
    acdlite committed Jun 10, 2017
    Configuration menu
    Copy the full SHA
    32cb894 View commit details
    Browse the repository at this point in the history