net: add writeQueueSize

[theanarkh]

add writeQueueSize for net.js.

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

Review requested:

• @nodejs/net

[ShogunPanda]

LGTM!

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/45914/

[lpinca]

Is there a way to test the feature without this hack? I mean, how would socket.writeQueueSize be used in the real world? The test should simulate that.

[theanarkh]

Yes, it is difficult to write the test, and i think this test is not good.writeQueueSize means how many bytes are waiting to write in libuv. and add socket._handle.writeQueueSize to the count of bytesWritten maybe a good idea. I'll take a look later, thanks !

[lpinca]

I think it makes more sense to undeprecate socket.bufferSize and add socket._handle.writeQueueSize to the count instead of adding a new field.

[theanarkh]

@pinca I've read the source code. I think 'bytesWritten' is equal to the length of all written bytes, which includes the data has been sent and data is waiting to be sent. And 'writeQueueSize' indicates the length of bytes waiting to be sent in Libuv.

[lpinca]

@theanarkh I was talking about socket.bufferSize and I meant something like this:

diff --git a/lib/net.js b/lib/net.js
index eaa5e594e5..ce02019a75 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -637,7 +637,7 @@ ObjectDefineProperty(Socket.prototype, 'bufferSize', {
   __proto__: null,
   get: function() {
     if (this._handle) {
-      return this.writableLength;
+      return this._handle.writeQueueSize + this.writableLength;
     }
   }
 });

instead of creating a new property.

[theanarkh]

Oh. Thanks. bufferSize = this.bytesWritten - this._bytesDispatched + this._handle.writeQueueSize is it right ?

[lpinca]

It seems socket.bytesWritten = socket._bytesDispatched + socket.<writableBuffer byte length> + <length of data written before connect> so yes it should be correct, but I think it is unnecessarily complex.

socket.writableLength should take into account socket.<writableBuffer byte length> + <length of data written before connect>

$ node
Welcome to Node.js v18.7.0.
Type ".help" for more information.
> var socket = net.createConnection({ port: 8080, lookup() {} });
undefined
> socket.write('foo')
true
> socket.bytesWritten
3
> socket.writableLength
3
>

I would just use socket._handle.writeQueueSize + socket.writableLength.

I also think that socket.bytesWritten could be simplified by using socket.writableLength but that is a different issue.

[github-actions]

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.