From 7907534a8d867132957b823db8603a418d763e42 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Thu, 28 Sep 2017 00:16:41 -0700 Subject: [PATCH] lib: faster type checks for some types PR-URL: https://github.com/nodejs/node/pull/15663 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Luigi Pinca Reviewed-By: Benedikt Meurer Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/_tls_common.js | 3 +- lib/assert.js | 5 ++- lib/buffer.js | 11 +++--- lib/child_process.js | 2 +- lib/dgram.js | 2 +- lib/fs.js | 3 +- lib/internal/child_process.js | 2 +- lib/internal/crypto/diffiehellman.js | 3 +- lib/internal/crypto/hash.js | 5 +-- lib/internal/crypto/random.js | 2 +- lib/internal/encoding.js | 6 ++- lib/internal/http2/core.js | 3 +- lib/internal/util/types.js | 36 ++++++++++++++++++ lib/repl.js | 3 +- lib/stream.js | 12 ++++-- lib/tls.js | 2 +- lib/util.js | 5 ++- lib/zlib.js | 7 ++-- node.gyp | 1 + test/parallel/test-types.js | 57 ++++++++++++++++++++++++++++ 20 files changed, 140 insertions(+), 30 deletions(-) create mode 100644 lib/internal/util/types.js create mode 100644 test/parallel/test-types.js diff --git a/lib/_tls_common.js b/lib/_tls_common.js index fa31fd7de6..54b27a7bca 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -22,6 +22,7 @@ 'use strict'; const { parseCertString } = require('internal/tls'); +const { isArrayBufferView } = require('internal/util/types'); const tls = require('tls'); const errors = require('internal/errors'); @@ -55,7 +56,7 @@ function SecureContext(secureProtocol, secureOptions, context) { } function validateKeyCert(value, type) { - if (typeof value !== 'string' && !ArrayBuffer.isView(value)) + if (typeof value !== 'string' && !isArrayBufferView(value)) throw new errors.TypeError( 'ERR_INVALID_ARG_TYPE', type, ['string', 'Buffer', 'TypedArray', 'DataView'] diff --git a/lib/assert.js b/lib/assert.js index 2aaf80eecc..a29408f47e 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -23,6 +23,7 @@ const { compare } = process.binding('buffer'); const { isSet, isMap, isDate, isRegExp } = process.binding('util'); const { objectToString } = require('internal/util'); +const { isArrayBufferView } = require('internal/util/types'); const errors = require('internal/errors'); const { propertyIsEnumerable } = Object.prototype; @@ -209,7 +210,7 @@ function strictDeepEqual(actual, expected, memos) { if (actual.message !== expected.message) { return false; } - } else if (ArrayBuffer.isView(actual)) { + } else if (isArrayBufferView(actual)) { if (!areSimilarTypedArrays(actual, expected, isFloatTypedArrayTag(actualTag) ? 0 : 300)) { return false; @@ -262,7 +263,7 @@ function looseDeepEqual(actual, expected, memos) { const actualTag = objectToString(actual); const expectedTag = objectToString(expected); if (actualTag === expectedTag) { - if (!isObjectOrArrayTag(actualTag) && ArrayBuffer.isView(actual)) { + if (!isObjectOrArrayTag(actualTag) && isArrayBufferView(actual)) { return areSimilarTypedArrays(actual, expected, isFloatTypedArrayTag(actualTag) ? Infinity : 300); diff --git a/lib/buffer.js b/lib/buffer.js index 7a44ca0f26..6a176756c0 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -46,15 +46,16 @@ const { kMaxLength, kStringMaxLength } = process.binding('buffer'); -const { - isAnyArrayBuffer, - isUint8Array -} = process.binding('util'); +const { isAnyArrayBuffer } = process.binding('util'); const { customInspectSymbol, normalizeEncoding, kIsEncodingSymbol } = require('internal/util'); +const { + isArrayBufferView, + isUint8Array +} = require('internal/util/types'); const { pendingDeprecation } = process.binding('config'); @@ -501,7 +502,7 @@ function base64ByteLength(str, bytes) { function byteLength(string, encoding) { if (typeof string !== 'string') { - if (ArrayBuffer.isView(string) || isAnyArrayBuffer(string)) { + if (isArrayBufferView(string) || isAnyArrayBuffer(string)) { return string.byteLength; } diff --git a/lib/child_process.js b/lib/child_process.js index b2679c5f50..a3cdadd0bc 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -23,13 +23,13 @@ const util = require('util'); const { deprecate, convertToValidSignal } = require('internal/util'); +const { isUint8Array } = require('internal/util/types'); const { createPromise, promiseResolve, promiseReject } = process.binding('util'); const debug = util.debuglog('child_process'); const Buffer = require('buffer').Buffer; const Pipe = process.binding('pipe_wrap').Pipe; -const { isUint8Array } = process.binding('util'); const { errname } = process.binding('uv'); const child_process = require('internal/child_process'); diff --git a/lib/dgram.js b/lib/dgram.js index 85e5f909c0..f33e872e64 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -26,6 +26,7 @@ const errors = require('internal/errors'); const Buffer = require('buffer').Buffer; const dns = require('dns'); const util = require('util'); +const { isUint8Array } = require('internal/util/types'); const EventEmitter = require('events'); const setInitTriggerId = require('async_hooks').setInitTriggerId; const UV_UDP_REUSEADDR = process.binding('constants').os.UV_UDP_REUSEADDR; @@ -34,7 +35,6 @@ const nextTick = require('internal/process/next_tick').nextTick; const UDP = process.binding('udp_wrap').UDP; const SendWrap = process.binding('udp_wrap').SendWrap; -const { isUint8Array } = process.binding('util'); const BIND_STATE_UNBOUND = 0; const BIND_STATE_BINDING = 1; diff --git a/lib/fs.js b/lib/fs.js index 24af0e58a3..5521094431 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -28,7 +28,8 @@ const constants = process.binding('constants').fs; const { S_IFIFO, S_IFLNK, S_IFMT, S_IFREG, S_IFSOCK } = constants; const util = require('util'); const pathModule = require('path'); -const { isUint8Array, createPromise, promiseResolve } = process.binding('util'); +const { isUint8Array } = require('internal/util/types'); +const { createPromise, promiseResolve } = process.binding('util'); const binding = process.binding('fs'); const fs = exports; diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 6613c72a5c..28ab13cd54 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -15,8 +15,8 @@ const TTY = process.binding('tty_wrap').TTY; const TCP = process.binding('tcp_wrap').TCP; const UDP = process.binding('udp_wrap').UDP; const SocketList = require('internal/socket_list'); -const { isUint8Array } = process.binding('util'); const { convertToValidSignal } = require('internal/util'); +const { isUint8Array } = require('internal/util/types'); const spawn_sync = process.binding('spawn_sync'); const { diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js index 8466df2f79..95b689df94 100644 --- a/lib/internal/crypto/diffiehellman.js +++ b/lib/internal/crypto/diffiehellman.js @@ -2,6 +2,7 @@ const { Buffer } = require('buffer'); const errors = require('internal/errors'); +const { isArrayBufferView } = require('internal/util/types'); const { getDefaultEncoding, toBuf @@ -25,7 +26,7 @@ function DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding) { if (typeof sizeOrKey !== 'number' && typeof sizeOrKey !== 'string' && - !ArrayBuffer.isView(sizeOrKey)) { + !isArrayBufferView(sizeOrKey)) { throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'sizeOrKey', ['number', 'string', 'Buffer', 'TypedArray', 'DataView']); diff --git a/lib/internal/crypto/hash.js b/lib/internal/crypto/hash.js index 12b3e1e78e..952cc34d21 100644 --- a/lib/internal/crypto/hash.js +++ b/lib/internal/crypto/hash.js @@ -10,15 +10,12 @@ const { toBuf } = require('internal/crypto/util'); -const { - isArrayBufferView -} = process.binding('util'); - const { Buffer } = require('buffer'); const errors = require('internal/errors'); const { inherits } = require('util'); const { normalizeEncoding } = require('internal/util'); +const { isArrayBufferView } = require('internal/util/types'); const LazyTransform = require('internal/streams/lazy_transform'); const kState = Symbol('state'); const kFinalized = Symbol('finalized'); diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js index 81025289d5..ee510ddf06 100644 --- a/lib/internal/crypto/random.js +++ b/lib/internal/crypto/random.js @@ -1,7 +1,7 @@ 'use strict'; const errors = require('internal/errors'); -const { isArrayBufferView } = process.binding('util'); +const { isArrayBufferView } = require('internal/util/types'); const { randomBytes, randomFill: _randomFill diff --git a/lib/internal/encoding.js b/lib/internal/encoding.js index de4ebcf196..5c2e207258 100644 --- a/lib/internal/encoding.js +++ b/lib/internal/encoding.js @@ -20,6 +20,8 @@ const { customInspectSymbol: inspect } = require('internal/util'); +const { isArrayBufferView } = require('internal/util/types'); + const { isArrayBuffer } = process.binding('util'); @@ -386,7 +388,7 @@ function makeTextDecoderICU() { throw new errors.TypeError('ERR_INVALID_THIS', 'TextDecoder'); if (isArrayBuffer(input)) { input = lazyBuffer().from(input); - } else if (!ArrayBuffer.isView(input)) { + } else if (!isArrayBufferView(input)) { throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'input', ['ArrayBuffer', 'ArrayBufferView']); } @@ -462,7 +464,7 @@ function makeTextDecoderJS() { throw new errors.TypeError('ERR_INVALID_THIS', 'TextDecoder'); if (isArrayBuffer(input)) { input = lazyBuffer().from(input); - } else if (ArrayBuffer.isView(input)) { + } else if (isArrayBufferView(input)) { input = lazyBuffer().from(input.buffer, input.byteOffset, input.byteLength); } else { diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 78a4896ab7..56b99cd701 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -21,8 +21,9 @@ const { onServerStream, } = require('internal/http2/compat'); const { utcDate } = require('internal/http'); const { promisify } = require('internal/util'); +const { isUint8Array } = require('internal/util/types'); const { _connectionListener: httpConnectionListener } = require('http'); -const { isUint8Array, createPromise, promiseResolve } = process.binding('util'); +const { createPromise, promiseResolve } = process.binding('util'); const debug = util.debuglog('http2'); diff --git a/lib/internal/util/types.js b/lib/internal/util/types.js new file mode 100644 index 0000000000..c990bea6db --- /dev/null +++ b/lib/internal/util/types.js @@ -0,0 +1,36 @@ +'use strict'; + +const ReflectApply = Reflect.apply; + +// This function is borrowed from the function with the same name on V8 Extras' +// `utils` object. V8 implements Reflect.apply very efficiently in conjunction +// with the spread syntax, such that no additional special case is needed for +// function calls w/o arguments. +// Refs: https://github.com/v8/v8/blob/d6ead37d265d7215cf9c5f768f279e21bd170212/src/js/prologue.js#L152-L156 +function uncurryThis(func) { + return (thisArg, ...args) => ReflectApply(func, thisArg, args); +} + +const TypedArrayPrototype = Object.getPrototypeOf(Uint8Array.prototype); + +const TypedArrayProto_toStringTag = + uncurryThis( + Object.getOwnPropertyDescriptor(TypedArrayPrototype, + Symbol.toStringTag).get); + +// Cached to make sure no userland code can tamper with it. +const isArrayBufferView = ArrayBuffer.isView; + +function isTypedArray(value) { + return TypedArrayProto_toStringTag(value) !== undefined; +} + +function isUint8Array(value) { + return TypedArrayProto_toStringTag(value) === 'Uint8Array'; +} + +module.exports = { + isArrayBufferView, + isTypedArray, + isUint8Array +}; diff --git a/lib/repl.js b/lib/repl.js index bc9b17de72..b032521993 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -44,6 +44,7 @@ const internalModule = require('internal/module'); const internalUtil = require('internal/util'); +const { isTypedArray } = require('internal/util/types'); const util = require('util'); const utilBinding = process.binding('util'); const inherits = util.inherits; @@ -726,7 +727,7 @@ const ARRAY_LENGTH_THRESHOLD = 1e6; function mayBeLargeObject(obj) { if (Array.isArray(obj)) { return obj.length > ARRAY_LENGTH_THRESHOLD ? ['length'] : null; - } else if (utilBinding.isTypedArray(obj)) { + } else if (isTypedArray(obj)) { return obj.length > ARRAY_LENGTH_THRESHOLD ? [] : null; } diff --git a/lib/stream.js b/lib/stream.js index 9c1ba986d0..0dd44e3a61 100644 --- a/lib/stream.js +++ b/lib/stream.js @@ -38,10 +38,16 @@ Stream.Stream = Stream; // Internal utilities try { - Stream._isUint8Array = process.binding('util').isUint8Array; + Stream._isUint8Array = require('internal/util/types').isUint8Array; } catch (e) { - // This throws for Node < 4.2.0 because there’s no util binding and - // returns undefined for Node < 7.4.0. + // Throws for code outside of Node.js core. + + try { + Stream._isUint8Array = process.binding('util').isUint8Array; + } catch (e) { + // This throws for Node < 4.2.0 because there’s no util binding and + // returns undefined for Node < 7.4.0. + } } if (!Stream._isUint8Array) { diff --git a/lib/tls.js b/lib/tls.js index 6c6e4b0918..2d1c539532 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -25,12 +25,12 @@ const errors = require('internal/errors'); const internalUtil = require('internal/util'); const internalTLS = require('internal/tls'); internalUtil.assertCrypto(); +const { isUint8Array } = require('internal/util/types'); const net = require('net'); const url = require('url'); const binding = process.binding('crypto'); const Buffer = require('buffer').Buffer; -const { isUint8Array } = process.binding('util'); // Allow {CLIENT_RENEG_LIMIT} client-initiated session renegotiations // every {CLIENT_RENEG_WINDOW} seconds. An error event is emitted if more diff --git a/lib/util.js b/lib/util.js index 590a3b4903..365a15fc9c 100644 --- a/lib/util.js +++ b/lib/util.js @@ -38,13 +38,16 @@ const { isPromise, isSet, isSetIterator, - isTypedArray, isRegExp, isDate, kPending, kRejected, } = process.binding('util'); +const { + isTypedArray +} = require('internal/util/types'); + const { customInspectSymbol, deprecate, diff --git a/lib/zlib.js b/lib/zlib.js index 35d998df26..49cf8f0315 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -24,6 +24,7 @@ const Buffer = require('buffer').Buffer; const Transform = require('_stream_transform'); const { _extend } = require('util'); +const { isArrayBufferView } = require('internal/util/types'); const binding = process.binding('zlib'); const assert = require('assert').ok; const kMaxLength = require('buffer').kMaxLength; @@ -62,7 +63,7 @@ for (var ck = 0; ck < ckeys.length; ck++) { function zlibBuffer(engine, buffer, callback) { // Streams do not support non-Buffer ArrayBufferViews yet. Convert it to a // Buffer without copying. - if (ArrayBuffer.isView(buffer) && + if (isArrayBufferView(buffer) && Object.getPrototypeOf(buffer) !== Buffer.prototype) { buffer = Buffer.from(buffer.buffer, buffer.byteOffset, buffer.byteLength); } @@ -109,7 +110,7 @@ function zlibBufferOnEnd() { function zlibBufferSync(engine, buffer) { if (typeof buffer === 'string') { buffer = Buffer.from(buffer); - } else if (!ArrayBuffer.isView(buffer)) { + } else if (!isArrayBufferView(buffer)) { throw new TypeError('"buffer" argument must be a string, Buffer, ' + 'TypedArray, or DataView'); } @@ -226,7 +227,7 @@ function Zlib(opts, mode) { } dictionary = opts.dictionary; - if (dictionary !== undefined && !ArrayBuffer.isView(dictionary)) { + if (dictionary !== undefined && !isArrayBufferView(dictionary)) { throw new TypeError( 'Invalid dictionary: it should be a Buffer, TypedArray, or DataView'); } diff --git a/node.gyp b/node.gyp index 338b7f324e..5ccc4ff423 100644 --- a/node.gyp +++ b/node.gyp @@ -123,6 +123,7 @@ 'lib/internal/tls.js', 'lib/internal/url.js', 'lib/internal/util.js', + 'lib/internal/util/types.js', 'lib/internal/http2/core.js', 'lib/internal/http2/compat.js', 'lib/internal/http2/util.js', diff --git a/test/parallel/test-types.js b/test/parallel/test-types.js new file mode 100644 index 0000000000..ea8adc6cb1 --- /dev/null +++ b/test/parallel/test-types.js @@ -0,0 +1,57 @@ +'use strict'; + +// Flags: --expose-internals + +require('../common'); +const assert = require('assert'); +const types = require('internal/util/types'); + +const primitive = true; +const arrayBuffer = new ArrayBuffer(); +const dataView = new DataView(arrayBuffer); +const int32Array = new Int32Array(arrayBuffer); +const uint8Array = new Uint8Array(arrayBuffer); +const buffer = Buffer.from(arrayBuffer); + +const fakeDataView = Object.create(DataView.prototype); +const fakeInt32Array = Object.create(Int32Array.prototype); +const fakeUint8Array = Object.create(Uint8Array.prototype); +const fakeBuffer = Object.create(Buffer.prototype); + +const stealthyDataView = + Object.setPrototypeOf(new DataView(arrayBuffer), Uint8Array.prototype); +const stealthyInt32Array = + Object.setPrototypeOf(new Int32Array(arrayBuffer), uint8Array); +const stealthyUint8Array = + Object.setPrototypeOf(new Uint8Array(arrayBuffer), ArrayBuffer.prototype); + +const all = [ + primitive, arrayBuffer, dataView, int32Array, uint8Array, buffer, + fakeDataView, fakeInt32Array, fakeUint8Array, fakeBuffer, + stealthyDataView, stealthyInt32Array, stealthyUint8Array +]; + +const expected = { + isArrayBufferView: [ + dataView, int32Array, uint8Array, buffer, + stealthyDataView, stealthyInt32Array, stealthyUint8Array + ], + isTypedArray: [ + int32Array, uint8Array, buffer, stealthyInt32Array, stealthyUint8Array + ], + isUint8Array: [ + uint8Array, buffer, stealthyUint8Array + ] +}; + +for (const testedFunc of Object.keys(expected)) { + const func = types[testedFunc]; + const yup = []; + for (const value of all) { + if (func(value)) { + yup.push(value); + } + } + console.log('Testing', testedFunc); + assert.deepStrictEqual(yup, expected[testedFunc]); +}