Optionaly patch nodes by keys.

[akhilman]

Patch old/new nodes by key if provided.

Continues #356
Solves #354

Progress:

• New patching algorithm
• Unit tests
• Example app
• Rebase to current master

[akhilman]

I will provide proper example.

Here how circles looks now:

[MartinKavik]

Good job, keep going 🙂 I'll add keyed Seed benchmark once it's merged and released.

[David-OConnor]

Hell yea brother

[akhilman]

I finally managed to do it right. Now I’m going to make a proper example and unit test,

[akhilman]

Here is the test app to play with.
I'm still not happy with the the element ordering. I will try to redo this.

[akhilman]

I did it!
The code is still dirty, but it works well.
Most magic happens in the src/virtual_dom/patch_commands.rs. Actually this can almost entirely replace current patch.rs.

Later I will explain how the algorithm works and why I consider it better than ELM's one.

Here how an example app looks like:

[MartinKavik]

Very cool! Let me know when I should dive into the code. Also I've written list of potential optimizations - #385 - feel free to update/extend (not only) Virtual DOM section.

[akhilman]

ELM's algorithm

Takes two nodes the old and the new one.
If the key doesn't match takes two more nodes.
If there is match in keys between the two new and the two old nodes patches the node with matching key and inserts/removes the rest two nodes.
Replaces nodes if there are still no matching keys.
Repeats from start.
https://github.com/elm-lang/virtual-dom/blob/45d7d63ddc26ee4b22b14432e6e426f7ac7a689b/src/Elm/Kernel/VirtualDom.js#L979

Suggested alorithm

Suppose we have old and new child nodes with el_keys:

old: [a] [b] [c] [d] [e] [f] [k] [l]
new: [a] [d] [e] [b] [c] [x] [f] [y]

The algorithm starts by taking nodes form the source iterators one by one in turn (new and old)
and putting them in the corresponding queues:

old: [a]
new: [a]

Now we have the matching key in queues. The algorithm takes nodes from the queues and yields
patch [a] by [a] command.

But then nodes become diverse:

old: [b] [c]
new: [d] [e] [b]

As soon as the algorithm finds the matching key [b] it yields three commands:
insert [d] before old [b], insert [e] before old [b] and then patch [b] by [b].
Old node [c] remains in queue.

The algorithm continues to fill queues and stops with matching nodes [c]. Then it issues patch
command:

old: [c]
new: [c]

Then nodes again become diverse:

old: [d] [e] [f]
new: [x] [f] [y]

The algorithm stops when finds the matching key [f] and yields three commands:
replace [d] by [x], remove [e], patch [f] by [f].

At this point the source iterator for the new nodes has been exhausted and the algorithm
continues to take only old nodes.

old: [k] [l]
new: [y]

At this point both source iterators are exhausted and the algorithm yields:
replace [k] by [y] and remove [l].
The append command means append as a last children and appears when there is no matching key in
front.

As bonus we do not need to call the next_sidling() anymore to find the next node. We always know where to put new node.

[MartinKavik]

I'm not the author of the previous patching algorithm and I've only fixed some related bugs so I don't know how much it differs from the old code (except the keys).
But your suggestions sounds good and Elm VDOM is fast so I don't see any problems with it, nice.

Please add also some tests so we don't break it during refactors.

And perhaps we should copy-paste your previous comment to the code as the documentation.

[akhilman]

Yes. The text from previous comment already in the source code documentation. Tests will also be written.

…

On Sat, 2020-03-21 at 13:32 -0700, Martin Kavík wrote: I'm not the author of the previous patching algorithm and I've only fixed some related bugs so I don't know how much it differs from the old code (except the keys). But your suggestions sounds good and Elm VDOM is fast so I don't see any problems with it, nice. Please add also some tests so we don't break it during refactors. And perhaps we should copy-paste your previous comment to the code as the documentation.

[akhilman]

@MartinKavik @David-OConnor I'm ready for review.
There still some optimizations that I have in mind, but they could be done later.
Please tell If there is some unclear code. I'll fix it or add comments with explanation.

[MartinKavik]

The first code review done. I've checked all files except patch.rs and patch_gen.rs. The most comments are about styling, best practices, missing docs etc., functionality looks good, thanks!

[akhilman]

On Mon, 2020-04-13 at 04:52 -0700, Martin Kavík wrote: @MartinKavik commented on this pull request. Is it in a debug mode? What about release mode + configured wasm-opt for size?

Yes it s in debug mode. I'll try to compile with optimization later.

Can we find out if those optimizations don't replace our inlines with their original function calls (or optimize them away by another way)? And what do you think about speed boost?

We could compile to WebAssebly text format and check the code. As I know there no debuger for wasm.

I'm asking because on the first glance they look like a premature optimization (or no optimization at all if the size is a trade-off and speed boost isn't reasonable big). And it's another thing we have to think about while writing and reading code, doing code reviews and test (once the bechmarks are configured).

For the native binary performance boost could be significant. But anyway we should do benchmarks to get the real numbers.

[MartinKavik]

Note: Waiting for https://github.com/akhilman/seed/pull/1

[akhilman]

Thanks for fixes.

[MartinKavik]

Local non-keyed bench:

• 0.6.0
  
• feat/keys
  

The most results are better, good job!
I think that keyed results will be very interesting :)

[akhilman]

I assume speed up is due to the fact that we no longer call the next_sidling() method. Otherwise, the new algorithm should be slower.