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

Experiment: Unsuspend all lanes on update #20660

Merged
merged 1 commit into from
Jan 26, 2021

Commits on Jan 26, 2021

  1. Experiment: Unsuspend all lanes on update

    Adds a feature flag to tweak the internal heuristic used to "unsuspend"
    lanes when a new update comes in.
    
    A lane is "suspended" if we couldn't finish rendering it because it was
    missing data, and we chose not to commit the fallback. (In this context,
    "suspended" does not include updates that finished with a fallback.)
    
    When we receive new data in the form of an update, we need to retry
    rendering the suspended lanes, since the new data may have unblocked the
    previously suspended work. For example, the new update could navigate
    back to an already loaded route.
    
    It's impractical to retry every combination of suspended lanes, so we
    need some heuristic that decides which lanes to retry and in
    which order.
    
    The existing heuristic roughly approximates the old Expiration Times
    model. It unsuspends all lower priority lanes, but leaves higher
    priority lanes suspended.
    
    Then when we start rendering, we choose the lanes that have the highest
    LanePriority and render those -- and then we add to that all the lanes
    that are highher priority.
    
    If this sounds terribly confusing, it's because it barely makes sense.
    (It made more sense in the Expiration Times world, I promise, but it
    was still confusing.) I don't think it's worth me trying to explain the
    old behavior too much because the point here is that we can replace it
    with something simpler.
    
    The new heurstic is to unsuspend all suspended lanes whenever there's
    an update.
    
    This is effectively what we already do except in a few very specific
    edge cases, ever since we removed the delayed suspense feature from
    everything that's not a refresh transition.
    
    We can optimize this in the future to only unsuspend lanes that are
    either 1) in the `lanes` or `subtreeLanes` of the node that was updated,
    or 2) in the `lanes` of the return path of the node that was updated.
    This would exclude lanes that are only located in unrelated sibling
    trees. But, this optimization wouldn't be useful currently because we
    assign the same transition lane to all transitions. It will become
    relevant again once we start assigning arbitrary lanes to transitions
    -- but that in turn requires us to implement entanglement of overlapping
    transitions, one of our planned projects.
    
    So to sum up: the goal here is to remove the weird edge cases and switch
    to a simpler model, on top of which we can make more substantial
    improvements.
    
    I put it behind a flag so I can run an A/B test and confirm it doesn't
    cause a regression.
    acdlite committed Jan 26, 2021
    Configuration menu
    Copy the full SHA
    40e168a View commit details
    Browse the repository at this point in the history