Skip to content

Commit c2835e5

Browse files
aks-addaleax
authored andcommitted
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: #19527 Refs: #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 c46e36b commit c2835e5

File tree

4 files changed

+108
-107
lines changed

4 files changed

+108
-107
lines changed

lib/internal/http2/core.js

Lines changed: 14 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@ const binding = process.binding('http2');
1212
const { FileHandle } = process.binding('fs');
1313
const { StreamPipe } = internalBinding('stream_pipe');
1414
const assert = require('assert');
15-
const { Buffer } = require('buffer');
1615
const EventEmitter = require('events');
1716
const net = require('net');
1817
const tls = require('tls');
1918
const util = require('util');
2019
const fs = require('fs');
2120
const {
22-
errnoException,
2321
codes: {
2422
ERR_HTTP2_ALTSVC_INVALID_ORIGIN,
2523
ERR_HTTP2_ALTSVC_LENGTH,
@@ -107,8 +105,13 @@ const {
107105
validateTimerDuration,
108106
refreshFnSymbol
109107
} = require('internal/timers');
108+
const {
109+
createWriteWrap,
110+
writeGeneric,
111+
writevGeneric
112+
} = require('internal/stream_base_commons');
110113

111-
const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap');
114+
const { ShutdownWrap } = process.binding('stream_wrap');
112115
const { constants, nameForErrorCode } = binding;
113116

114117
const NETServer = net.Server;
@@ -1429,28 +1432,6 @@ class ClientHttp2Session extends Http2Session {
14291432
}
14301433
}
14311434

1432-
function createWriteReq(req, handle, data, encoding) {
1433-
switch (encoding) {
1434-
case 'utf8':
1435-
case 'utf-8':
1436-
return handle.writeUtf8String(req, data);
1437-
case 'ascii':
1438-
return handle.writeAsciiString(req, data);
1439-
case 'ucs2':
1440-
case 'ucs-2':
1441-
case 'utf16le':
1442-
case 'utf-16le':
1443-
return handle.writeUcs2String(req, data);
1444-
case 'latin1':
1445-
case 'binary':
1446-
return handle.writeLatin1String(req, data);
1447-
case 'buffer':
1448-
return handle.writeBuffer(req, data);
1449-
default:
1450-
return handle.writeBuffer(req, Buffer.from(data, encoding));
1451-
}
1452-
}
1453-
14541435
function trackWriteState(stream, bytes) {
14551436
const session = stream[kSession];
14561437
stream[kState].writeQueueSize += bytes;
@@ -1674,16 +1655,12 @@ class Http2Stream extends Duplex {
16741655
if (!this.headersSent)
16751656
this[kProceed]();
16761657

1677-
const handle = this[kHandle];
1678-
const req = new WriteWrap();
1658+
const req = createWriteWrap(this[kHandle], afterDoStreamWrite);
16791659
req.stream = this[kID];
1680-
req.handle = handle;
16811660
req.callback = cb;
1682-
req.oncomplete = afterDoStreamWrite;
1683-
req.async = false;
1684-
const err = createWriteReq(req, handle, data, encoding);
1685-
if (err)
1686-
return this.destroy(errnoException(err, 'write', req.error), cb);
1661+
1662+
writeGeneric(this, req, data, encoding, cb);
1663+
16871664
trackWriteState(this, req.bytes);
16881665
}
16891666

@@ -1711,22 +1688,12 @@ class Http2Stream extends Duplex {
17111688
if (!this.headersSent)
17121689
this[kProceed]();
17131690

1714-
const handle = this[kHandle];
1715-
const req = new WriteWrap();
1691+
var req = createWriteWrap(this[kHandle], afterDoStreamWrite);
17161692
req.stream = this[kID];
1717-
req.handle = handle;
17181693
req.callback = cb;
1719-
req.oncomplete = afterDoStreamWrite;
1720-
req.async = false;
1721-
const chunks = new Array(data.length << 1);
1722-
for (var i = 0; i < data.length; i++) {
1723-
const entry = data[i];
1724-
chunks[i * 2] = entry.chunk;
1725-
chunks[i * 2 + 1] = entry.encoding;
1726-
}
1727-
const err = handle.writev(req, chunks);
1728-
if (err)
1729-
return this.destroy(errnoException(err, 'write', req.error), cb);
1694+
1695+
writevGeneric(this, req, data, cb);
1696+
17301697
trackWriteState(this, req.bytes);
17311698
}
17321699

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
@@ -46,12 +46,17 @@ const { TCP, constants: TCPConstants } = process.binding('tcp_wrap');
4646
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
4747
const { TCPConnectWrap } = process.binding('tcp_wrap');
4848
const { PipeConnectWrap } = process.binding('pipe_wrap');
49-
const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap');
49+
const { ShutdownWrap } = process.binding('stream_wrap');
5050
const {
5151
newAsyncId,
5252
defaultTriggerAsyncIdScope,
5353
symbols: { async_id_symbol }
5454
} = require('internal/async_hooks');
55+
const {
56+
createWriteWrap,
57+
writevGeneric,
58+
writeGeneric
59+
} = require('internal/stream_base_commons');
5560
const errors = require('internal/errors');
5661
const {
5762
ERR_INVALID_ADDRESS_FAMILY,
@@ -740,38 +745,15 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) {
740745
return false;
741746
}
742747

743-
var req = new WriteWrap();
744-
req.handle = this._handle;
745-
req.oncomplete = afterWrite;
746-
req.async = false;
747-
var err;
748-
749-
if (writev) {
750-
var allBuffers = data.allBuffers;
751-
var chunks;
752-
var i;
753-
if (allBuffers) {
754-
chunks = data;
755-
for (i = 0; i < data.length; i++)
756-
data[i] = data[i].chunk;
757-
} else {
758-
chunks = new Array(data.length << 1);
759-
for (i = 0; i < data.length; i++) {
760-
var entry = data[i];
761-
chunks[i * 2] = entry.chunk;
762-
chunks[i * 2 + 1] = entry.encoding;
763-
}
764-
}
765-
err = this._handle.writev(req, chunks, allBuffers);
766-
767-
// Retain chunks
768-
if (err === 0) req._chunks = chunks;
769-
} else {
770-
err = createWriteReq(req, this._handle, data, encoding);
771-
}
748+
var ret;
749+
var req = createWriteWrap(this._handle, afterWrite);
750+
if (writev)
751+
ret = writevGeneric(this, req, data, cb);
752+
else
753+
ret = writeGeneric(this, req, data, encoding, cb);
772754

773-
if (err)
774-
return this.destroy(errnoException(err, 'write', req.error), cb);
755+
// Bail out if handle.write* returned an error
756+
if (ret) return ret;
775757

776758
this._bytesDispatched += req.bytes;
777759

@@ -794,34 +776,6 @@ Socket.prototype._write = function(data, encoding, cb) {
794776
this._writeGeneric(false, data, encoding, cb);
795777
};
796778

797-
function createWriteReq(req, handle, data, encoding) {
798-
switch (encoding) {
799-
case 'latin1':
800-
case 'binary':
801-
return handle.writeLatin1String(req, data);
802-
803-
case 'buffer':
804-
return handle.writeBuffer(req, data);
805-
806-
case 'utf8':
807-
case 'utf-8':
808-
return handle.writeUtf8String(req, data);
809-
810-
case 'ascii':
811-
return handle.writeAsciiString(req, data);
812-
813-
case 'ucs2':
814-
case 'ucs-2':
815-
case 'utf16le':
816-
case 'utf-16le':
817-
return handle.writeUcs2String(req, data);
818-
819-
default:
820-
return handle.writeBuffer(req, Buffer.from(data, encoding));
821-
}
822-
}
823-
824-
825779
protoGetter('bytesWritten', function bytesWritten() {
826780
var bytes = this._bytesDispatched;
827781
const state = this._writableState;

node.gyp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@
144144
'lib/internal/v8_prof_polyfill.js',
145145
'lib/internal/v8_prof_processor.js',
146146
'lib/internal/vm/Module.js',
147+
'lib/internal/stream_base_commons.js',
147148
'lib/internal/streams/lazy_transform.js',
148149
'lib/internal/streams/async_iterator.js',
149150
'lib/internal/streams/BufferList.js',

0 commit comments

Comments
 (0)