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

Performance improvements for keyed lists changes #945

Merged
merged 2 commits into from
Jun 14, 2019

Conversation

lifeart
Copy link
Contributor

@lifeart lifeart commented May 29, 2019

this is failing tests related to #940

This PR must increase keyed lists swap perf up to 10x! Moon

(See swap rows row)

from this:
image

to this:
image

@lifeart
Copy link
Contributor Author

lifeart commented May 30, 2019

mutations.map((item)=>{ return item.removedNodes.length ? ['-', item.removedNodes[0]] : ['+', item.addedNodes[0]] }).map(n=>{
	n.push(n[1].textContent);
	return n;
})

from

<li>1</li><li>2</li><li>3</li><li>4</li><li>5</li>

to

<li>1</li><li>4</li><li>3</li><li>2</li><li>5</li>

2 -> 4, 4 -> 2

for @Index test:

image

for keyed test:
image

@lifeart
Copy link
Contributor Author

lifeart commented May 30, 2019

looks like it's working like this:

for muation [1, 2, 3, 4, 5] to [1, 4, 3, 2, 5]

index-0 - ok.
index-1 - now for el 4, move el 4 to index-1,

  • invalidate all index-2 - index-3 nodes (rerender) // to prev element 4 index

add 3 to index-2,
add 2 to index-3

for lists 1-8, 2->7, 7->2

image

if we found first changed ref, we trying to find last changed ref and invalidate middle?

@lifeart
Copy link
Contributor Author

lifeart commented May 30, 2019

Iterator Synchronizer doing a lot of unnecessary work.

for [0...8], 1->7, 7->1

image

@lifeart
Copy link
Contributor Author

lifeart commented May 30, 2019

[0,1,2,3,4,5,6,7,8]

1->7,7->1

[0,7,2,3,4,5,6,1,8]

0 - retain          | [0,1,2,3,4,5,6,7,8]
7 -> move before 1  | [0,7,1,2,3,4,5,6,8]
2 -> move before 8  | [0,7,1,3,4,5,6,2,8]
3 -> move before 8  | [0,7,1,4,5,6,2,3,8]
4 -> move before 8  | [0,7,1,5,6,2,3,4,8]
5 -> move before 8  | [0,7,1,6,2,3,4,5,8]
6 -> move before 8  | [0,7,1,2,3,4,5,6,8]
1 -> move before 8  | [0,7,2,3,4,5,6,1,8]
8 - retain

@lifeart
Copy link
Contributor Author

lifeart commented May 30, 2019

how fix work?

cursor 0 -> retain
cursor 1 -> new item, found new item, insert before cursor 1 -> next cursor ...

@lifeart lifeart changed the title tests: keyed lists mutations counter feat: performance improvements for keyed lists changes May 31, 2019
@lifeart lifeart force-pushed the keyed-list-mutations-counter-test branch from 7787e73 to fe950b9 Compare May 31, 2019 14:23
@lifeart lifeart force-pushed the keyed-list-mutations-counter-test branch from 630bf24 to 20bf405 Compare June 1, 2019 09:29
@lifeart
Copy link
Contributor Author

lifeart commented Jun 3, 2019

@wycats is any updates/fixes I can add?

@wycats
Copy link
Contributor

wycats commented Jun 13, 2019

@lifeart I'm taking a look at this today 😄

Thanks for all the effort!

@wycats
Copy link
Contributor

wycats commented Jun 14, 2019

💥

@wycats wycats merged commit cad7655 into glimmerjs:master Jun 14, 2019
@rwjblue rwjblue changed the title feat: performance improvements for keyed lists changes Performance improvements for keyed lists changes Jun 25, 2019
@lifeart lifeart deleted the keyed-list-mutations-counter-test branch March 27, 2020 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants