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 issue with runes / non-runes interop and deep_read on large datasets #10637

Closed
Prinzhorn opened this issue Feb 26, 2024 · 6 comments
Milestone

Comments

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Feb 26, 2024

Describe the bug

Originally posted as #10628, but now with a much more simplified demo.

I've ported over a demo of my virtual list from Svelte 4 to Svelte 5. There's an App.svelte that renders items using VirtualList.svelte. In the process I've noticed that when the App.svelte uses runes but VirtualList.svelte does not, performance tanks massively. Depending on the number of items in the virtual list, you can completely freeze the app. I assume this happens because a large proxied $state array with items (e.g. 10k or even 100k) in App.svelte is converted to vanilla array via deep_read for the VirtualList.svelte to consume.

The REPL should be self explanatory. For simulation I have 10k items and the virtual list renders the first 10 of them. A setTimeout is used to simulate new data arriving every 100ms. In the interop version you'll see spikes every 100ms. With 100k items my fans become unbearably loud. The other two versions are chilling.

Non-runes

image

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAA41UUW-bMBD-Kzc6KaAySF8pRKq0TYq0PnXaHkpVOXApVoyN7CNphPjvk-1AmLqHJQ_m7O--u-_u7CHYc4EmyJ6HQLIWgyx46LogDujcWcMcURAGcWBUryu7k5tK8442pQTgbac0wQBKPqpeUgxKfkVDWp1hhL1WLaw8w-p-gf_FNfVM_OCGLqAkXewlV5dSlrTvZUVcSWjZAbeErQll37qPGAwxTdsaClhHMFh4SRqp1xIknuBBa3ae4VGy50KEshciSlrWheFrDDyCYjO5zs7OBvfjdTaHuQUeX0-Ik8AMVj_tCiu4hUdGTaKZrFUbRjNy9ORjZFdrSACBBIa3vWCE3xGtAtI93k9n3CYMxUL03fp1vV5Hl6pcKh6GU_o2UJrC04XTya8ZMWBa8yOXbxBi8pbAkTP4jbsnVR2QosQ7VkoaAuItaijAIG0loT4y8Tc_AN9D-GmZd3Q9A_DFu582Rp-q_fsAEk_bf-iKvdpEoHyjJpr9_W7XmyZMkmRydhVwpFON3HrxGmO4u1Zp7md47bLTK5DpWaPTPdGOlme0Vp7Oo17KvLnb-NyHZbJjnjZ3HrCcagcpPHK0_czsp8VRXvPjxoe6cYCE1yN88bSJGyk3L3nqgfnyavhInbt8-a4nUhLsTS3KwFtlAEpmleDVoRi86HAq0_NLNG6-tR2d89SjPY9gOxSbnMuup4mtarA67NR7GcCOyzpzNtbFsOz9COnmOnB7O8Uh6zqUtZs-FzfKU89vlXQu_yAOWlXzPcc6yOzQj_H8-Cy0_u8jhO_uUZkvzaX1aQo_G26AGzg1jIAahKNnB2F7tFNcGKjVSQKpzLp8zsAIXuE0UomzwrUdqQ8DMdwgq5qLAzPOwzfOCEW-nbZApRxSixw_yn4Z_wBlRBMvfwUAAA==

Runes / non-runes interop

image
image

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAA41U32-bMBD-V260UkBlkL5SEqnSNinS-tRpeyhV5cClWDE2so-kEeJ_n2wDYerL4AHO_u7Hd_fZfXDgAk2QvfSBZA0GWfDYtkEc0KW1hjmhIAziwKhOl3YlN6XmLW0LCcCbVmmCHpR8Up2kGJT8hoa0usAAB60aWPkIq4cF_jfX1DHxkxsaQUm6WEuuLoUs6NDJkriS0LAj7ggbE8qucT8xGGKadhVsYB1Bb-EFaaROS5B4hket2WWGR8mBCxHKTogoaVgbhm8x8Ag228l1dnY2uIdX2ZzmDnh83SFOAjNY_bJfWMEdPDGqE81kpZowmpGDDz5E9msNCSCQwPCmE4zwB6JlcGuIEYakO4weJgy3hV83rz24X7-t1-soGrs0TiAMJzo2cZrC85jDtaNixIBpzU9cvkOIyXsCJ87gD-6fVXlEihLvWCppCIg3qGEDBmknCfWJiX_jA_ADhF-WPKLrHoBv5sO0MPhS7esTSDzvRn5LYrFnnQiU71RHs79fbTtTh0mSTM7T_hDD_Xo99WOeZHidr2MmkOmZjWPoXMhNBmCwVp7OIi9kXt9vfZX9sqwhT-t7D1jq2UE2HjnYCWb21-Ior_hp61PdOEDCqwG--rCJE5NTSp56YL48FD5T645dvu-IlAR7RjdF4K0iACWzUvDyuOk96XASz8trNGy_Ny1d8tSjfRzB9ii2OZdtR1O0ssbyuFcfRQB7LqvM2Vht-uWUB0i3V2kdrH5D1rYoK6czlzfKUx_fMmld_UEcNKriB45VkFmdD_F87Sy4_u_1gx_uOpmPyTj6NIVfNTfADZxrRkA1wslHB2FntFdcGKjUWQKpzLrcZmAELxE2o8icFa6tpD4Jor9BVtajAzPOww_OCEV-nLZBhexTixw-034d_gKGtIHeeQUAAA==

Runes

image

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAA41UTY-bMBD9K1O2UkBLgVxZEmmltlKk7mmr9rCsVg5MFivGRvaQNEL898p2IFR7aXKAsd98vJk3DMGBCzRB_jIEkrUY5MFj1wVxQJfOGuaEgjCIA6N6XdmTwlSad7QtJQBvO6UJBlDySfWSYlDyKxrS6gIjHLRqYeUjrB4W-F9cU8_ED27oCkrSxVlycyllSYdeVsSVhJYdcUfYmlD2rXuJwRDTtKthA1kEg4WXpJF6LUHiGR61ZpcZHiUHLkQoeyGipGVdGL7FwCPYbCfX2dnZ4H68zuc098Dj2w1xEpjD6qd9wgru4YlRk2gma9WG0YwcffAxsk9rSACBBIa3vWCE3xEtg8-GGGFIusfoYcJwW_jt8taDdfaWZVkUXbt0nUAYTnRs4jSF52sO146aEQOmNT9x-Q4hJu8JnDiD37h_VtURKUq8Y6WkISDeooYNGKSdJNQnJv6ND8APEH5a8ohudwC-mQ_TwehLtX-fQOJ5d-W3JBZ71olA-U5NNPv70643TZgkyeQ83Y8xrLNs6sc8yfA2X8dMINMzG8fQuZCbDMBorSKdRV7KollvfZXDsqyxSJu1Byz17CAbjxztBHP7anFU1Py09anuHCDh9QhffNjEickppUg9sFguhc_UubUr9j2RkmB3dFMG3ioDUDKvBK-Om8GTDifxvLxG4_Zb29GlSD3axxFsj2JbcNn1NEWrGqyOe_WnDGDPZZ07G-vNsJzyCOn2Jq2D1W_Iug5l7XTm8kZF6uNbJp2rP4iDVtX8wLEOcqvzMZ4_Owuu__v5sfsxXDdktDvSadWZcJJAmsLPhhvgBs4NI6AG4eSzgLCz2isuDNTqLIFUbl3cVgpeoY1Wo-YnrH0XE3ccZlZk0QeNDHfIqubqyoyryc_SCEV-wrZnpRxSixw_duJ1_Auw5YchjAUAAA==


Sure this is an edge case, but since Svelte 5 is meant to be backwards compatible, it's not unrealistic that you're using a Svelte 4 component in a Svelte 5 app.

In my app it doesn't matter, because the items in my virtual list come from a readable store that abstracts WebSockets, so the items aren't $state.

Since VirtualList.svelte never accesses the entire items, maybe the deep_read conversion should happen lazily when calling slice()? Or alternatively all operations on the proxy, such as push should be mirrored into a non-proxied version immediately. No idea if that's possible, right now I assume you convert all props via deep_read and throw them at the other component.

Reproduction

See REPLs. In the "Runes / non-runes interop" REPL the checkbox conveniently acts as a checkbox for you PC fans as well.

Logs

No response

System Info

Likely irrelevant

Severity

blocking an upgrade

@dummdidumm dummdidumm added this to the 5.0 milestone Feb 26, 2024
@dummdidumm
Copy link
Member

The problem is that proxied state means that updates to a property that is reactively tracked will only update the signal that is underlying to that property. So in order for a Svelte 4 component to know that anything inside a variable has changed, it needs to traverse the whole object to setup listeners for all properties before any possible mutations.

We'll try to make it less bad, but there's probably an unavoidable overhead when using a Svelte 4 component from a runes component.

dummdidumm added a commit that referenced this issue Feb 27, 2024
…side legacy mode and inside actions, because the input we get could be a state proxy. This proxy will have fine-grained reactivity and therefore only update the signal at the location that got an update. In order to have interop with the coarse-grained "notify me when _anything_ in that object changes" behavior, we need to setup up signals for all of the object's properties before they're potentially mutated.

This has a lot of overhead for large lists, and we can at least diminish in the "no state proxy" case by applying a sensible heuristic:
- If the value passed is a state proxy, read it
- If not, and if the value is an array, then bail because an array of state proxies is highly unlikely
- Traverse the first level of properties of the object and look if these are state, if not bail. State proxies nested further down are highly unlikely, too

part of #10637
dummdidumm added a commit that referenced this issue Feb 27, 2024
This has a lot of overhead for large lists, and we can at least diminish in the "no state proxy" case by applying a sensible heuristic:
- If the value passed is a state proxy, read it
- If not, and if the value is an array, then bail because an array of state proxies is highly unlikely
- Traverse the first level of properties of the object and look if these are state, if not bail. State proxies nested further down are highly unlikely, too

part of #10637
@trueadm
Copy link
Contributor

trueadm commented May 11, 2024

This looks like it has since been addressed, so closing.

@trueadm trueadm closed this as completed May 11, 2024
@Prinzhorn
Copy link
Contributor Author

Has pretty much not changed at all:

image

image

@trueadm
Copy link
Contributor

trueadm commented May 11, 2024

Did you look in prod or dev mode?

@Prinzhorn
Copy link
Contributor Author

I opened the REPLs from above

@trueadm
Copy link
Contributor

trueadm commented May 11, 2024

So I don't see anywhere near as much overhead for me, I see 24ms. I know this isn't ideal, but unfortunately, this will be a side-effect from mixing both systems because of how Svelte 4 works under-the-hood.

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

No branches or pull requests

3 participants