Skip to content

Commit

Permalink
util: improve performance inspecting proxies
Browse files Browse the repository at this point in the history
This makes sure we do not retrieve the handler in case it's not
required. This improves the performance a tiny bit for these cases.

Backport-PR-URL: #31431
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
BridgeAR authored and MylesBorins committed Jan 30, 2020
1 parent a1555cb commit a309ee1
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 11 deletions.
2 changes: 1 addition & 1 deletion benchmark/util/inspect-proxy.js
Expand Up @@ -4,7 +4,7 @@ const util = require('util');
const common = require('../common.js');

const bench = common.createBenchmark(main, {
n: [2e4],
n: [1e5],
showProxy: [0, 1],
isProxy: [0, 1]
});
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/util/inspect.js
Expand Up @@ -640,12 +640,12 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
const context = value;
// Always check for proxies to prevent side effects and to prevent triggering
// any proxy handlers.
const proxy = getProxyDetails(value);
const proxy = getProxyDetails(value, !!ctx.showProxy);
if (proxy !== undefined) {
if (ctx.showProxy) {
return formatProxy(ctx, proxy, recurseTimes);
}
value = proxy[0];
value = proxy;
}

// Provide a hook for user-specified inspect functions.
Expand Down
20 changes: 14 additions & 6 deletions src/node_util.cc
Expand Up @@ -91,15 +91,23 @@ static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
if (!args[0]->IsProxy())
return;

CHECK(args[1]->IsBoolean());

Local<Proxy> proxy = args[0].As<Proxy>();

Local<Value> ret[] = {
proxy->GetTarget(),
proxy->GetHandler()
};
if (args[1]->IsTrue()) {
Local<Value> ret[] = {
proxy->GetTarget(),
proxy->GetHandler()
};

args.GetReturnValue().Set(
Array::New(args.GetIsolate(), ret, arraysize(ret)));
args.GetReturnValue().Set(
Array::New(args.GetIsolate(), ret, arraysize(ret)));
} else {
Local<Value> ret = proxy->GetTarget();

args.GetReturnValue().Set(ret);
}
}

static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {
Expand Down
7 changes: 5 additions & 2 deletions test/parallel/test-util-inspect-proxy.js
Expand Up @@ -43,10 +43,13 @@ util.inspect(proxyObj, opts);

// getProxyDetails is an internal method, not intended for public use.
// This is here to test that the internals are working correctly.
const details = processUtil.getProxyDetails(proxyObj);
let details = processUtil.getProxyDetails(proxyObj, true);
assert.strictEqual(target, details[0]);
assert.strictEqual(handler, details[1]);

details = processUtil.getProxyDetails(proxyObj, false);
assert.strictEqual(target, details);

assert.strictEqual(
util.inspect(proxyObj, opts),
'Proxy [\n' +
Expand Down Expand Up @@ -105,7 +108,7 @@ const expected6 = 'Proxy [\n' +
' ]\n' +
']';
assert.strictEqual(
util.inspect(proxy1, { showProxy: true, depth: null }),
util.inspect(proxy1, { showProxy: 1, depth: null }),
expected1);
assert.strictEqual(util.inspect(proxy2, opts), expected2);
assert.strictEqual(util.inspect(proxy3, opts), expected3);
Expand Down

0 comments on commit a309ee1

Please sign in to comment.