Skip to content

Commit 5894cd3

Browse files
aks-BridgeAR
aks-
authored and
BridgeAR
committed
lib: merge stream code for http2 streams & net.Socket
Squashed from: - lib: separate writev responsibilities from writeGeneric - lib: fix calling of cb twice - lib: extract streamId out of stream_base to caller - lib: add symbols instead of methods to hide impl details - lib: remove unneeded lines - lib: use Object.assign instead of apply - lib: rename mixin StreamBase to StreamSharedMethods - lib: use stream shared funcs as top level instead of properties of prototypes - lib: mv lib/internal/stream_shared_methods.js lib/internal/stream_base_commons.js - lib: add comment for readability - lib: refactor _writev in Http2Stream - lib: rephrase comment - lib: revert usage of const,let for perf reasons PR-URL: nodejs#19527 Refs: nodejs#19060 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent c6fe531 commit 5894cd3

4 files changed

Lines changed: 108 additions & 106 deletions

File tree

‎lib/internal/http2/core.js‎

Lines changed: 14 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ const { async_id_symbol } = process.binding('async_wrap');
88
const http = require('http');
99
const binding = process.binding('http2');
1010
const assert = require('assert');
11-
const { Buffer } = require('buffer');
1211
const EventEmitter = require('events');
1312
const net = require('net');
1413
const tls = require('tls');
@@ -60,8 +59,13 @@ const {
6059
enroll,
6160
unenroll
6261
} = require('timers');
62+
const {
63+
createWriteWrap,
64+
writeGeneric,
65+
writevGeneric
66+
} = require('internal/stream_base_commons');
6367

64-
const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap');
68+
const { ShutdownWrap } = process.binding('stream_wrap');
6569
const { constants, nameForErrorCode } = binding;
6670

6771
const NETServer = net.Server;
@@ -1395,28 +1399,6 @@ class ClientHttp2Session extends Http2Session {
13951399
}
13961400
}
13971401

1398-
function createWriteReq(req, handle, data, encoding) {
1399-
switch (encoding) {
1400-
case 'utf8':
1401-
case 'utf-8':
1402-
return handle.writeUtf8String(req, data);
1403-
case 'ascii':
1404-
return handle.writeAsciiString(req, data);
1405-
case 'ucs2':
1406-
case 'ucs-2':
1407-
case 'utf16le':
1408-
case 'utf-16le':
1409-
return handle.writeUcs2String(req, data);
1410-
case 'latin1':
1411-
case 'binary':
1412-
return handle.writeLatin1String(req, data);
1413-
case 'buffer':
1414-
return handle.writeBuffer(req, data);
1415-
default:
1416-
return handle.writeBuffer(req, Buffer.from(data, encoding));
1417-
}
1418-
}
1419-
14201402
function trackWriteState(stream, bytes) {
14211403
const session = stream[kSession];
14221404
stream[kState].writeQueueSize += bytes;
@@ -1637,16 +1619,12 @@ class Http2Stream extends Duplex {
16371619
if (!this.headersSent)
16381620
this[kProceed]();
16391621

1640-
const handle = this[kHandle];
1641-
const req = new WriteWrap();
1622+
const req = createWriteWrap(this[kHandle], afterDoStreamWrite);
16421623
req.stream = this[kID];
1643-
req.handle = handle;
16441624
req.callback = cb;
1645-
req.oncomplete = afterDoStreamWrite;
1646-
req.async = false;
1647-
const err = createWriteReq(req, handle, data, encoding);
1648-
if (err)
1649-
return this.destroy(errors.errnoException(err, 'write', req.error), cb);
1625+
1626+
writeGeneric(this, req, data, encoding, cb);
1627+
16501628
trackWriteState(this, req.bytes);
16511629
}
16521630

@@ -1674,22 +1652,12 @@ class Http2Stream extends Duplex {
16741652
if (!this.headersSent)
16751653
this[kProceed]();
16761654

1677-
const handle = this[kHandle];
1678-
const req = new WriteWrap();
1655+
var req = createWriteWrap(this[kHandle], afterDoStreamWrite);
16791656
req.stream = this[kID];
1680-
req.handle = handle;
16811657
req.callback = cb;
1682-
req.oncomplete = afterDoStreamWrite;
1683-
req.async = false;
1684-
const chunks = new Array(data.length << 1);
1685-
for (var i = 0; i < data.length; i++) {
1686-
const entry = data[i];
1687-
chunks[i * 2] = entry.chunk;
1688-
chunks[i * 2 + 1] = entry.encoding;
1689-
}
1690-
const err = handle.writev(req, chunks);
1691-
if (err)
1692-
return this.destroy(errors.errnoException(err, 'write', req.error), cb);
1658+
1659+
writevGeneric(this, req, data, cb);
1660+
16931661
trackWriteState(this, req.bytes);
16941662
}
16951663

‎lib/internal/stream_base_commons.js‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
'use strict';
2+
3+
const { Buffer } = require('buffer');
4+
const errors = require('internal/errors');
5+
const { WriteWrap } = process.binding('stream_wrap');
6+
7+
const errnoException = errors.errnoException;
8+
9+
function handleWriteReq(req, data, encoding) {
10+
const { handle } = req;
11+
12+
switch (encoding) {
13+
case 'buffer':
14+
return handle.writeBuffer(req, data);
15+
case 'latin1':
16+
case 'binary':
17+
return handle.writeLatin1String(req, data);
18+
case 'utf8':
19+
case 'utf-8':
20+
return handle.writeUtf8String(req, data);
21+
case 'ascii':
22+
return handle.writeAsciiString(req, data);
23+
case 'ucs2':
24+
case 'ucs-2':
25+
case 'utf16le':
26+
case 'utf-16le':
27+
return handle.writeUcs2String(req, data);
28+
default:
29+
return handle.writeBuffer(req, Buffer.from(data, encoding));
30+
}
31+
}
32+
33+
function createWriteWrap(handle, oncomplete) {
34+
var req = new WriteWrap();
35+
36+
req.handle = handle;
37+
req.oncomplete = oncomplete;
38+
req.async = false;
39+
40+
return req;
41+
}
42+
43+
function writevGeneric(self, req, data, cb) {
44+
var allBuffers = data.allBuffers;
45+
var chunks;
46+
var i;
47+
if (allBuffers) {
48+
chunks = data;
49+
for (i = 0; i < data.length; i++)
50+
data[i] = data[i].chunk;
51+
} else {
52+
chunks = new Array(data.length << 1);
53+
for (i = 0; i < data.length; i++) {
54+
var entry = data[i];
55+
chunks[i * 2] = entry.chunk;
56+
chunks[i * 2 + 1] = entry.encoding;
57+
}
58+
}
59+
var err = req.handle.writev(req, chunks, allBuffers);
60+
61+
// Retain chunks
62+
if (err === 0) req._chunks = chunks;
63+
64+
if (err)
65+
return self.destroy(errnoException(err, 'write', req.error), cb);
66+
}
67+
68+
function writeGeneric(self, req, data, encoding, cb) {
69+
var err = handleWriteReq(req, data, encoding);
70+
71+
if (err)
72+
return self.destroy(errnoException(err, 'write', req.error), cb);
73+
}
74+
75+
module.exports = {
76+
createWriteWrap,
77+
writevGeneric,
78+
writeGeneric
79+
};

‎lib/net.js‎

Lines changed: 14 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,15 @@ const { TCP, constants: TCPConstants } = process.binding('tcp_wrap');
4545
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
4646
const { TCPConnectWrap } = process.binding('tcp_wrap');
4747
const { PipeConnectWrap } = process.binding('pipe_wrap');
48-
const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap');
48+
const { ShutdownWrap } = process.binding('stream_wrap');
4949
const { async_id_symbol } = process.binding('async_wrap');
5050
const { newUid, defaultTriggerAsyncIdScope } = require('internal/async_hooks');
5151
const { nextTick } = require('internal/process/next_tick');
52+
const {
53+
createWriteWrap,
54+
writevGeneric,
55+
writeGeneric
56+
} = require('internal/stream_base_commons');
5257
const errors = require('internal/errors');
5358
const {
5459
ERR_INVALID_ADDRESS_FAMILY
@@ -714,38 +719,15 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) {
714719
return false;
715720
}
716721

717-
var req = new WriteWrap();
718-
req.handle = this._handle;
719-
req.oncomplete = afterWrite;
720-
req.async = false;
721-
var err;
722-
723-
if (writev) {
724-
var allBuffers = data.allBuffers;
725-
var chunks;
726-
var i;
727-
if (allBuffers) {
728-
chunks = data;
729-
for (i = 0; i < data.length; i++)
730-
data[i] = data[i].chunk;
731-
} else {
732-
chunks = new Array(data.length << 1);
733-
for (i = 0; i < data.length; i++) {
734-
var entry = data[i];
735-
chunks[i * 2] = entry.chunk;
736-
chunks[i * 2 + 1] = entry.encoding;
737-
}
738-
}
739-
err = this._handle.writev(req, chunks, allBuffers);
740-
741-
// Retain chunks
742-
if (err === 0) req._chunks = chunks;
743-
} else {
744-
err = createWriteReq(req, this._handle, data, encoding);
745-
}
722+
var ret;
723+
var req = createWriteWrap(this._handle, afterWrite);
724+
if (writev)
725+
ret = writevGeneric(this, req, data, cb);
726+
else
727+
ret = writeGeneric(this, req, data, encoding, cb);
746728

747-
if (err)
748-
return this.destroy(errnoException(err, 'write', req.error), cb);
729+
// Bail out if handle.write* returned an error
730+
if (ret) return ret;
749731

750732
this._bytesDispatched += req.bytes;
751733

@@ -768,34 +750,6 @@ Socket.prototype._write = function(data, encoding, cb) {
768750
this._writeGeneric(false, data, encoding, cb);
769751
};
770752

771-
function createWriteReq(req, handle, data, encoding) {
772-
switch (encoding) {
773-
case 'latin1':
774-
case 'binary':
775-
return handle.writeLatin1String(req, data);
776-
777-
case 'buffer':
778-
return handle.writeBuffer(req, data);
779-
780-
case 'utf8':
781-
case 'utf-8':
782-
return handle.writeUtf8String(req, data);
783-
784-
case 'ascii':
785-
return handle.writeAsciiString(req, data);
786-
787-
case 'ucs2':
788-
case 'ucs-2':
789-
case 'utf16le':
790-
case 'utf-16le':
791-
return handle.writeUcs2String(req, data);
792-
793-
default:
794-
return handle.writeBuffer(req, Buffer.from(data, encoding));
795-
}
796-
}
797-
798-
799753
protoGetter('bytesWritten', function bytesWritten() {
800754
var bytes = this._bytesDispatched;
801755
const state = this._writableState;

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@
143143
'lib/internal/v8_prof_polyfill.js',
144144
'lib/internal/v8_prof_processor.js',
145145
'lib/internal/vm/Module.js',
146+
'lib/internal/stream_base_commons.js',
146147
'lib/internal/streams/lazy_transform.js',
147148
'lib/internal/streams/BufferList.js',
148149
'lib/internal/streams/legacy.js',

0 commit comments

Comments
 (0)