Skip to content

Commit 9a68c8c

Browse files
darrachequesnedarrachequesne
committed
perf(websocket): use bound callbacks
Instead of allocating one temporary function for each WebSocket `send()` call. Regarding the test removal, the permessage-deflate threshold was implemented in the "ws" package in [1], so it's not needed anymore. [1]: websockets/ws@6b3904b
1 parent 62f59b6 commit 9a68c8c

File tree

2 files changed

+38
-105
lines changed

2 files changed

+38
-105
lines changed

‎lib/transports/websocket.ts‎

Lines changed: 38 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Transport } from "../transport";
22
import debugModule from "debug";
3+
import type { Packet, RawData } from "engine.io-parser";
34

45
const debug = debugModule("engine:ws");
56

@@ -45,52 +46,27 @@ export class WebSocket extends Transport {
4546
return true;
4647
}
4748

48-
/**
49-
* Writes a packet payload.
50-
*
51-
* @param {Array} packets
52-
* @api private
53-
*/
54-
send(packets) {
49+
send(packets: Packet[]) {
5550
this.writable = false;
5651

5752
for (let i = 0; i < packets.length; i++) {
5853
const packet = packets[i];
5954
const isLast = i + 1 === packets.length;
6055

61-
// always creates a new object since ws modifies it
62-
const opts: { compress?: boolean } = {};
63-
if (packet.options) {
64-
opts.compress = packet.options.compress;
65-
}
66-
67-
const onSent = (err) => {
68-
if (err) {
69-
return this.onError("write error", err.stack);
70-
} else if (isLast) {
71-
this.writable = true;
72-
this.emit("drain");
73-
}
74-
};
75-
76-
const send = (data) => {
77-
if (this.perMessageDeflate) {
78-
const len =
79-
"string" === typeof data ? Buffer.byteLength(data) : data.length;
80-
if (len < this.perMessageDeflate.threshold) {
81-
opts.compress = false;
82-
}
83-
}
84-
debug('writing "%s"', data);
85-
this.socket.send(data, opts, onSent);
86-
};
87-
8856
if (this._canSendPreEncodedFrame(packet)) {
8957
// the WebSocket frame was computed with WebSocket.Sender.frame()
9058
// see https://github.com/websockets/ws/issues/617#issuecomment-283002469
91-
this.socket._sender.sendFrame(packet.options.wsPreEncodedFrame, onSent);
59+
this.socket._sender.sendFrame(
60+
// @ts-ignore
61+
packet.options.wsPreEncodedFrame,
62+
isLast ? this._onSentLast : this._onSent
63+
);
9264
} else {
93-
this.parser.encodePacket(packet, this.supportsBinary, send);
65+
this.parser.encodePacket(
66+
packet,
67+
this.supportsBinary,
68+
isLast ? this._doSendLast : this._doSend
69+
);
9470
}
9571
}
9672
}
@@ -100,20 +76,39 @@ export class WebSocket extends Transport {
10076
* @param packet
10177
* @private
10278
*/
103-
private _canSendPreEncodedFrame(packet) {
79+
private _canSendPreEncodedFrame(packet: Packet) {
10480
return (
10581
!this.perMessageDeflate &&
10682
typeof this.socket?._sender?.sendFrame === "function" &&
83+
// @ts-ignore
10784
packet.options?.wsPreEncodedFrame !== undefined
10885
);
10986
}
11087

111-
/**
112-
* Closes the transport.
113-
*
114-
* @api private
115-
*/
116-
doClose(fn) {
88+
private _doSend = (data: RawData) => {
89+
this.socket.send(data, this._onSent);
90+
};
91+
92+
private _doSendLast = (data: RawData) => {
93+
this.socket.send(data, this._onSentLast);
94+
};
95+
96+
private _onSent = (err?: Error) => {
97+
if (err) {
98+
this.onError("write error", err.stack);
99+
}
100+
};
101+
102+
private _onSentLast = (err?: Error) => {
103+
if (err) {
104+
this.onError("write error", err.stack);
105+
} else {
106+
this.writable = true;
107+
this.emit("drain");
108+
}
109+
};
110+
111+
doClose(fn?: () => void) {
117112
debug("closing");
118113
this.socket.close();
119114
fn && fn();

‎test/server.js‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -3337,68 +3337,6 @@ describe("server", () => {
33373337
});
33383338
});
33393339

3340-
describe("permessage-deflate", () => {
3341-
it("should set threshold", function (done) {
3342-
if (process.env.EIO_WS_ENGINE === "uws") {
3343-
return this.skip();
3344-
}
3345-
const engine = listen(
3346-
{ transports: ["websocket"], perMessageDeflate: { threshold: 0 } },
3347-
(port) => {
3348-
engine.on("connection", (conn) => {
3349-
const socket = conn.transport.socket;
3350-
const send = socket.send;
3351-
socket.send = (data, opts, callback) => {
3352-
socket.send = send;
3353-
socket.send(data, opts, callback);
3354-
3355-
expect(opts.compress).to.be(true);
3356-
conn.close();
3357-
done();
3358-
};
3359-
3360-
const buf = Buffer.allocUnsafe(100);
3361-
for (let i = 0; i < buf.length; i++) buf[i] = i % 0xff;
3362-
conn.send(buf, { compress: true });
3363-
});
3364-
new ClientSocket(`http://localhost:${port}`, {
3365-
transports: ["websocket"],
3366-
});
3367-
}
3368-
);
3369-
});
3370-
3371-
it("should not compress when the byte size is below threshold", function (done) {
3372-
if (process.env.EIO_WS_ENGINE === "uws") {
3373-
return this.skip();
3374-
}
3375-
const engine = listen(
3376-
{ transports: ["websocket"], perMessageDeflate: true },
3377-
(port) => {
3378-
engine.on("connection", (conn) => {
3379-
const socket = conn.transport.socket;
3380-
const send = socket.send;
3381-
socket.send = (data, opts, callback) => {
3382-
socket.send = send;
3383-
socket.send(data, opts, callback);
3384-
3385-
expect(opts.compress).to.be(false);
3386-
conn.close();
3387-
done();
3388-
};
3389-
3390-
const buf = Buffer.allocUnsafe(100);
3391-
for (let i = 0; i < buf.length; i++) buf[i] = i % 0xff;
3392-
conn.send(buf, { compress: true });
3393-
});
3394-
new ClientSocket(`http://localhost:${port}`, {
3395-
transports: ["websocket"],
3396-
});
3397-
}
3398-
);
3399-
});
3400-
});
3401-
34023340
describe("extraHeaders", function () {
34033341
this.timeout(5000);
34043342

0 commit comments

Comments
 (0)