colyseus / uWebSockets-express

Express API compatibility layer for uWebSockets.js
https://npmjs.com/package/uwebsockets-express
MIT License
56 stars 13 forks source link

Corking #18

Closed romanzy313 closed 1 year ago

romanzy313 commented 2 years ago

You need to implement corking. Basically wrap the ServerResponse in a callback inside cork.

    res.cork(() => {
      res.writeStatus(...)
      res.writeHeader(...)
      res.write(...);
    })

Its easy to do and will improve performance significantly. If you are having troubles, I can open a PR.

From the lib: Corking a response is a performance improvement in both CPU and network, as you ready the IO system for writing multiple chunks at once. By default, you're corked in the immediately executing top portion of the route handler. In all other cases, such as when returning from await, or when being called back from an async database request or anything that isn't directly executing in the route handler, you'll want to cork before calling writeStatus, writeHeader or just write. Corking takes a callback in which you execute the writeHeader, writeStatus and such calls, in one atomic IO operation. This is important, not only for TCP but definitely for TLS where each write would otherwise result in one TLS block being sent off, each with one send syscall.

endel commented 2 years ago

Hi @romanzy-1612, a PR would be very welcome for this!

hunkydoryrepair commented 1 year ago

image any movement on this? it's basically blocking us from upgrading to node 18 or colyseus 0.15

endel commented 1 year ago

Hi @hunkydoryrepair, I've never managed to reproduce this, can you share how to reproduce it? 🙏

Always-Pixels commented 1 year ago

Hi Endel, I just created a PR for this. Also, bumped the version of "uWebSockets.js" to v20 (from v19) to support Node 18.

hunkydoryrepair commented 1 year ago

Yeah, the issue appears when uWebSockets v20 is used. v19 doesn't work with Node 18 (just crashes), and our CICD platform is removing support for Node 16 in 2 months.

It actually works, but floods our logs with warning messages.

endel commented 1 year ago

Thank you @Always-Pixels 🙏 https://github.com/colyseus/uWebSockets-express/pull/25 has been merged and published!