Skip to content

Commit

Permalink
util,assert: improve performance
Browse files Browse the repository at this point in the history
This significantly improves regular and typed array performance by
not checking the indices keys anymore. This can be done with a V8
API that allows to only retrieve the non indices property keys.

PR-URL: #22197
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
BridgeAR authored and targos committed Sep 3, 2018
1 parent c17e980 commit 2b1cb3b
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 44 deletions.
81 changes: 37 additions & 44 deletions lib/internal/util/comparisons.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { compare } = process.binding('buffer');
const { isArrayBufferView } = require('internal/util/types');
const { internalBinding } = require('internal/bootstrap/loaders');
const { isDate, isMap, isRegExp, isSet } = internalBinding('types');
const { getOwnNonIndexProperties } = process.binding('util');

const ReflectApply = Reflect.apply;

Expand Down Expand Up @@ -106,24 +107,11 @@ function strictDeepEqual(val1, val2, memos) {
if (val1.length !== val2.length) {
return false;
}
const keys = objectKeys(val1);
if (keys.length !== objectKeys(val2).length) {
const keys1 = getOwnNonIndexProperties(val1);
if (keys1.length !== getOwnNonIndexProperties(val2).length) {
return false;
}
// Fast path for non sparse arrays (no key comparison for indices
// properties).
// See https://tc39.github.io/ecma262/#sec-ordinaryownpropertykeys
if (val1.length === keys.length) {
if (keys.length === 0 || keys[val1.length - 1] === `${val1.length - 1}`) {
return keyCheck(val1, val2, kStrict, memos, kIsArray, []);
}
} else if (keys.length > val1.length &&
keys[val1.length - 1] === `${val1.length - 1}`) {
const minimalKeys = keys.slice(val1.length);
return keyCheck(val1, val2, kStrict, memos, kIsArray, minimalKeys);
}
// Only set this to kIsArray in case the array is not sparse!
return keyCheck(val1, val2, kStrict, memos, kNoIterator, keys);
return keyCheck(val1, val2, kStrict, memos, kIsArray, keys1);
}
if (val1Tag === '[object Object]') {
return keyCheck(val1, val2, kStrict, memos, kNoIterator);
Expand All @@ -148,18 +136,14 @@ function strictDeepEqual(val1, val2, memos) {
if (!areSimilarTypedArrays(val1, val2)) {
return false;
}
// Buffer.compare returns true, so val1.length === val2.length
// if they both only contain numeric keys, we don't need to exam further.
const keys = objectKeys(val1);
if (keys.length !== objectKeys(val2).length) {
// Buffer.compare returns true, so val1.length === val2.length. If they both
// only contain numeric keys, we don't need to exam further than checking
// the symbols.
const keys1 = getOwnNonIndexProperties(val1);
if (keys1.length !== getOwnNonIndexProperties(val2).length) {
return false;
}
if (keys.length === val1.length) {
return keyCheck(val1, val2, kStrict, memos, kNoIterator, []);
}
// Only compare the special keys.
const minimalKeys = keys.slice(val1.length);
return keyCheck(val1, val2, kStrict, memos, kNoIterator, minimalKeys);
return keyCheck(val1, val2, kStrict, memos, kNoIterator, keys1);
} else if (isSet(val1)) {
if (!isSet(val2) || val1.size !== val2.size) {
return false;
Expand All @@ -173,21 +157,10 @@ function strictDeepEqual(val1, val2, memos) {
// TODO: Make the valueOf checks safe.
} else if (typeof val1.valueOf === 'function') {
const val1Value = val1.valueOf();
if (val1Value !== val1) {
if (typeof val2.valueOf !== 'function') {
return false;
}
if (!innerDeepEqual(val1Value, val2.valueOf(), true))
return false;
// Fast path for boxed primitive strings.
if (typeof val1Value === 'string') {
const keys = objectKeys(val1);
if (keys.length !== objectKeys(val2).length) {
return false;
}
const minimalKeys = keys.slice(val1.length);
return keyCheck(val1, val2, kStrict, memos, kNoIterator, minimalKeys);
}
if (val1Value !== val1 &&
(typeof val2.valueOf !== 'function' ||
!innerDeepEqual(val1Value, val2.valueOf(), kStrict))) {
return false;
}
}
return keyCheck(val1, val2, kStrict, memos, kNoIterator);
Expand Down Expand Up @@ -277,7 +250,7 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
}
}

if (strict) {
if (strict && arguments.length === 5) {
const symbolKeysA = getOwnPropertySymbols(val1);
if (symbolKeysA.length !== 0) {
let count = 0;
Expand Down Expand Up @@ -310,7 +283,7 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
if (aKeys.length === 0 &&
(iterationType === kNoIterator ||
iterationType === kIsArray && val1.length === 0 ||
val1.size === 0)) {
val1.size === 0)) {
return true;
}

Expand Down Expand Up @@ -578,8 +551,28 @@ function objEquiv(a, b, strict, keys, memos, iterationType) {
}
} else if (iterationType === kIsArray) {
for (; i < a.length; i++) {
if (!innerDeepEqual(a[i], b[i], strict, memos)) {
if (hasOwnProperty(a, i)) {
if (!hasOwnProperty(b, i) ||
!innerDeepEqual(a[i], b[i], strict, memos)) {
return false;
}
} else if (hasOwnProperty(b, i)) {
return false;
} else {
// Array is sparse.
const keysA = objectKeys(a);
i++;
for (; i < keysA.length; i++) {
const key = keysA[i];
if (!hasOwnProperty(b, key) ||
!innerDeepEqual(a[key], b[i], strict, memos)) {
return false;
}
}
if (keysA.length !== objectKeys(b).length) {
return false;
}
return true;
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,29 @@ using v8::Proxy;
using v8::String;
using v8::Value;

static void GetOwnNonIndexProperties(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Local<Context> context = env->context();

if (!args[0]->IsObject())
return;

v8::Local<v8::Object> object = args[0].As<v8::Object>();

// Return only non-enumerable properties by default.
v8::Local<v8::Array> properties;

if (!object->GetPropertyNames(
context, v8::KeyCollectionMode::kOwnOnly,
v8::ONLY_ENUMERABLE,
v8::IndexFilter::kSkipIndices)
.ToLocal(&properties)) {
return;
}
args.GetReturnValue().Set(properties);
}

static void GetPromiseDetails(const FunctionCallbackInfo<Value>& args) {
// Return undefined if it's not a Promise.
if (!args[0]->IsPromise())
Expand Down Expand Up @@ -177,6 +200,8 @@ void Initialize(Local<Object> target,
env->SetMethodNoSideEffect(target, "getProxyDetails", GetProxyDetails);
env->SetMethodNoSideEffect(target, "safeToString", SafeToString);
env->SetMethodNoSideEffect(target, "previewEntries", PreviewEntries);
env->SetMethodNoSideEffect(target, "getOwnNonIndexProperties",
GetOwnNonIndexProperties);

env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog);
env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog);
Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,17 @@ assert.throws(() => assert.deepStrictEqual(new Boolean(true), {}),
' [\n 1,\n- 2\n+ 2,\n+ 3\n ]' }
);
util.inspect.defaultOptions = tmp;

const invalidTrap = new Proxy([1, 2, 3], {
ownKeys() { return []; }
});
assert.throws(
() => assert.deepStrictEqual(invalidTrap, [1, 2, 3]),
{
name: 'TypeError',
message: "'ownKeys' on proxy: trap result did not include 'length'"
}
);
}

// Basic valueOf check.
Expand Down

0 comments on commit 2b1cb3b

Please sign in to comment.