net: remove redundant code from _writeGeneric() - #18429
Conversation
Sorry, something went wrong.
The encoding is already handled by `Writable.prototype.write()`.
Sorry, something went wrong.
mcollina
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
cjihrig
left a comment
There was a problem hiding this comment.
LGTM. We might want to run CITGM though, in case any popular modules are calling _writeGeneric() directly.
Sorry, something went wrong.
Sorry, something went wrong.
|
I can't spot any noticeable difference from the previous CITGM run. On a side note: I noticed that Socket.prototpe._writeGeneric() is a "hot" function profiling a basic net echo server (10k roundtrips of 64 B - binary data). Instantiating a WriteWrap is expensive but I'm not sure if it's that or if we can do anything about this. |
Sorry, something went wrong.
_writeGeneric is one of hottest functions in core. I think part of the problem is it being 'generic', as it's in the path for both _write and _writev flows. This function has to instantiate a WriteWrap. |
Sorry, something went wrong.
|
Yes I think that instantiating WriteWrap is what makes it slow but I may be wrong. |
Sorry, something went wrong.
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
@lpinca does this apply to 6.x or 8.x? |
Sorry, something went wrong.
|
Yes, it applies to both 6.x and 8.x. Let me know if it doesn't apply cleanly. |
Sorry, something went wrong.
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: #18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by `Writable.prototype.write()`. PR-URL: nodejs#18429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The encoding is already handled by Writable.prototype.write().
Checklist
Affected core subsystem(s)
net