Skip to content
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

lib: faster type checks for some types #15663

Closed
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 number Diff line number Diff 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 number Diff line number Diff 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 Down Expand 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 number Diff line number Diff 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 Down Expand 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 Down Expand 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 number Diff line number Diff 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 Down Expand 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 number Diff line number Diff 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 number Diff line number Diff 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 number Diff line number Diff 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 number Diff line number Diff 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 number Diff line number Diff 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 number Diff line number Diff 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 number Diff line number Diff 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 number Diff line number Diff 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 Down Expand 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 Down Expand 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 number Diff line number Diff 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 number Diff line number Diff 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) {
Copy link
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.

Copy link
Member Author

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 = 0
source_position = 1086
kind = OPTIMIZED_FUNCTION
name = isUint8Array
stack_slots = 4
compiler = turbofan
Instructions (size = 194)
0x367169e055c0     0  55             push rbp
0x367169e055c1     1  4889e5         REX.W movq rbp,rsp
0x367169e055c4     4  56             push rsi
0x367169e055c5     5  57             push rdi
0x367169e055c6     6  493ba5e80c0000 REX.W cmpq rsp,[r13+0xce8]
0x367169e055cd     d  0f8656000000   jna 0x367169e05629  <+0x69>
0x367169e055d3    13  48bf59b6c858e83c0000 REX.W movq rdi,0x3ce858c8b659    ;; object: 0x3ce858c8b659 <JSFunction b (sfi = 0x2fc5e3493879)>
0x367169e055dd    1d  488b7727       REX.W movq rsi,[rdi+0x27]
0x367169e055e1    21  488b5d10       REX.W movq rbx,[rbp+0x10]
0x367169e055e5    25  53             push rbx
0x367169e055e6    26  498b55a0       REX.W movq rdx,[r13-0x60]
0x367169e055ea    2a  33c0           xorl rax,rax
0x367169e055ec    2c  ff5737         call [rdi+0x37]
0x367169e055ef    2f  a801           test al,0x1
0x367169e055f1    31  0f844e000000   jz 0x367169e05645  <+0x85>
0x367169e055f7    37  488b58ff       REX.W movq rbx,[rax-0x1]
0x367169e055fb    3b  f6430bc0       testb [rbx+0xb],0xc0
0x367169e055ff    3f  0f8545000000   jnz 0x367169e0564a  <+0x8a>
0x367169e05605    45  48bb711549e3c52f0000 REX.W movq rbx,0x2fc5e3491571    ;; object: 0x2fc5e3491571 <String[10]: Uint8Array>
0x367169e0560f    4f  483bd8         REX.W cmpq rbx,rax
0x367169e05612    52  0f840b000000   jz 0x367169e05623  <+0x63>
0x367169e05618    58  498b45c0       REX.W movq rax,[r13-0x40]
0x367169e0561c    5c  488be5         REX.W movq rsp,rbp
0x367169e0561f    5f  5d             pop rbp
0x367169e05620    60  c21000         ret 0x10
0x367169e05623    63  498b45b8       REX.W movq rax,[r13-0x48]
0x367169e05627    67  ebf3           jmp 0x367169e0561c  <+0x5c>
0x367169e05629    69  48bb1067f50000000000 REX.W movq rbx,0xf56710
0x367169e05633    73  33c0           xorl rax,rax
0x367169e05635    75  488b75f8       REX.W movq rsi,[rbp-0x8]
0x367169e05639    79  e8a2efe7ff     call 0x367169c845e0     ;; code: STUB, CEntryStub, minor: 8
0x367169e0563e    7e  eb93           jmp 0x367169e055d3  <+0x13>
0x367169e05640    80  e8bbe9cfff     call 0x367169b04000     ;; deoptimization bailout 0
0x367169e05645    85  e8c0e9cfff     call 0x367169b0400a     ;; deoptimization bailout 1
0x367169e0564a    8a  e8c5e9cfff     call 0x367169b04014     ;; deoptimization bailout 2
0x367169e0564f    8f  e8cae9cfff     call 0x367169b0401e     ;; deoptimization bailout 3
0x367169e05654    94  90             nop
0x367169e05655    95  90             nop
...

Source positions:
 pc offset  position
        26       553
        69      1086

Inlined functions (count = 1)
 0x3ce858ce6719 <SharedFunctionInfo args>

Deoptimization Input Data (deopt points = 4)
 index  bytecode-offset  trampoline_pc     pc
     0               28             80     2f 
     1               15             85     NA 
     2               15             8a     NA 
     3                0             8f     7e 

Safepoints (size = 30)
0x367169e055ef    2f  0000 (sp -> fp)       0
0x367169e0563e    7e  0000 (sp -> fp)       3

RelocInfo (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)).

Copy link
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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
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 number Diff line number Diff 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 Down Expand 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 number Diff line number Diff 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 number Diff line number Diff 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 number Diff line number Diff 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 number Diff line number Diff 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 Down Expand 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 Down Expand 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 Down Expand 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