socket.io
socket.io copied to clipboard
Pooled memory management currently impossible with socket.io interface
Is your feature request related to a problem? Please describe.
Currently, my application maintains reasonably large Buffer objects in a pool, reusing them to avoid tons of GC contention. However, socket.io is a bit of a black hole for those buffers. Once we .emit one, we don't know when it has been actually sent to the client and "released" from socket.io. Thus we cannot reuse those buffers and they must be GC. We're spending upwards of 60% of our CPU time in GC on these objects we send to socket.io right now.
Describe the solution you'd like
I would like socket.io to expose a released event that is emitted every time socket.io has successfully sent an object to the client. It would contain as its body the object that was released. These could be buffers, classes, POJO, etc., anything that lives on the heap and socket.io is now done with.
Describe alternatives you've considered I don't see any alternatives at this time.
Additional context I'd be very happy to implement this feature myself but before I spend time on it, I want to know that the solution I proposed is acceptable and the PR would likely to be merged.
@darrachequesne is this something you would be open to?
Hi!
Could the drain event of the low-level connection fit your use case?
io.on("connection", (socket) => {
socket.conn.on("drain", () => {
// clean up
});
});
Reference: https://socket.io/docs/v4/server-api/#socketconn
@darrachequesne No; that happens when the low-level buffer that's internalized to the socket is drained. It doesn't happen when socket.io is done with the buffer because it has been placed into that TCP buffer.
@darrachequesne sorry to ping again and poke, but this is a huge issue at my company and I'm mostly just looking for your acknowledgement that you find my approach acceptable. At that point I'll start working on it. I would rather not develop something that then turns into a patch I have to apply to socket.io forever on my server.
Yes, that sounds like a reasonable use case 👍 could you please open a PR?
Hi @darrachequesne, the PR has been opened now with no engagement for several months. Can we do something to resolve this and get it merged?
Hi!
Sorry for the delay. To be honest, I think the suggested change needlessly complicates the source code, for a really specific use case, which is why I'm not sure about merging it.