From 47f5cc1ad1f885d4596a141323c0d1732fb3bc6d Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 30 Mar 2019 14:13:50 -0700 Subject: [PATCH] lib: faster FreeList Make FreeList faster by using Reflect.apply and not using is_reused_symbol, but rather just checking whether any items are present in the list prior to calling alloc. PR-URL: https://github.com/nodejs/node/pull/27021 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Gus Caplan Reviewed-By: Ruben Bridgewater Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- benchmark/misc/freelist.js | 4 +++- lib/_http_client.js | 4 ++-- lib/_http_common.js | 2 +- lib/_http_server.js | 4 ++-- lib/internal/freelist.js | 25 ++++++++--------------- test/parallel/test-freelist.js | 2 +- test/sequential/test-http-regr-gh-2928.js | 4 ++-- 7 files changed, 20 insertions(+), 25 deletions(-) diff --git a/benchmark/misc/freelist.js b/benchmark/misc/freelist.js index 7fa9af4f3ddb7f..e6868fae1ce8c5 100644 --- a/benchmark/misc/freelist.js +++ b/benchmark/misc/freelist.js @@ -9,7 +9,9 @@ const bench = common.createBenchmark(main, { }); function main({ n }) { - const { FreeList } = require('internal/freelist'); + let FreeList = require('internal/freelist'); + if (FreeList.FreeList) + FreeList = FreeList.FreeList; const poolSize = 1000; const list = new FreeList('test', poolSize, Object); var j; diff --git a/lib/_http_client.js b/lib/_http_client.js index 3c3bc53e694084..70e8e55eadc726 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -46,7 +46,6 @@ const { ERR_UNESCAPED_CHARACTERS } = require('internal/errors').codes; const { getTimerDuration } = require('internal/timers'); -const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol; const { DTRACE_HTTP_CLIENT_REQUEST, DTRACE_HTTP_CLIENT_RESPONSE @@ -631,10 +630,11 @@ function emitFreeNT(socket) { } function tickOnSocket(req, socket) { + const isParserReused = parsers.hasItems(); const parser = parsers.alloc(); req.socket = socket; req.connection = socket; - parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]); + parser.reinitialize(HTTPParser.RESPONSE, isParserReused); parser.socket = socket; parser.outgoing = req; req.parser = parser; diff --git a/lib/_http_common.js b/lib/_http_common.js index a6efa5a832aeb5..560d4f7b71fa05 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -29,7 +29,7 @@ const { methods, HTTPParser } = getOptionValue('--http-parser') === 'legacy' ? internalBinding('http_parser') : internalBinding('http_parser_llhttp'); -const { FreeList } = require('internal/freelist'); +const FreeList = require('internal/freelist'); const { ondrain } = require('internal/http'); const incoming = require('_http_incoming'); const { diff --git a/lib/_http_server.js b/lib/_http_server.js index ea5362b347fd7d..bc2c80599db24b 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -41,7 +41,6 @@ const { defaultTriggerAsyncIdScope, getOrSetAsyncId } = require('internal/async_hooks'); -const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol; const { IncomingMessage } = require('_http_incoming'); const { ERR_HTTP_HEADERS_SENT, @@ -348,8 +347,9 @@ function connectionListenerInternal(server, socket) { socket.setTimeout(server.timeout); socket.on('timeout', socketOnTimeout); + const isParserReused = parsers.hasItems(); const parser = parsers.alloc(); - parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]); + parser.reinitialize(HTTPParser.REQUEST, isParserReused); parser.socket = socket; // We are starting to wait for our headers. diff --git a/lib/internal/freelist.js b/lib/internal/freelist.js index 04d684e8334ff5..16437b2b8a7a6d 100644 --- a/lib/internal/freelist.js +++ b/lib/internal/freelist.js @@ -1,6 +1,6 @@ 'use strict'; -const is_reused_symbol = Symbol('isReused'); +const { Reflect } = primordials; class FreeList { constructor(name, max, ctor) { @@ -10,16 +10,14 @@ class FreeList { this.list = []; } + hasItems() { + return this.list.length > 0; + } + alloc() { - let item; - if (this.list.length > 0) { - item = this.list.pop(); - item[is_reused_symbol] = true; - } else { - item = this.ctor.apply(this, arguments); - item[is_reused_symbol] = false; - } - return item; + return this.list.length > 0 ? + this.list.pop() : + Reflect.apply(this.ctor, this, arguments); } free(obj) { @@ -31,9 +29,4 @@ class FreeList { } } -module.exports = { - FreeList, - symbols: { - is_reused_symbol - } -}; +module.exports = FreeList; diff --git a/test/parallel/test-freelist.js b/test/parallel/test-freelist.js index 03946dfda257c2..eb43308dbe59cc 100644 --- a/test/parallel/test-freelist.js +++ b/test/parallel/test-freelist.js @@ -4,7 +4,7 @@ require('../common'); const assert = require('assert'); -const { FreeList } = require('internal/freelist'); +const FreeList = require('internal/freelist'); assert.strictEqual(typeof FreeList, 'function'); diff --git a/test/sequential/test-http-regr-gh-2928.js b/test/sequential/test-http-regr-gh-2928.js index 094193f4574af0..91db08df47cb97 100644 --- a/test/sequential/test-http-regr-gh-2928.js +++ b/test/sequential/test-http-regr-gh-2928.js @@ -6,7 +6,6 @@ const common = require('../common'); const assert = require('assert'); const httpCommon = require('_http_common'); -const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol; const { HTTPParser } = require('_http_common'); const net = require('net'); @@ -25,7 +24,7 @@ function execAndClose() { process.stdout.write('.'); const parser = parsers.pop(); - parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]); + parser.reinitialize(HTTPParser.RESPONSE, !!parser.reused); const socket = net.connect(common.PORT); socket.on('error', (e) => { @@ -33,6 +32,7 @@ function execAndClose() { // https://github.com/nodejs/node/issues/2663. if (common.isSunOS && e.code === 'ECONNREFUSED') { parsers.push(parser); + parser.reused = true; socket.destroy(); setImmediate(execAndClose); return;