Skip to content

Commit

Permalink
lib: use Object.create(null) directly
Browse files Browse the repository at this point in the history 
After V8 5.6, using Object.create(null) directly is now faster than
using a constructor for map-like objects.

PR-URL: #11930
Refs: emberjs/ember.js#15001
Refs: https://crrev.com/532c16eca071df3ec8eed394dcebb932ef584ee6
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@TimothyGu
TimothyGu committed Mar 24, 2017
1 parent 14a9195 commit cfc8422
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 34 deletions.
5 changes: 2 additions & 3 deletions lib/_http_outgoing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const common = require('_http_common');
const checkIsHttpToken = common._checkIsHttpToken;
const checkInvalidHeaderChar = common._checkInvalidHeaderChar;
const outHeadersKey = require('internal/http').outHeadersKey;
const StorageObject = require('internal/querystring').StorageObject;

const CRLF = common.CRLF;
const debug = common.debug;
Expand Down Expand Up @@ -143,7 +142,7 @@ Object.defineProperty(OutgoingMessage.prototype, '_headerNames', {
get: function() {
const headers = this[outHeadersKey];
if (headers) {
const out = new StorageObject();
const out = Object.create(null);
const keys = Object.keys(headers);
for (var i = 0; i < keys.length; ++i) {
const key = keys[i];
Expand Down Expand Up @@ -552,7 +551,7 @@ OutgoingMessage.prototype.getHeaderNames = function getHeaderNames() {
// Returns a shallow copy of the current outgoing headers.
OutgoingMessage.prototype.getHeaders = function getHeaders() {
const headers = this[outHeadersKey];
const ret = new StorageObject();
const ret = Object.create(null);
if (headers) {
const keys = Object.keys(headers);
for (var i = 0; i < keys.length; ++i) {
Expand Down
20 changes: 7 additions & 13 deletions lib/events.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@

var domain;

// This constructor is used to store event handlers. Instantiating this is
// faster than explicitly calling `Object.create(null)` to get a "clean" empty
// object (tested with v8 v4.9).
function EventHandlers() {}
EventHandlers.prototype = Object.create(null);

function EventEmitter() {
EventEmitter.init.call(this);
}
Expand Down Expand Up @@ -75,7 +69,7 @@ EventEmitter.init = function() {
}

if (!this._events || this._events === Object.getPrototypeOf(this)._events) {
this._events = new EventHandlers();
this._events = Object.create(null);
this._eventsCount = 0;
}

Expand Down Expand Up @@ -245,7 +239,7 @@ function _addListener(target, type, listener, prepend) {

events = target._events;
if (!events) {
events = target._events = new EventHandlers();
events = target._events = Object.create(null);
target._eventsCount = 0;
} else {
// To avoid recursion in the case that type === "newListener"! Before
Expand Down Expand Up @@ -360,7 +354,7 @@ EventEmitter.prototype.removeListener =

if (list === listener || list.listener === listener) {
if (--this._eventsCount === 0)
this._events = new EventHandlers();
this._events = Object.create(null);
else {
delete events[type];
if (events.removeListener)
Expand All @@ -383,7 +377,7 @@ EventEmitter.prototype.removeListener =
if (list.length === 1) {
list[0] = undefined;
if (--this._eventsCount === 0) {
this._events = new EventHandlers();
this._events = Object.create(null);
return this;
} else {
delete events[type];
Expand Down Expand Up @@ -412,11 +406,11 @@ EventEmitter.prototype.removeAllListeners =
// not listening for removeListener, no need to emit
if (!events.removeListener) {
if (arguments.length === 0) {
this._events = new EventHandlers();
this._events = Object.create(null);
this._eventsCount = 0;
} else if (events[type]) {
if (--this._eventsCount === 0)
this._events = new EventHandlers();
this._events = Object.create(null);
else
delete events[type];
}
Expand All @@ -432,7 +426,7 @@ EventEmitter.prototype.removeAllListeners =
this.removeAllListeners(key);
}
this.removeAllListeners('removeListener');
this._events = new EventHandlers();
this._events = Object.create(null);
this._eventsCount = 0;
return this;
}
Expand Down
11 changes: 5 additions & 6 deletions lib/fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ const internalUtil = require('internal/util');
const assertEncoding = internalFS.assertEncoding;
const stringToFlags = internalFS.stringToFlags;
const getPathFromURL = internalURL.getPathFromURL;
const { StorageObject } = require('internal/querystring');

Object.defineProperty(exports, 'constants', {
configurable: false,
Expand Down Expand Up @@ -1560,7 +1559,7 @@ if (isWindows) {
nextPart = function nextPart(p, i) { return p.indexOf('/', i); };
}

const emptyObj = new StorageObject();
const emptyObj = Object.create(null);
fs.realpathSync = function realpathSync(p, options) {
if (!options)
options = emptyObj;
Expand All @@ -1580,8 +1579,8 @@ fs.realpathSync = function realpathSync(p, options) {
return maybeCachedResult;
}

const seenLinks = new StorageObject();
const knownHard = new StorageObject();
const seenLinks = Object.create(null);
const knownHard = Object.create(null);
const original = p;

// current character position in p
Expand Down Expand Up @@ -1700,8 +1699,8 @@ fs.realpath = function realpath(p, options, callback) {
return;
p = pathModule.resolve(p);

const seenLinks = new StorageObject();
const knownHard = new StorageObject();
const seenLinks = Object.create(null);
const knownHard = Object.create(null);

// current character position in p
var pos;
Expand Down
8 changes: 1 addition & 7 deletions lib/internal/querystring.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@ const isHexTable = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 // ... 256
];

// Instantiating this is faster than explicitly calling `Object.create(null)`
// to get a "clean" empty object (tested with v8 v4.9).
function StorageObject() {}
StorageObject.prototype = Object.create(null);

module.exports = {
hexTable,
isHexTable,
StorageObject
isHexTable
};
3 changes: 1 addition & 2 deletions lib/querystring.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

const { Buffer } = require('buffer');
const {
StorageObject,
hexTable,
isHexTable
} = require('internal/querystring');
Expand Down Expand Up @@ -281,7 +280,7 @@ const defEqCodes = [61]; // =

// Parse a key/val string.
function parse(qs, sep, eq, options) {
const obj = new StorageObject();
const obj = Object.create(null);

if (typeof qs !== 'string' || qs.length === 0) {
return obj;
Expand Down
6 changes: 3 additions & 3 deletions lib/url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

const { toASCII } = process.binding('config').hasIntl ?
process.binding('icu') : require('punycode');
const { StorageObject, hexTable } = require('internal/querystring');
const { hexTable } = require('internal/querystring');
const internalUrl = require('internal/url');
exports.parse = urlParse;
exports.resolve = urlResolve;
Expand Down Expand Up @@ -197,7 +197,7 @@ Url.prototype.parse = function(url, parseQueryString, slashesDenoteHost) {
}
} else if (parseQueryString) {
this.search = '';
this.query = new StorageObject();
this.query = Object.create(null);
}
return this;
}
Expand Down Expand Up @@ -390,7 +390,7 @@ Url.prototype.parse = function(url, parseQueryString, slashesDenoteHost) {
} else if (parseQueryString) {
// no query string, but parseQueryString still requested
this.search = '';
this.query = new StorageObject();
this.query = Object.create(null);
}

var firstIdx = (questionIdx !== -1 &&
Expand Down

3 comments on commit cfc8422

@twoi
Copy link

@twoi twoi commented on cfc8422 Apr 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's faster now, but it is not backwards compatible, e.g. it broke the following code:

var reqArgs = url.parse(req.url, true).query;
if (reqArgs.hasOwnProperty("id"))
  ...

now throws TypeError: reqArgs.hasOwnProperty is not a function

@Trott
Copy link
Member

@Trott Trott commented on cfc8422 Apr 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@twoi This change did not break that code, at least not for values of req.url that I'm testing. That code threw a TypeError before this change. (For example, it throws in Node.js 6.17.0, which does not have this change.)

The breaking change is (probably) #6055.

@Trott
Copy link
Member

@Trott Trott commented on cfc8422 Apr 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And just to show an example value of req.url in case it works for some values and not others....

$ node -v
v6.0.0
$ node
> var reqArgs = url.parse('http://q.b.c./', true).query;
undefined
> reqArgs.hasOwnProperty('id')
TypeError: reqArgs.hasOwnProperty is not a function
    at repl:1:9
    at REPLServer.defaultEval (repl.js:272:27)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:441:10)
    at emitOne (events.js:101:20)
    at REPLServer.emit (events.js:188:7)
    at REPLServer.Interface._onLine (readline.js:219:10)
    at REPLServer.Interface._line (readline.js:561:8)
    at REPLServer.Interface._ttyWrite (readline.js:838:14)
>

Please sign in to comment.