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

Significant performance optimizations possible in requestResponseMessage #647

Open
josephrocca opened this issue Nov 9, 2023 · 4 comments

Comments

@josephrocca
Copy link

I'm using Comlink on a Deno server that is handling a lot of traffic - I'm just passing a string to the worker, and then the worker is sending back ArrayBuffers (I'm using Comlink.transfer for sending those, of course), and it turns out that Comlink has become a bottleneck on the main thread - specifically this code:

image

I'd have thought that V8 would have some magic optimizations to make the function creation here not as expensive as it seems to be. It looks like it's creating a "fresh" function each time, and then isn't able to optimize it since it only gets called once, and so simple stuff like !ev.data || !ev.data.id runs slow because it's basically running in "interpreted" mode. That's my guess here, anyway.

I'm wondering if you'd welcome a pull request which optimizes this, and if so, do you have any preferred approach? My thinking here is that you'd just have a single function (rather than creating a new one for every request), and it uses a Map which maps an id to its resolver.

@Vitaminaq
Copy link

Vitaminaq commented Nov 27, 2023

+1
When I perform a lot of operations during a certain period of time, the performance degradation is very severe

The createProxy method is also applicable

@qdhuxp
Copy link

qdhuxp commented Dec 1, 2023

+1
I event frequently get OOM from this function of "resolve(ev.data);". I know my data is large, and I don't know how to avoid it through coding.

@mhofman
Copy link
Contributor

mhofman commented Dec 1, 2023

I'm not related to this project, but most likely I would have a single listener per ep, and a Map of pending calls, associating the id to the resolver. That would ensure there is only a single event listener executed per received message.

@benjamind
Copy link
Collaborator

benjamind commented Dec 14, 2023

Hey folks, sorry for the slow replies here. Myself and @surma are not super active on this project (looking for maintainers if you're interested!).

Definitely open to a PR here to explore optimizations. Unless there's some specific reason we're not already taking this approach @surma ?

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

Successfully merging a pull request may close this issue.

5 participants