From 6b85c6bc0b1b3e41dcf3350499af4b21ec43da5e Mon Sep 17 00:00:00 2001 From: CosminPerRam Date: Sat, 24 Feb 2024 18:26:48 +0200 Subject: [PATCH] perf: revise core class and make it cleaner (#548) * feat: rework core protocol a bit * feat: transform some strings to literals * feat: check if a param is a promise with instanceof * fix: shortest RTT not registering properly * feat: use another optional chaining * fix: grammatic error in comment * docs: add CHANGELOG line * fix: move a start time indicator closer to the source --- CHANGELOG.md | 1 + protocols/core.js | 133 +++++++++++++++++++++------------------------- 2 files changed, 62 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51ced60..9892568 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Assetto Corsa - Fixed how `state.numplayers` is set (By @podrivo #538) * TeamSpeak 2 - Fixed how `state.name` is set (By @podrivo #544) * Grand Theft Auto: San Andreas OpenMP - Fixed `state.players` returning an empty array (By @Focus04 #547) +* Perf: Re-write of the `core` class. ## 5.0.0-beta.2 * Fixed support for projects using `require`. diff --git a/protocols/core.js b/protocols/core.js index e717dee..033e5c2 100644 --- a/protocols/core.js +++ b/protocols/core.js @@ -31,7 +31,9 @@ export default class Core extends EventEmitter { // Runs a single attempt with a timeout and cleans up afterward async runOnceSafe () { - if (this.options.debug) { + const { debug, attemptTimeout } = this.options + + if (debug) { this.logger.debugEnabled = true } this.logger.prefix = 'Q#' + (uid++) @@ -41,16 +43,16 @@ export default class Core extends EventEmitter { this.logger.debug('Options:', this.options) let abortCall = null - this.abortedPromise = new Promise((resolve, reject) => { + this.abortedPromise = new Promise((_resolve, reject) => { abortCall = () => reject(new Error('Query is finished -- cancelling outstanding promises')) }).catch(() => { - // Make sure that if this promise isn't attached to, it doesn't throw a unhandled promise rejection + // Make sure that if this promise isn't attached to, it doesn't throw an unhandled promise rejection }) let timeout try { const promise = this.runOnce() - timeout = Promises.createTimeout(this.options.attemptTimeout, 'Attempt') + timeout = Promises.createTimeout(attemptTimeout, 'Attempt') const result = await Promise.race([promise, timeout]) this.logger.debug('Query was successful') return result @@ -58,9 +60,9 @@ export default class Core extends EventEmitter { this.logger.debug('Query failed with error', e) throw e } finally { - timeout && timeout.cancel() + timeout?.cancel() try { - abortCall() + abortCall?.() } catch (e) { this.logger.debug('Error during abort cleanup: ' + e.stack) } @@ -68,34 +70,29 @@ export default class Core extends EventEmitter { } async runOnce () { - const options = this.options + const { options, dnsResolver } = this + if (('host' in options) && !('address' in options)) { - const resolved = await this.dnsResolver.resolve(options.host, options.ipFamily, this.srvRecord) + const resolved = await dnsResolver.resolve(options.host, options.ipFamily, this.srvRecord) options.address = resolved.address - if (resolved.port) options.port = resolved.port + options.port ||= resolved.port } const state = new Results() - await this.run(state) - state.queryPort = options.port + state.queryPort = options.port // because lots of servers prefix with spaces to try to appear first state.name = (state.name || '').trim() - - if (!('connect' in state)) { - state.connect = '' + - (state.gameHost || this.options.host || this.options.address) + - ':' + - (state.gamePort || this.options.port) - } + state.connect = `${state.gameHost || options.host || options.address}:${state.gamePort || options.port}` state.ping = this.shortestRTT + delete state.gameHost delete state.gamePort this.logger.debug(log => { - log('Size of players array: ' + state.players.length) - log('Size of bots array: ' + state.bots.length) + log('Size of players array:', state.players.length) + log('Size of bots array:', state.bots.length) }) return state @@ -104,19 +101,20 @@ export default class Core extends EventEmitter { async run (/** Results */ state) {} /** Param can be a time in ms, or a promise (which will be timed) */ - registerRtt (param) { - if (param.then) { - const start = Date.now() - param.then(() => { - const end = Date.now() - const rtt = end - start - this.registerRtt(rtt) - }).catch(() => {}) - } else { - this.logger.debug('Registered RTT: ' + param + 'ms') - if (this.shortestRTT === 0 || param < this.shortestRTT) { - this.shortestRTT = param + async registerRtt (param) { + try { + if (param instanceof Promise) { + const start = Date.now() + await param + await this.registerRtt(Date.now() - start) + } else { + this.logger.debug(`Registered RTT: ${param}ms`) + if (this.shortestRTT === 0 || param < this.shortestRTT) { + this.shortestRTT = param + } } + } catch (error) { + this.logger.debug(`Error in promise: ${error}`) } } @@ -164,8 +162,10 @@ export default class Core extends EventEmitter { */ async withTcp (fn, port) { this.usedTcp = true + const { options, logger } = this const address = this.options.address - if (!port) port = this.options.port + port ??= options.port + this.assertValidPort(port) let socket, connectionTimeout @@ -176,28 +176,29 @@ export default class Core extends EventEmitter { // Prevent unhandled 'error' events from dumping straight to console socket.on('error', () => {}) - this.logger.debug(log => { - this.logger.debug(address + ':' + port + ' TCP Connecting') + logger.debug(log => { + logger.debug(address + ':' + port + ' TCP Connecting') const writeHook = socket.write socket.write = (...args) => { log(address + ':' + port + ' TCP-->') log(debugDump(args[0])) writeHook.apply(socket, args) } + socket.on('error', e => log('TCP Error:', e)) socket.on('close', () => log('TCP Closed')) socket.on('data', (data) => { - log(address + ':' + port + ' <--TCP') + log(`${address}:${port} <--TCP`) log(data) }) - socket.on('ready', () => log(address + ':' + port + ' TCP Connected')) + socket.on('ready', () => log(`${address}:${port} TCP Connected`)) }) const connectionPromise = new Promise((resolve, reject) => { socket.on('ready', resolve) socket.on('close', () => reject(new Error('TCP Connection Refused'))) }) - this.registerRtt(connectionPromise) + await this.registerRtt(connectionPromise) connectionTimeout = Promises.createTimeout(this.options.socketTimeout, 'TCP Opening') await Promise.race([ connectionPromise, @@ -206,8 +207,8 @@ export default class Core extends EventEmitter { ]) return await fn(socket) } finally { - socket && socket.destroy() - connectionTimeout && connectionTimeout.cancel() + socket?.destroy() + connectionTimeout?.cancel() } } @@ -221,7 +222,7 @@ export default class Core extends EventEmitter { async tcpSend (socket, buffer, ondata) { let timeout try { - const promise = new Promise((resolve, reject) => { + const promise = new Promise((resolve, _reject) => { let received = Buffer.from([]) const onData = (data) => { received = Buffer.concat([received, data]) @@ -237,7 +238,7 @@ export default class Core extends EventEmitter { timeout = Promises.createTimeout(this.options.socketTimeout, 'TCP') return await Promise.race([promise, timeout, this.abortedPromise]) } finally { - timeout && timeout.cancel() + timeout?.cancel() } } @@ -249,8 +250,7 @@ export default class Core extends EventEmitter { * @template T */ async udpSend (buffer, onPacket, onTimeout) { - const address = this.options.address - const port = this.options.port + const { address, port, debug, socketTimeout } = this.options this.assertValidPort(port) if (typeof buffer === 'string') buffer = Buffer.from(buffer, 'binary') @@ -264,19 +264,14 @@ export default class Core extends EventEmitter { let socketCallback let timeout + try { const promise = new Promise((resolve, reject) => { const start = Date.now() - let end = null socketCallback = (fromAddress, fromPort, buffer) => { try { - if (fromAddress !== address) return - if (fromPort !== port) return - if (end === null) { - end = Date.now() - const rtt = end - start - this.registerRtt(rtt) - } + if (fromAddress !== address || fromPort !== port) return + this.registerRtt(Date.now() - start) const result = onPacket(buffer) if (result !== undefined) { this.logger.debug('UDP send finished by callback') @@ -286,30 +281,24 @@ export default class Core extends EventEmitter { reject(e) } } - socket.addCallback(socketCallback, this.options.debug) + socket.addCallback(socketCallback, debug) }) - timeout = Promises.createTimeout(this.options.socketTimeout, 'UDP') - const wrappedTimeout = new Promise((resolve, reject) => { - timeout.catch((e) => { - this.logger.debug('UDP timeout detected') - if (onTimeout) { - try { - const result = onTimeout() - if (result !== undefined) { - this.logger.debug('UDP timeout resolved by callback') - resolve(result) - return - } - } catch (e) { - reject(e) - } + timeout = Promises.createTimeout(socketTimeout, 'UDP') + const wrappedTimeout = Promise.resolve(timeout).catch((e) => { + this.logger.debug('UDP timeout detected') + if (onTimeout) { + const result = onTimeout() + if (result !== undefined) { + this.logger.debug('UDP timeout resolved by callback') + return result } - reject(e) - }) + } + throw e }) + return await Promise.race([promise, wrappedTimeout, this.abortedPromise]) } finally { - timeout && timeout.cancel() + timeout?.cancel() socketCallback && socket.removeCallback(socketCallback) } } @@ -344,7 +333,7 @@ export default class Core extends EventEmitter { }) return await Promise.race([wrappedPromise, this.abortedPromise]) } finally { - requestPromise && requestPromise.cancel() + requestPromise?.cancel() } } }