-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
Improve performance of structuredClone
#50320
Comments
Your report isn't actionable unless you have concrete improvement suggestions. "Increase throughput" isn't. In case you're unaware: structuredClone is implemented in terms of V8's value serializer/deserializer API, i.e., the meat of the work is done inside V8, not node. |
@nodejs/performance we could consider a fast path for certain types of values if that's an interesting use case. |
I really doubt that once you handle all the edge cases it's going to be significantly faster than what V8 already does. For example, OP's deepClone function doesn't handle arrays with non-element properties correctly, to say nothing of recursive data structures. |
@bnoordhuis not going through an extra MessageChannel might be a good start https://github.com/nodejs/node/blob/main/lib/internal/structured_clone.js but even so - I wouldn't assume V8's implementation is particularly fast, that they optimized it a lot or that it's too difficult to beat (in a spec compliant manner obviously), I may be wrong. |
Oh sure, no disagreement from me there, but OP's suggestion is to shortcut the algorithm and I don't think that's going to be a fruitful endeavor. Adding cycle detection alone probably wipes out much of the performance advantage, never mind everything else. (I recently wrote a golang library that speaks V8's serde protocol so this is all sparking fresh on my mind.) |
I would expect more often than not the input is going to be a complex object, not a primitive which a fast path could be added for. I don't think there's much we can do here beyond pulling out the clone algorithm so you don't need to construct a the MessageChannel to pass through it, and then start looking at what can be done in V8. One could possibly inform the V8 optimizer with shape information to compute ahead if other code passed in a shape known to not have pitfalls like circular references and skip the checks. That would be a complicated effort though. I'm happy to mentor anyone wanting to take it on when I can, just be prepared for it to take several weeks or even months. 😅 |
I wrote up a simplified implementation of structuredClone in native and it improves the performance a bit:
Would be even better if we just don't use ports. But baby steps. That would also allow us to support structuredClone in customized snapshots (I am fairly certain that there was an issue for this - cannot find it though - or someone asked me about this). |
Thanks for getting that change in so quickly @joyeecheung. Do you think this issue should be closed and a separate one should be reopened for future work? |
With the snippet in the OP I got this:
So I think there are still work to be done to make structured clone more performant. One is #50330 (comment) and another is using fast APIs when there's no transfer list. But I think even with those there's still a lot for structured clone to catch up with other alternatives. |
There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the
never-stale
|
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment. For more information on how the project manages feature requests, please consult the feature request management document. |
What is the problem this feature will solve?
structuredClone
performance is quite slow compared to alternative object copying(while those also have their downsides in some capacity, the performance loss is worth polyfilling at the moment).I also noticed in the source code there's already a TODO about performance and it would also have WebStreams ReadableStream
There was another discussion here which I thought was insightful #34355 (comment)
Consider code like the following:
The output gives me:
What is the feature you are proposing to solve the problem?
Increase throughput on
structuredClone
callsWhat alternatives have you considered?
Polyfilling my own object clone methods
The text was updated successfully, but these errors were encountered: