Improve trunk eviction in EditManager#16713
Merged
Merged
Conversation
Collaborator
⯅ @fluid-example/bundle-size-tests: +12 Bytes
Baseline commit: 19c99b7 |
yann-achard-MS
approved these changes
Aug 8, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The SharedTree EditManager's trunk commit eviction was both more complicated than necessary and also buggy. This PR cleans up the code and addresses the known bugs. Previously, when the minimum sequence number advanced to some sequence number M, all commits with sequence numbers below M were dropped, but a commit with exactly sequence number M was not because any number of branches might be based off of that commit (e.g. the local branch), and dropping it was impossible because a branch might continue to reference it. After this PR, that commit is still not dropped from memory, however it takes the place of the special "trunk base" commit or "origin" commit which precedes the trunk but is not considered part of the trunk. Not only does this remove multiple edge cases from the eviction logic (and the bugs that currently dwell therein), it also makes it more intuitive to predict which commits are removed from the trunk as the minimum sequence number advances.
This PR also does some minor cleanup and optimizations in EditManager, adds some more eviction test cases, and enables an eviction pass after tracked branches rebase (which will evict the trunk more aggressively in scenarios where a long-lived user-created branch repeatedly rebases over a parent branch).