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.

PR-URL: #30767
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
BridgeAR authored and targos committed Dec 9, 2019
1 parent ad4d52d commit eeaeb51
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -638,12 +638,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 eeaeb51

Please sign in to comment.