-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[draft] Benchmark linked list v11 impl #4256
Draft
andrewiggins
wants to merge
72
commits into
v11-benchmark-base
Choose a base branch
from
v11-linked-list-prev-index-nextDom-2-benchmark
base: v11-benchmark-base
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
[draft] Benchmark linked list v11 impl #4256
andrewiggins
wants to merge
72
commits into
v11-benchmark-base
from
v11-linked-list-prev-index-nextDom-2-benchmark
Conversation
This file contains 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
- Removed vnode arg to mount - Fixed up mounting with linked list - Use _index property on internal to track null holes in children Co-authored-by: Jovi De Croock <[email protected]> Co-authored-by: Jason Miller <[email protected]> Co-authored-by: Marvin Hagemeister <[email protected]>
After running into problems with the 'should move multiple keyed children to the beginning' test with the previous commit, we are switching our insertion loop to iterate backwards through the internal children to do insertions. In summary: First loop: - find matching internals (using if _prev pointer is set to determine if an internal has been used) - setup _prev pointers to be correct Second loop: - setup _next pointers to be correct - insert or mount dom children (detect if a node was moved by comparing if internal._prev._next == internal) There are still bugs though but above is the general idea we are going for Co-authored-by: Jason Miller <[email protected]> Co-authored-by: Marvin Hagemeister <[email protected]>
Unmount before inserting means we can use _prev == null to detect internals that need to be unmounted and removes unmounted nodes from the internal children linked list when doing inserts (so all _prev and _next pointers point to internals that will be included) Use `.data` to detect if a component has been mounted or not. Fix up next pointers when mounting or moving internals Co-authored-by: Jason Miller <[email protected]>
- Comment out and skip tests we want to currently ignore - Just focus on number of ops for reverse list atm
Here I'm using a variant of the longest increasing subsequence algorithm to determine which nodes to move. There are two key differences in my algorithm: 1. I am looping over the nodes in reverse order, so my algorithm is actually a longest decreasing subsequence (LDS) instead of longest increasing subsequence (LIS). 2. I don't use a binary search to insert new nodes into the "wip" LDS array.
… recursion I focused on the keys, render, fragments, and placholder tests for this commit
See test "should support moving Fragments between beginning and end" for an example failure this change fixes
See test "should insert a portal before new siblings when changing container to match siblings"
* Remove usages of MODE_UNMOUNTING & MATCHED_INTERNAL * Clean up and improve comments
... since we are reusing it for different purposes in the diff algorithm
To fix this I had to teach getDomSibling to skip over nodes marked for insertion, as well as properly handle nodes with no children. See the fragment test "should preserve state when it does not change positions" for a test case that was fixed. Note: This breaks a Portal test when changing a portal from having a different parent to the same parent. Probably need to detect that and set the moved flag to true. Maybe we could detect it in findMatches and set the move flag as well as a PORTAL flag on the internal to speed up similar checks
Speeds up benchmark perf on many_updates
Reduces recursion level by 1 function
I think this saves bytes cuz the implementation is another "loop over internals" and putting it into its own function gives the core internal variable the same minified name as other functions that also loop over internals.
Lots of tests changed cuz we unmount before doing insertions now
…DOM nodes Fixes the portal test "should insert a portal before new siblings when changing container to match siblings"
Size Change: +290 B (0%) Total Size: 37.9 kB
ℹ️ View Unchanged
|
Thompson1985
approved these changes
Oct 6, 2024
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.
No description provided.