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

[draft] Benchmark linked list v11 impl #4256

Draft
wants to merge 72 commits into
base: v11-benchmark-base
Choose a base branch
from

Conversation

andrewiggins
Copy link
Member

No description provided.

andrewiggins and others added 30 commits December 21, 2022 11:26
- 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"
Copy link

Size Change: +290 B (0%)

Total Size: 37.9 kB

Filename Size Change
compat/dist/compat.js 3.53 kB -12 B (0%)
compat/dist/compat.umd.js 3.6 kB -14 B (0%)
debug/dist/debug.js 3.12 kB +31 B (0%)
debug/dist/debug.umd.js 3.19 kB +20 B (0%)
dist/preact.js 4.62 kB +94 B (2%)
dist/preact.min.js 4.67 kB +92 B (1%)
dist/preact.umd.js 4.71 kB +79 B (1%)
ℹ️ View Unchanged
Filename Size Change
devtools/dist/devtools.js 232 B 0 B
devtools/dist/devtools.umd.js 317 B 0 B
hooks/dist/hooks.js 1.47 kB 0 B
hooks/dist/hooks.umd.js 1.55 kB 0 B
jsx-runtime/dist/jsxRuntime.js 296 B 0 B
jsx-runtime/dist/jsxRuntime.umd.js 379 B 0 B
server/dist/server.js 2.6 kB 0 B
server/dist/server.umd.js 2.69 kB 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.umd.js 522 B 0 B

compressed-size-action

@preactjs preactjs deleted a comment from github-actions bot Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants