Skip to content

Commit

Permalink
perf_hooks: fix function wrapped by timerify to work correctly
Browse files Browse the repository at this point in the history
PR-URL: #43330
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
cola119 authored and danielleadams committed Jun 13, 2022
1 parent e1b8c85 commit 112518f
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 15 deletions.
11 changes: 3 additions & 8 deletions lib/internal/perf/timerify.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ const {
isHistogram
} = require('internal/histogram');

const {
isConstructor,
} = internalBinding('util');

const {
codes: {
ERR_INVALID_ARG_TYPE,
Expand Down Expand Up @@ -72,14 +68,13 @@ function timerify(fn, options = kEmptyObject) {
histogram);
}

const constructor = isConstructor(fn);

function timerified(...args) {
const isConstructorCall = new.target !== undefined;
const start = now();
const result = constructor ?
const result = isConstructorCall ?
ReflectConstruct(fn, args, fn) :
ReflectApply(fn, this, args);
if (!constructor && typeof result?.finally === 'function') {
if (!isConstructorCall && typeof result?.finally === 'function') {
return result.finally(
FunctionPrototypeBind(
processComplete,
Expand Down
7 changes: 0 additions & 7 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,6 @@ static void GuessHandleType(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(OneByteString(env->isolate(), type));
}

static void IsConstructor(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsFunction());
args.GetReturnValue().Set(args[0].As<v8::Function>()->IsConstructor());
}

static void ToUSVString(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK_GE(args.Length(), 2);
Expand Down Expand Up @@ -344,7 +339,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(WeakReference::IncRef);
registry->Register(WeakReference::DecRef);
registry->Register(GuessHandleType);
registry->Register(IsConstructor);
registry->Register(ToUSVString);
}

Expand Down Expand Up @@ -384,7 +378,6 @@ void Initialize(Local<Object> target,
env->SetMethodNoSideEffect(target, "getConstructorName", GetConstructorName);
env->SetMethodNoSideEffect(target, "getExternalValue", GetExternalValue);
env->SetMethod(target, "sleep", Sleep);
env->SetMethodNoSideEffect(target, "isConstructor", IsConstructor);

env->SetMethod(target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
Local<Object> constants = Object::New(env->isolate());
Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-performance-function.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,22 @@ const {
});
});
})().then(common.mustCall());

// Regression tests for https://github.com/nodejs/node/issues/40623
{
assert.strictEqual(performance.timerify(function func() {
return 1;
})(), 1);
assert.strictEqual(performance.timerify(function() {
return 1;
})(), 1);
assert.strictEqual(performance.timerify(() => {
return 1;
})(), 1);
class C {}
const wrap = performance.timerify(C);
assert.ok(new wrap() instanceof C);
assert.throws(() => wrap(), {
name: 'TypeError',
});
}

0 comments on commit 112518f

Please sign in to comment.