Improve hydration by reordering optimally

[hbirler]

(Hopefully) Closes #4308.

I am new to working with Svelte so I apologize for any errors beforehand.

Explanation

I tried to improve #6204 based on the thread #4308. I personally had problems with elements with CSS animations flickering due to hydration detaching and reattaching nodes (implicitly by re-inserting them) as described in the thread. The page that I was having problems with was almost entirely static, so seeing all the static nodes refresh went against my intuition as to how hydrate would work. So I looked into how this situation could be improved.

Old algorithm

Edit: The algorithm has been improved significantly. See #6395 (comment) for the new algorithm

During hydration, this new implementation tries to pick a sequence of nodes that we are able to not detach: the first common subsequence of the original Nodes and the claim_* requests.

For example, if the original nodes are:
B C D E F D G

and the claim_element function calls are as follows:
A B C D D E F G H

the first common subsequence would look like
B C D D G

You can find this subsequence by iterating through the claim_elements and keeping a pointer on the original nodes that you are only able to move forward. For every claim_element, if you are able to move the pointer to a matching node on the right, do it. Otherwise, do not move the pointer.

The algorithm chooses to preserve the nodes within this subsequence (i.e. not detach them). The rest of the nodes that are requested by claim_* are handled as before: they are either found among existing nodes to be detached or created anew.

Normally, after the claims are done, append and insert are called with every single child, since the caller assumes that all the children are detached. Since this is not the case anymore (the shared subsequence was not detached), we need to transparently make sure that append does what is expected, which in this case might be to insert a node in the middle of existing non-detached children. During append, the algorithm tracks the last appended node, i.e., what the caller thinks is actually the last child of the target. Thus, we can deduce where the caller actually wants to append the node. The insert function works similarly.

Possible optimizations

Note that the first common subsequence is not the longest common subsequence:
B C D E F G

We would prefer to not detach as many nodes as possible, so the longest common subsequence would theoretically be preferable. However, since the claim_* functions (if I understand them correctly) are expected to return Nodes directly (and greedily), I am not sure a change can be implemented without changing the compiler side.

Even though finding the optimal (longest) subsequence to not detach is problematic, it should be possible to further improve the greedy approach based on heuristics. In the example above, the second D node that is claimed results in the two nodes E and F getting detached. If the D node is just Text and E/F are large Elements, that would be quite a sad occurrence. This could be optimized in the future handling Text Nodes differently. I did not implement such an optimization since I was not sure my assumptions are correct and would love to receive feedback.

One might also look into what virtual DOM front-end frameworks are doing to find minimal differences https://github.com/localvoid/ivi/blob/2c81ead934b9128e092cc2a5ef2d3cabc73cb5dd/packages/ivi/src/vdom/implementation.ts#L1367 . But I am not sure how applicable those will be to our situation.

There may be many more possible optimizations! I am relatively inexperienced with front-end development so I might be missing many optimization opportunities.

Possible issues

Within the code, I added an additional property actual_end_child to Node objects during append and insert calls to make tracking last appended nodes easier. If this is a problem, we can switch to using a Map<Node, Node>.

Tests

The code passed all the tests but I did not add any new tests since I wasn't sure how to test for whether nodes are being detached. Maybe a total number of nodes detached counter could be used to ensure that the algorithm is working and further optimizations do not cause regressions. Any feedback on this is appreciated.

[hbirler]

From what I understand #6392 is similar in that it implements a similar insert/append logic, but it does not change the claim_* functions. The original claim_* functions always assign the first possible matching node to a claimed element, if such a node exists.

In the example that I gave with the original nodes:
B C D E F D G

and the claim_element function calls:
A B C D D E F G H

that pull request seems to do the following mutations (step-by-step states):

B C D E F D G    p:0 (create A)
A B C D E F D G    p:1 (nothing B)
A B C D E F D G    p:2 (nothing C)
A B C D E F D G    p:3 (nothing D)
A B C D D E F G    p:4 (move D)
A B C D D E F G    p:5 (nothing E)
A B C D D E F G    p:6 (nothing F)
A B C D D E F G    p:7 (nothing G)
A B C D D E F G H    p:8 (create H)

So B C D E F G is not detached which is better than my implementation in this example.
But on the actual page that I was testing on that pull request causes a refresh of most elements while this pull request does not.

A worst case for this pull request is when an early element is matched very late (since the first common subsequence will be very short).

A worst case for #6392 is when a late element is matched very early (since all the other elements have to move in front of that element).

[hbirler]

From what I know & understand, given arbitrary claim_* functions, the minimum amount of reorderings that assign & insert have to do can be computed via the longest increasing subsequence.

So, in a sense, we could potentially implement the claim_* functions such that they make the longest increasing subsequence as long as possible (using heuristics).

Then, we can theoretically implement insert & append in a way that they do the minimum number of moves given the claims.

[hbirler]

I have implemented optimal reordering and changed the claim_* logic somewhat. Here is the new algorithm which is also influenced by #6392:

New algorithm

The new algorithm computes the optimal amount of reorderings for a given set of claims. The claims are made in such a way that the optimal amount of reorderings is likely small.

There are two steps in hydration that are relevant to us. The first step is the claim step where existing nodes are picked to be reused one by one (or created when no corresponding node already exists). The second step is the modification step where claimed nodes are reordered and newly created nodes are inserted.

Let's say that our existing nodes are:
B C D X F G E H

And we claim:
A B C D E F G H I

During the modification step, there are three operations that we do modify our existing nodes to match the claims.

1. Detach: We remove nodes we have not claimed (X):
   B C D F G E H

2. Reorder: We reorder the nodes to match the claim order (reorder E to the left). This step is executed at the first append/insert:
   B C D E F G H

3. Insert: We insert newly created nodes into their corresponding spots:
   A B C D E F G H I

What we can optimize in this approach is to minimize the total number of reorderings. There are two factors that effect the total number of reorderings:

The first factor is the reordering operations that we actually choose to do.
For example, the sequence B C D A can be sorted in multiple ways.
Either we move A to the left and do only one operation.
Or we move B, C, and D one-by-one to the right and do three operations.

Luckily the optimal amount of reorderings can be computed algorithmically in O(n log n) time, which I have implemented in the latest commit. The optimal amount of reorderings is given by (number_of_nodes - longest_ordered_subsequence_of_claimed_nodes). So if we maximize the longest ordered subsequence of claimed nodes, we minimize the amount of reordering, since only the nodes that are not in the longest ordered subsequence are reordered.

The second factor is, of course, the claiming order. If there are multiple elements with the same tag we are trying to claim, we can choose any one of those elements. This choice will effect the total number of reordering. What I do is, I remember the position of the last claim, and try to claim a node to its right side. If I cannot do it, I claim a node to the left (or create a new node when no matching node exists). I am hoping that this heuristic will maximize the longest ordered subsequence. (Additionally, if I am only claiming a text node, I do not update the position of the last claim, since I do not want text nodes to break a sequence of real elements. My notes under possible optimizations in the original post contain an example for this #6395 (comment))

New possible issues

• More data is now stored in nodes. We can choose to use a Map<Node, Data> instead.
• I am not sure about the following check in the code https://github.com/sveltejs/svelte/pull/6395/files#diff-7ad300a4efc821b2576200a7bfc99205d558792b81c037fbc20f9236f8b6f1bbR114 .
  The ((target.actual_end_child !== null) && (target.actual_end_child.parentElement !== target)) is to satisfy an error I received in head-detached-in-dynamic-component where a title node was stored as the actual_end_child but was later missing.

[hbirler]

I updated the title to better reflect what the algorithm is doing now.

[ehrencrona]

This is very clever. I ran some benchmarks on my site and this seems significantly faster than my PR #6392; maybe 15% faster or so. So I'd support this PR over mine. I'll try to add some detailed comments on the code later.

[benmccann]

@hbirler we just checked in some new tests for hydration. Can you rebase this PR against master so that we can make sure the tests still pass?

[Conduitry]

I just tried this locally, and all of the new tests pass with these changes.

[hbirler]

I rebased the branch. The tests pass locally for me as well.

[nickreese]

@hbirler Thank you soo much for your effort on this.

[benmccann]

@hbirler thanks for this. Hydration performance has been a long standing issue, so it's great to see improvements!

I just updated a SvelteKit site of mine to use it. I see in the Chrome performance tab screenshots that the logo on my site (https://c3.ventures/) momentarily disappears and then is rerendered. I set hydration: false and this flash no longer occurs. I'm curious why the flicker is occurring given this change. The content on my site is all static and the page is prerendered, so there should be nothing like a timestamp that might differ between server and client. I'd thus expect the common longest subsequence to be the entire DOM. Do you have any idea why this optimization might not be working for me? If you'd be interested or willing to take a look, I can add you to the GitHub repo for my SvelteKit project so that you can take a peek.

[hbirler]

@benmccann I think I might have found out what the issue is.
To update an image's src attribute, svelte generates the following code:
if (img.src !== (img_src_value = srcVariable)) attr(img, "src", img_src_value);

The idea is, if the source is already set correctly, do not update it.
In your website, I have found a matching line in compiled index.svelte:
he.src !== (me = L) && m(he, "src", me)

However, if I look at the actual values:
he.src === https://c3.ventures/_app/assets/c3-logo-alpha-7c844443.webp
me === /_app/assets/c3-logo-alpha-7c844443.webp

So even though the values are theoretically equivalent, we still end up changing the src attribute to something else. I believe this might be the issue.

The img node pre-hydration looks like:
<img src="/_app/assets/c3-logo-alpha-7c844443.webp" width="262" height="68" alt="C3 Ventures">

So the issue is not that the generated ssr html is wrong. Reading the src attribute client side somehow returns the full url. Maybe someone more experienced with web development than me can comment on this? Maybe a more proper equivalence check can be done rather than just ===?

[hbirler]

Whatever the solution, the srcset attribute may need a similar correction.

[hbirler]

(new URL(relativePath, location)).href seems to be a good way to normalize a URL.
https://stackoverflow.com/a/67509097/4415649

[hbirler]

@benmccann Does https://github.com/hbirler/svelte/tree/better-hydration-src fix your issue?

[ankitstarski]

In my case, I have a stylesheet linked in head. That link tag is getting removed and added back again, giving me a Flash of unstyled content. Also, increasing my LCP by a lot.

[ankitstarski]

I'll check with your branch if that fixes it.

[ankitstarski]

Tried your branch, does not work for me.

[hbirler]

I only tried to fix the src issue in the branch :( .

[hbirler]

@ankitstarski It might be that the header tags generated by the SSR somehow do not match the order the client side expects them to be in, but I do not know how this might happen.

Edit: claiming elements from the header seems to work differently than normal nodes in Svelte. I am not sure what kinds of changes would be required to properly handle nodes under <head>

[hbirler]

@ankitstarski Does the branch solve your problem now? I have committed another change.

[ankitstarski]

It could be that I'm using sapper with some tags in head inside template.html.

I'll check with your branch and update here.

[benmccann]

Thanks @hbirler. Unfortunately I get the same behavior on that branch

[hbirler]

The branch does improve a local test of mine for a similar situation. It's weird that it did not work for you, maybe your website hits another corner case.

[ankitstarski]

@hbirler It does fix the issue for me. But high LCP and high Time to Interactive are still there compared to a custom build of svelte that I'm using based on Avi's patch .

Also, thanks for your contribution! 🥇

[hbirler]

@ankitstarski You can try the branch again to see if there are any improvements. The claim_text change improved a local test of mine.

[ankitstarski]

No noticeable change.

[non25]

I believe this PR is causing #6444
@hbirler can you test by wrapping each in div with and w/o your PR?
Thanks.

[hbirler]

@non25 I believe #6445 fixes that issue