Closed DiegoRBaquero closed 3 years ago
@DiegoRBaquero Agreed, this is important.
From your testing, do you know which objects are using the most memory?
@feross Took a heapdump: https://webseed.btorrent.xyz/heapdump-1587638310.868854.heapsnapshot
Sintel in the tracker memory:
Other ways to approach this problem:
@feross
Tested uWS and it was a HUGE improvement (from 450MB to 30MB RAM for 800+ peers). The only thing lost is the real IP address (which is never used, actually protects peers' privacy) and the full headers of the request (Again, protects peers' privacy from trackers' operators but would not allow filtering or blocking).
@alxhotel @jakefb You might want to check this out.
That looks interesting! Nice work! Have you looked at the heap when using µWS? Do you thing we could make significant memory optimizations in our codebase?
@yciabaud I didn't even bother, but I could get a heapshot if you'd like to see if there's anything else we can improve.
We are only left with clearing 1-dead-peer-only zombie infoHashes (https://github.com/feross/bittorrent-tracker/issues/164)
By the way, guys, could I get comments on: https://medium.com/@diegorbaquero/%C2%B5ws-as-your-next-websocket-library-d34209686357#.twgf337zc
Thanks
@DiegoRBaquero interesting. Did you try to disable permessage-deflate in both ws
and uws
? I know that the overhead for ws
is pretty big. Also to reduce memory usage a little you can try to null the upgradeReq
object when the 'connection' event is emitted, uws
does this under the hood.
wss.on('connection', (ws) => {
ws.upgradeReq = null;
});
I'm actually not sure if this works as expected.
It might also be interesting to see if installing ws
from master makes any difference. There are some improvements, including breaking changes, that have not been released yet.
npm i websockets/ws bufferutil
I also see that the clientTracking
option for ws
is set to true
(default value). It seems that this lib uses its own way to track clients so you may want to set it to false
.
@lpinca Had not try either the options you describe, had I seen them in the docs por better performance I would have tried them before.
I'll be monitoring uWS performance for the weekend and PR the change on monday if everything is cool.
@alexhultman
I'm actually not sure if this works as expected.
I'm not sure because there might be other references that prevent the upgradeReq
object from being GC'd and yes it's not exactly the same of uws
as ws
uses net.Socket
so that object (net.Socket
) will never be GC'd.
Well, you certainly seem to know a lot about the upgradeReq since you have been insinuating all over that ws could easily scale to µWS levels if upgradeReq was removed.
I've never said that ws
could scale at the same levels of uws, if I did show me where please. I've only said that nulling the upgradeReq
object in ws
could reduce memory usage and that your numbers are wrong and they still are. permessage-deflate seems to have serious issues in ws
, see https://github.com/websockets/ws/issues/804.
Instead of acting like some sort of dictator inside of the Primus camp? I have told you ages ago that you have to remove the upgradeReq in the µWS transformer
This makes me sad... Did you actually read why we can't do that? I guess not. This is what we are doing now: https://github.com/primus/primus/blob/uws/transformers/uws/server.js#L92-L95. If you know how to improve it further given our constraints please tell us or send a pr. You have banned me from uWebSockets but I haven't banned you from Primus because I think every contribution is valuable even the ones you don't agree with and the ones that are not easy to handle but I'm the dictator, ok. Want to discuss about Primus? I'm all for it but it's not relevant to this issue. Let's do it somewhere else.
I've only suggested how to improve ws performance here.
@DiegoRBaquero can we move forward on this? Do you still think we should switch to uWS? Did you test disabling permessage-deflate?
@alxhotel @jakefb Could you guys update to latest and test memory consumption with per message deflate and client tracking off in ws?
@DiegoRBaquero Done! Results from the last 12 hours are aprox: 5x less memory consumption and 3x more network bandwidth.
@alxhotel Were there any restarts due to errors?
I think I caused a restart when loading /stats, which evicts old peers.
If you want a sweetspot between low memory usage and low network usage you can spend some time optimizing the WebTorrent protocol. I assume it is based on JSON right now and that is very ineffective vs something binary. I bet you can improve it a ton in size.
@alexhultman We could try switching back to raw bencoding since that's what the other tracker implementations (http and udp) use. But we would need to consider a transition period where we supported both bencoding/json for several months until clients are updated.
@DiegoRBaquero Yep. See issue #205
I seeded a 4G file, it had token almost 8G memory.
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?
Although we have made several improvements and clear memory leaks, everything can always be improved.
In order for the webtorrent-webrtc-based projects to reach more users easily, the trackers need to be able to scale with greater performance.
At the time of writing, 1K connected users go for about 500MB of RAM and each connected user is around 0.45MB (BTorrent's tracker)