From 4e27aa9603a281caf8241b1bba8e0f6d57554c57 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Sun, 2 Jul 2017 15:59:23 +0200 Subject: [PATCH] async_hooks: use common emitBefore and emitAfter Timers and nextTick have special emitBefore and emitAfter functions for historic reasons. These function are not needed any more, thus the public emitBefore and emitAfter function can be used. PR-URL: https://github.com/nodejs/node/pull/14050 Reviewed-By: Trevor Norris Reviewed-By: Anna Henningsen --- lib/async_hooks.js | 5 ++-- lib/internal/process/next_tick.js | 31 ++++------------------ lib/timers.js | 29 ++++---------------- test/async-hooks/test-callback-error.js | 15 ++++++----- test/async-hooks/test-emit-before-after.js | 14 +++++----- 5 files changed, 28 insertions(+), 66 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 02cfce0262..54db3ec85d 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -389,9 +389,8 @@ function emitBeforeS(asyncId, triggerAsyncId = asyncId) { pushAsyncIds(asyncId, triggerAsyncId); - if (async_hook_fields[kBefore] === 0) - return; - emitBeforeNative(asyncId); + if (async_hook_fields[kBefore] > 0) + emitBeforeNative(asyncId); } diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index 5b41c4ba09..c063c22cd8 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -56,12 +56,9 @@ function setupNextTick() { // Two arrays that share state between C++ and JS. const { async_hook_fields, async_uid_fields } = async_wrap; // Used to change the state of the async id stack. - const { pushAsyncIds, popAsyncIds } = async_wrap; - // The needed emit*() functions. const { emitInit, emitBefore, emitAfter, emitDestroy } = async_hooks; // Grab the constants necessary for working with internal arrays. - const { kInit, kBefore, kAfter, kDestroy, kAsyncUidCntr } = - async_wrap.constants; + const { kInit, kDestroy, kAsyncUidCntr } = async_wrap.constants; const { async_id_symbol, trigger_id_symbol } = async_wrap; var nextTickQueue = new NextTickQueue(); var microtasksScheduled = false; @@ -149,24 +146,6 @@ function setupNextTick() { } } - // TODO(trevnorris): Using std::stack of Environment::AsyncHooks::ids_stack_ - // is much slower here than was the Float64Array stack used in a previous - // implementation. Problem is the Float64Array stack was a bit brittle. - // Investigate how to harden that implementation and possibly reintroduce it. - function nextTickEmitBefore(asyncId, triggerAsyncId) { - if (async_hook_fields[kBefore] > 0) - emitBefore(asyncId, triggerAsyncId); - else - pushAsyncIds(asyncId, triggerAsyncId); - } - - function nextTickEmitAfter(asyncId) { - if (async_hook_fields[kAfter] > 0) - emitAfter(asyncId); - else - popAsyncIds(asyncId); - } - // Run callbacks that have no domain. // Using domains will cause this to be overridden. function _tickCallback() { @@ -182,7 +161,7 @@ function setupNextTick() { // CHECK(Number.isSafeInteger(tock[trigger_id_symbol])) // CHECK(tock[trigger_id_symbol] > 0) - nextTickEmitBefore(tock[async_id_symbol], tock[trigger_id_symbol]); + emitBefore(tock[async_id_symbol], tock[trigger_id_symbol]); // emitDestroy() places the async_id_symbol into an asynchronous queue // that calls the destroy callback in the future. It's called before // calling tock.callback so destroy will be called even if the callback @@ -200,7 +179,7 @@ function setupNextTick() { // performance hit associated with using `fn.apply()` _combinedTickCallback(args, callback); - nextTickEmitAfter(tock[async_id_symbol]); + emitAfter(tock[async_id_symbol]); if (kMaxCallbacksPerLoop < tickInfo[kIndex]) tickDone(); @@ -227,7 +206,7 @@ function setupNextTick() { // CHECK(Number.isSafeInteger(tock[trigger_id_symbol])) // CHECK(tock[trigger_id_symbol] > 0) - nextTickEmitBefore(tock[async_id_symbol], tock[trigger_id_symbol]); + emitBefore(tock[async_id_symbol], tock[trigger_id_symbol]); // TODO(trevnorris): See comment in _tickCallback() as to why this // isn't a good solution. if (async_hook_fields[kDestroy] > 0) @@ -238,7 +217,7 @@ function setupNextTick() { // performance hit associated with using `fn.apply()` _combinedTickCallback(args, callback); - nextTickEmitAfter(tock[async_id_symbol]); + emitAfter(tock[async_id_symbol]); if (kMaxCallbacksPerLoop < tickInfo[kIndex]) tickDone(); diff --git a/lib/timers.js b/lib/timers.js index 6558bff6c9..caa31459fb 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -34,13 +34,10 @@ const kOnTimeout = TimerWrap.kOnTimeout | 0; const initTriggerId = async_hooks.initTriggerId; // Two arrays that share state between C++ and JS. const { async_hook_fields, async_uid_fields } = async_wrap; -// Used to change the state of the async id stack. -const { pushAsyncIds, popAsyncIds } = async_wrap; // The needed emit*() functions. const { emitInit, emitBefore, emitAfter, emitDestroy } = async_hooks; // Grab the constants necessary for working with internal arrays. -const { kInit, kBefore, kAfter, kDestroy, kAsyncUidCntr } = - async_wrap.constants; +const { kInit, kDestroy, kAsyncUidCntr } = async_wrap.constants; // Symbols for storing async id state. const async_id_symbol = Symbol('asyncId'); const trigger_id_symbol = Symbol('triggerAsyncId'); @@ -149,22 +146,6 @@ exports._unrefActive = function(item) { }; -function timerEmitBefore(asyncId, triggerAsyncId) { - if (async_hook_fields[kBefore] > 0) - emitBefore(asyncId, triggerAsyncId); - else - pushAsyncIds(asyncId, triggerAsyncId); -} - - -function timerEmitAfter(asyncId) { - if (async_hook_fields[kAfter] > 0) - emitAfter(asyncId); - else - popAsyncIds(asyncId); -} - - // The underlying logic for scheduling or re-scheduling a timer. // // Appends a timer onto the end of an existing timers list, or creates a new @@ -318,14 +299,14 @@ function tryOnTimeout(timer, list) { timer[async_id_symbol] : null; var threw = true; if (timerAsyncId !== null) - timerEmitBefore(timerAsyncId, timer[trigger_id_symbol]); + emitBefore(timerAsyncId, timer[trigger_id_symbol]); try { ontimeout(timer); threw = false; } finally { if (timerAsyncId !== null) { if (!threw) - timerEmitAfter(timerAsyncId); + emitAfter(timerAsyncId); if (!timer._repeat && async_hook_fields[kDestroy] > 0 && !timer._destroyed) { emitDestroy(timerAsyncId); @@ -756,7 +737,7 @@ function processImmediate() { // 4.7) what is in this smaller function. function tryOnImmediate(immediate, oldTail) { var threw = true; - timerEmitBefore(immediate[async_id_symbol], immediate[trigger_id_symbol]); + emitBefore(immediate[async_id_symbol], immediate[trigger_id_symbol]); try { // make the actual call outside the try/catch to allow it to be optimized runCallback(immediate); @@ -765,7 +746,7 @@ function tryOnImmediate(immediate, oldTail) { // clearImmediate checks _callback === null for kDestroy hooks. immediate._callback = null; if (!threw) - timerEmitAfter(immediate[async_id_symbol]); + emitAfter(immediate[async_id_symbol]); if (async_hook_fields[kDestroy] > 0 && !immediate._destroyed) { emitDestroy(immediate[async_id_symbol]); immediate._destroyed = true; diff --git a/test/async-hooks/test-callback-error.js b/test/async-hooks/test-callback-error.js index b110e700e6..a4b8a99f33 100644 --- a/test/async-hooks/test-callback-error.js +++ b/test/async-hooks/test-callback-error.js @@ -12,9 +12,9 @@ switch (arg) { oninit: common.mustCall(() => { throw new Error(arg); }) }).enable(); async_hooks.emitInit( - async_hooks.executionAsyncId(), + async_hooks.newUid(), `${arg}_type`, - async_hooks.triggerAsyncId() + async_hooks.executionAsyncId() ); return; @@ -22,12 +22,13 @@ switch (arg) { initHooks({ onbefore: common.mustCall(() => { throw new Error(arg); }) }).enable(); + const newAsyncId = async_hooks.newUid(); async_hooks.emitInit( - async_hooks.executionAsyncId(), + newAsyncId, `${arg}_type`, - async_hooks.triggerAsyncId() + async_hooks.executionAsyncId() ); - async_hooks.emitBefore(async_hooks.executionAsyncId()); + async_hooks.emitBefore(newAsyncId, async_hooks.executionAsyncId()); return; case 'test_callback_abort': @@ -35,9 +36,9 @@ switch (arg) { oninit: common.mustCall(() => { throw new Error(arg); }) }).enable(); async_hooks.emitInit( - async_hooks.executionAsyncId(), + async_hooks.newUid(), `${arg}_type`, - async_hooks.triggerAsyncId() + async_hooks.executionAsyncId() ); return; } diff --git a/test/async-hooks/test-emit-before-after.js b/test/async-hooks/test-emit-before-after.js index f4757a2887..1334767c76 100644 --- a/test/async-hooks/test-emit-before-after.js +++ b/test/async-hooks/test-emit-before-after.js @@ -8,17 +8,19 @@ const initHooks = require('./init-hooks'); switch (process.argv[2]) { case 'test_invalid_async_id': - async_hooks.emitBefore(-1); - break; + async_hooks.emitBefore(-1, 1); + return; case 'test_invalid_trigger_id': async_hooks.emitBefore(1, -1); - break; + return; } +assert.ok(!process.argv[2]); + const c1 = spawnSync(process.execPath, [__filename, 'test_invalid_async_id']); assert.strictEqual(c1.stderr.toString().split(/[\r\n]+/g)[0], 'Error: before(): asyncId or triggerAsyncId is less than ' + - 'zero (asyncId: -1, triggerAsyncId: -1)'); + 'zero (asyncId: -1, triggerAsyncId: 1)'); assert.strictEqual(c1.status, 1); const c2 = spawnSync(process.execPath, [__filename, 'test_invalid_trigger_id']); @@ -32,7 +34,7 @@ const expectedTriggerId = async_hooks.newUid(); const expectedType = 'test_emit_before_after_type'; // Verify that if there is no registered hook, then nothing will happen. -async_hooks.emitBefore(expectedId); +async_hooks.emitBefore(expectedId, expectedTriggerId); async_hooks.emitAfter(expectedId); initHooks({ @@ -42,5 +44,5 @@ initHooks({ }).enable(); async_hooks.emitInit(expectedId, expectedType, expectedTriggerId); -async_hooks.emitBefore(expectedId); +async_hooks.emitBefore(expectedId, expectedTriggerId); async_hooks.emitAfter(expectedId);