Skip to content

Commit

Permalink
lib: faster FreeList
Browse files Browse the repository at this point in the history
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: #27021
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
apapirovski authored and danbev committed Apr 11, 2019
1 parent 547576f commit 47f5cc1
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 25 deletions.
4 changes: 3 additions & 1 deletion benchmark/misc/freelist.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
25 changes: 9 additions & 16 deletions lib/internal/freelist.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const is_reused_symbol = Symbol('isReused');
const { Reflect } = primordials;

class FreeList {
constructor(name, max, ctor) {
Expand All @@ -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) {
Expand All @@ -31,9 +29,4 @@ class FreeList {
}
}

module.exports = {
FreeList,
symbols: {
is_reused_symbol
}
};
module.exports = FreeList;
2 changes: 1 addition & 1 deletion test/parallel/test-freelist.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
4 changes: 2 additions & 2 deletions test/sequential/test-http-regr-gh-2928.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -25,14 +24,15 @@ 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) => {
// If SmartOS and ECONNREFUSED, then retry. See
// https://github.com/nodejs/node/issues/2663.
if (common.isSunOS && e.code === 'ECONNREFUSED') {
parsers.push(parser);
parser.reused = true;
socket.destroy();
setImmediate(execAndClose);
return;
Expand Down

0 comments on commit 47f5cc1

Please sign in to comment.