Skip to content

Commit 9d26358

Browse files
apapirovskiMylesBorins
apapirovski
authored and
MylesBorins
committed
async_hooks: remove internal only error checking
This error checking is mostly unnecessary and is just a Node core developer nicety, rather than something that is needed for the user-land. It can be safely removed without any practical impact while making nextTick, timers, immediates and AsyncResource substantially faster. PR-URL: #30967 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent d48f592 commit 9d26358

4 files changed

Lines changed: 26 additions & 110 deletions

File tree

‎lib/async_hooks.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88

99
const{
1010
ERR_ASYNC_CALLBACK,
11+
ERR_ASYNC_TYPE,
1112
ERR_INVALID_ASYNC_ID
1213
}=require('internal/errors').codes;
1314
const{ validateString }=require('internal/validators');
@@ -31,6 +32,7 @@ const {
3132
emitBefore,
3233
emitAfter,
3334
emitDestroy,
35+
enabledHooksExist,
3436
initHooksExist,
3537
}=internal_async_hooks;
3638

@@ -157,6 +159,10 @@ class AsyncResource {
157159
this[trigger_async_id_symbol]=triggerAsyncId;
158160

159161
if(initHooksExist()){
162+
if(enabledHooksExist()&&type.length===0){
163+
thrownewERR_ASYNC_TYPE(type);
164+
}
165+
160166
emitInit(asyncId,type,triggerAsyncId,this);
161167
}
162168

‎lib/internal/async_hooks.js‎

Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,10 @@
33
const{
44
Error,
55
FunctionPrototypeBind,
6-
NumberIsSafeInteger,
76
ObjectDefineProperty,
87
Symbol,
98
}=primordials;
109

11-
const{
12-
ERR_ASYNC_TYPE,
13-
ERR_INVALID_ASYNC_ID
14-
}=require('internal/errors').codes;
15-
1610
constasync_wrap=internalBinding('async_wrap');
1711
/* async_hook_fields is a Uint32Array wrapping the uint32_t array of
1812
* Environment::AsyncHooks::fields_[]. Each index tracks the number of active
@@ -117,15 +111,6 @@ function fatalError(e) {
117111
}
118112

119113

120-
functionvalidateAsyncId(asyncId,type){
121-
// Skip validation when async_hooks is disabled
122-
if(async_hook_fields[kCheck]<=0)return;
123-
124-
if(!NumberIsSafeInteger(asyncId)||asyncId<-1){
125-
fatalError(newERR_INVALID_ASYNC_ID(type,asyncId));
126-
}
127-
}
128-
129114
// Emit From Native //
130115

131116
// Used by C++ to call all init() callbacks. Because some state can be setup
@@ -314,6 +299,9 @@ function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
314299
}
315300
}
316301

302+
functionenabledHooksExist(){
303+
returnasync_hook_fields[kCheck]>0;
304+
}
317305

318306
functioninitHooksExist(){
319307
returnasync_hook_fields[kInit]>0;
@@ -329,21 +317,11 @@ function destroyHooksExist() {
329317

330318

331319
functionemitInitScript(asyncId,type,triggerAsyncId,resource){
332-
validateAsyncId(asyncId,'asyncId');
333-
if(triggerAsyncId!==null)
334-
validateAsyncId(triggerAsyncId,'triggerAsyncId');
335-
if(async_hook_fields[kCheck]>0&&
336-
(typeoftype!=='string'||type.length<=0)){
337-
thrownewERR_ASYNC_TYPE(type);
338-
}
339-
340320
// Short circuit all checks for the common case. Which is that no hooks have
341321
// been set. Do this to remove performance impact for embedders (and core).
342322
if(async_hook_fields[kInit]===0)
343323
return;
344324

345-
// This can run after the early return check b/c running this function
346-
// manually means that the embedder must have used getDefaultTriggerAsyncId().
347325
if(triggerAsyncId===null){
348326
triggerAsyncId=getDefaultTriggerAsyncId();
349327
}
@@ -353,12 +331,6 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) {
353331

354332

355333
functionemitBeforeScript(asyncId,triggerAsyncId){
356-
// Validate the ids. An id of -1 means it was never set and is visible on the
357-
// call graph. An id < -1 should never happen in any circumstance. Throw
358-
// on user calls because async state should still be recoverable.
359-
validateAsyncId(asyncId,'asyncId');
360-
validateAsyncId(triggerAsyncId,'triggerAsyncId');
361-
362334
pushAsyncIds(asyncId,triggerAsyncId);
363335

364336
if(async_hook_fields[kBefore]>0)
@@ -367,8 +339,6 @@ function emitBeforeScript(asyncId, triggerAsyncId) {
367339

368340

369341
functionemitAfterScript(asyncId){
370-
validateAsyncId(asyncId,'asyncId');
371-
372342
if(async_hook_fields[kAfter]>0)
373343
emitAfterNative(asyncId);
374344

@@ -377,8 +347,6 @@ function emitAfterScript(asyncId) {
377347

378348

379349
functionemitDestroyScript(asyncId){
380-
validateAsyncId(asyncId,'asyncId');
381-
382350
// Return early if there are no destroy callbacks, or invalid asyncId.
383351
if(async_hook_fields[kDestroy]===0||asyncId<=0)
384352
return;
@@ -418,8 +386,7 @@ function popAsyncIds(asyncId) {
418386
conststackLength=async_hook_fields[kStackLength];
419387
if(stackLength===0)returnfalse;
420388

421-
if(async_hook_fields[kCheck]>0&&
422-
async_id_fields[kExecutionAsyncId]!==asyncId){
389+
if(enabledHooksExist()&&async_id_fields[kExecutionAsyncId]!==asyncId){
423390
// Do the same thing as the native code (i.e. crash hard).
424391
returnpopAsyncIds_(asyncId);
425392
}
@@ -464,6 +431,7 @@ module.exports = {
464431
getOrSetAsyncId,
465432
getDefaultTriggerAsyncId,
466433
defaultTriggerAsyncIdScope,
434+
enabledHooksExist,
467435
initHooksExist,
468436
afterHooksExist,
469437
destroyHooksExist,

‎test/async-hooks/test-emit-before-after.js‎

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,9 @@
33

44
constcommon=require('../common');
55
constassert=require('assert');
6-
constspawnSync=require('child_process').spawnSync;
76
constasync_hooks=require('internal/async_hooks');
87
constinitHooks=require('./init-hooks');
98

10-
if(!common.isMainThread)
11-
common.skip('Worker bootstrapping works differently -> different async IDs');
12-
13-
switch(process.argv[2]){
14-
case'test_invalid_async_id':
15-
async_hooks.emitBefore(-2,1);
16-
return;
17-
case'test_invalid_trigger_id':
18-
async_hooks.emitBefore(1,-2);
19-
return;
20-
}
21-
assert.ok(!process.argv[2]);
22-
23-
24-
constc1=spawnSync(process.execPath,[
25-
'--expose-internals',__filename,'test_invalid_async_id'
26-
]);
27-
assert.strictEqual(
28-
c1.stderr.toString().split(/[\r\n]+/g)[0],
29-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid asyncId value: -2');
30-
assert.strictEqual(c1.status,1);
31-
32-
constc2=spawnSync(process.execPath,[
33-
'--expose-internals',__filename,'test_invalid_trigger_id'
34-
]);
35-
assert.strictEqual(
36-
c2.stderr.toString().split(/[\r\n]+/g)[0],
37-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: -2');
38-
assert.strictEqual(c2.status,1);
39-
409
constexpectedId=async_hooks.newAsyncId();
4110
constexpectedTriggerId=async_hooks.newAsyncId();
4211
constexpectedType='test_emit_before_after_type';
@@ -45,9 +14,22 @@ const expectedType = 'test_emit_before_after_type';
4514
async_hooks.emitBefore(expectedId,expectedTriggerId);
4615
async_hooks.emitAfter(expectedId);
4716

17+
constchkBefore=common.mustCall((id)=>assert.strictEqual(id,expectedId));
18+
constchkAfter=common.mustCall((id)=>assert.strictEqual(id,expectedId));
19+
20+
constcheckOnce=(fn)=>{
21+
letcalled=false;
22+
return(...args)=>{
23+
if(called)return;
24+
25+
called=true;
26+
fn(...args);
27+
};
28+
};
29+
4830
initHooks({
49-
onbefore: common.mustCall((id)=>assert.strictEqual(id,expectedId)),
50-
onafter: common.mustCall((id)=>assert.strictEqual(id,expectedId)),
31+
onbefore: checkOnce(chkBefore),
32+
onafter: checkOnce(chkAfter),
5133
allowNoInit: true
5234
}).enable();
5335

‎test/async-hooks/test-emit-init.js‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
constcommon=require('../common');
55
constassert=require('assert');
6-
constspawnSync=require('child_process').spawnSync;
76
constasync_hooks=require('internal/async_hooks');
87
constinitHooks=require('./init-hooks');
98

@@ -23,45 +22,6 @@ const hooks1 = initHooks({
2322

2423
hooks1.enable();
2524

26-
switch(process.argv[2]){
27-
case'test_invalid_async_id':
28-
async_hooks.emitInit();
29-
return;
30-
case'test_invalid_trigger_id':
31-
async_hooks.emitInit(expectedId);
32-
return;
33-
case'test_invalid_trigger_id_negative':
34-
async_hooks.emitInit(expectedId,expectedType,-2);
35-
return;
36-
}
37-
assert.ok(!process.argv[2]);
38-
39-
40-
constc1=spawnSync(process.execPath,[
41-
'--expose-internals',__filename,'test_invalid_async_id'
42-
]);
43-
assert.strictEqual(
44-
c1.stderr.toString().split(/[\r\n]+/g)[0],
45-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid asyncId value: undefined');
46-
assert.strictEqual(c1.status,1);
47-
48-
constc2=spawnSync(process.execPath,[
49-
'--expose-internals',__filename,'test_invalid_trigger_id'
50-
]);
51-
assert.strictEqual(
52-
c2.stderr.toString().split(/[\r\n]+/g)[0],
53-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: undefined');
54-
assert.strictEqual(c2.status,1);
55-
56-
constc3=spawnSync(process.execPath,[
57-
'--expose-internals',__filename,'test_invalid_trigger_id_negative'
58-
]);
59-
assert.strictEqual(
60-
c3.stderr.toString().split(/[\r\n]+/g)[0],
61-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: -2');
62-
assert.strictEqual(c3.status,1);
63-
64-
6525
async_hooks.emitInit(expectedId,expectedType,expectedTriggerId,
6626
expectedResource);
6727

0 commit comments

Comments
 (0)