Skip to content

lib: faster type checks for some types#15663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TimothyGu wants to merge 2 commits into from
Closed

lib: faster type checks for some types #15663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1f939a
lib: faster type checks for some types
TimothyGu Sep 28, 2017
6116c39
benchmark: add util/type-check
TimothyGu Sep 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions benchmark/util/type-check.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
'use strict';

const common = require('../common');

const arrayBuffer = new ArrayBuffer();
const dataView = new DataView(arrayBuffer);
const uint8Array = new Uint8Array(arrayBuffer);
const int32Array = new Int32Array(arrayBuffer);

const args = {
ArrayBufferView: {
'true': dataView,
'false-primitive': true,
'false-object': arrayBuffer
},
TypedArray: {
'true': int32Array,
'false-primitive': true,
'false-object': arrayBuffer
},
Uint8Array: {
'true': uint8Array,
'false-primitive': true,
'false-object': int32Array
}
};

const bench = common.createBenchmark(main, {
type: Object.keys(args),
version: ['native', 'js'],
argument: ['true', 'false-primitive', 'false-object'],
millions: ['5']
}, {
flags: ['--expose-internals']
});

function main(conf) {
const util = process.binding('util');
const types = require('internal/util/types');

const n = (+conf.millions * 1e6) | 0;
const func = { native: util, js: types }[conf.version][`is${conf.type}`];
const arg = args[conf.type][conf.argument];

bench.start();
for (var i = 0; i < n; i++) {
func(arg);
}
bench.end(n);
}
3 changes: 2 additions & 1 deletion lib/_tls_common.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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');

Expand DownExpand Up@@ -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']
Expand Down
5 changes: 3 additions & 2 deletions lib/assert.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;

Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -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);
Expand Down
11 changes: 6 additions & 5 deletions lib/buffer.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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');
Expand DownExpand Up@@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/child_process.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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');

Expand Down
2 changes: 1 addition & 1 deletion lib/dgram.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -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;
Expand Down
3 changes: 2 additions & 1 deletion lib/fs.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/child_process.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/crypto/diffiehellman.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@

const { Buffer } = require('buffer');
const errors = require('internal/errors');
const { isArrayBufferView } = require('internal/util/types');
const {
getDefaultEncoding,
toBuf
Expand All@@ -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']);
Expand Down
5 changes: 1 addition & 4 deletions lib/internal/crypto/hash.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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');
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/crypto/random.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/encoding.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,8 @@ const {
customInspectSymbol: inspect
} = require('internal/util');

const { isArrayBufferView } = require('internal/util/types');

const {
isArrayBuffer
} = process.binding('util');
Expand DownExpand Up@@ -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']);
}
Expand DownExpand Up@@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/http2/core.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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');


Expand Down
36 changes: 36 additions & 0 deletions lib/internal/util/types.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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) {

@bmeurerbmeurerSep 30, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Have you checked the generated code for isUint8Array? I don't think we get the Reflect.apply optimization with V8 6.1 already.

@TimothyGuTimothyGuSep 30, 2017

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes I have:

--- Raw source ---(value) { return TypedArrayProto_toStringTag(value) === 'Uint8Array';}--- Optimized code ---optimization_id = 0source_position = 1086kind = OPTIMIZED_FUNCTIONname = isUint8Arraystack_slots = 4compiler = turbofanInstructions (size = 194)0x367169e055c0055pushrbp0x367169e055c11 4889e5 REX.W movqrbp,rsp0x367169e055c4456pushrsi0x367169e055c5557pushrdi0x367169e055c66 493ba5e80c0000 REX.W cmpq rsp,[r13+0xce8]0x367169e055cd d 0f8656000000 jna0x367169e05629 <+0x69>0x367169e055d313 48bf59b6c858e83c0000 REX.W movqrdi,0x3ce858c8b659 ;; object: 0x3ce858c8b659 <JSFunction b (sfi = 0x2fc5e3493879)>0x367169e055dd 1d 488b7727 REX.W movqrsi,[rdi+0x27]0x367169e055e121 488b5d10 REX.W movqrbx,[rbp+0x10]0x367169e055e52553pushrbx0x367169e055e626 498b55a0 REX.W movqrdx,[r13-0x60]0x367169e055ea 2a 33c0 xorl rax,rax0x367169e055ec 2c ff5737 call[rdi+0x37]0x367169e055ef 2f a801 testal,0x10x367169e055f131 0f844e000000 jz0x367169e05645 <+0x85>0x367169e055f737 488b58ff REX.W movqrbx,[rax-0x1]0x367169e055fb 3b f6430bc0 testb [rbx+0xb],0xc00x367169e055ff 3f 0f8545000000 jnz0x367169e0564a <+0x8a>0x367169e0560545 48bb711549e3c52f0000 REX.W movqrbx,0x2fc5e3491571 ;; object: 0x2fc5e3491571 <String[10]: Uint8Array>0x367169e0560f 4f 483bd8 REX.W cmpq rbx,rax0x367169e0561252 0f840b000000 jz0x367169e05623 <+0x63>0x367169e0561858 498b45c0 REX.W movqrax,[r13-0x40]0x367169e0561c 5c 488be5 REX.W movqrsp,rbp0x367169e0561f 5f 5d poprbp0x367169e0562060 c21000 ret0x100x367169e0562363 498b45b8 REX.W movqrax,[r13-0x48]0x367169e0562767 ebf3 jmp0x367169e0561c <+0x5c>0x367169e0562969 48bb1067f50000000000 REX.W movqrbx,0xf567100x367169e0563373 33c0 xorl rax,rax0x367169e0563575 488b75f8 REX.W movqrsi,[rbp-0x8]0x367169e0563979 e8a2efe7ff call0x367169c845e0 ;; code: STUB, CEntryStub, minor: 80x367169e0563e 7e eb93 jmp0x367169e055d3 <+0x13>0x367169e0564080 e8bbe9cfff call0x367169b04000 ;; deoptimization bailout 00x367169e0564585 e8c0e9cfff call0x367169b0400a ;; deoptimization bailout 10x367169e0564a 8a e8c5e9cfff call0x367169b04014 ;; deoptimization bailout 20x367169e0564f 8f e8cae9cfff call0x367169b0401e ;; deoptimization bailout 30x367169e056549490nop0x367169e056559590nop...Source positions: pc offset position26553691086Inlined functions (count = 1)0x3ce858ce6719 <SharedFunctionInfo args>Deoptimization Input Data (deopt points = 4) index bytecode-offset trampoline_pc pc02880 2f 11585 NA 215 8a NA 30 8f 7e Safepoints (size = 30)0x367169e055ef 2f 0000 (sp-> fp) 00x367169e0563e 7e 0000 (sp-> fp) 3RelocInfo (size = 11)0x367169e055d5 embedded object (0x3ce858c8b659 <JSFunction b (sfi = 0x2fc5e3493879)>)0x367169e05607 embedded object (0x2fc5e3491571 <String[10]: Uint8Array>)0x367169e0563a code target (STUB) (0x367169c845e0)0x367169e05641 runtime entry (deoptimization bailout 0)0x367169e05646 runtime entry (deoptimization bailout 1)0x367169e0564b runtime entry (deoptimization bailout 2)0x367169e05650 runtime entry (deoptimization bailout 3)--- End code ---

Note: function b() is in fact %TypedArray%.prototype.toStringTag, and args is the name given to the wrapper function created in uncurryThis, which as you can tell is fully inlined.

Note 2: I don't really care if Reflect.apply is fully optimized for now, because regardless of that this function is much faster than the check we are currently using (#15663 (comment)).

@bmeurerbmeurerOct 2, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, with more recent V8 this will get even better (I also have a patch to inline TypedArray.prototype[@@toStringTag]). Nevertheless, why don't we just use F.p.call here and avoid uncurryThis completely?

@TimothyGuTimothyGuOct 3, 2017

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@bmeurerFunction.prototype.call can be overwritten by userland code. I don't want that possibility for a type checking function.

@bmeurerbmeurerOct 3, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. Makes sense.

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
};
3 changes: 2 additions & 1 deletion lib/repl.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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;
}

Expand Down
12 changes: 9 additions & 3 deletions lib/stream.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/tls.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
5 changes: 4 additions & 1 deletion lib/util.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
7 changes: 4 additions & 3 deletions lib/zlib.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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);
}
Expand DownExpand Up@@ -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');
}
Expand DownExpand Up@@ -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');
}
Expand Down
Loading