Skip to content

Commit

Permalink
async_hooks: use common emitBefore and emitAfter
Browse files Browse the repository at this point in the history 
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: #14050
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
@AndreasMadsen @addaleax
AndreasMadsen authored and addaleax committed Jul 18, 2017
1 parent b799498 commit 9f66f19
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 66 deletions.
5 changes: 2 additions & 3 deletions lib/async_hooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down
31 changes: 5 additions & 26 deletions lib/internal/process/next_tick.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand All @@ -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();
Expand All @@ -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)
Expand All @@ -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();
Expand Down
29 changes: 5 additions & 24 deletions lib/timers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
15 changes: 8 additions & 7 deletions test/async-hooks/test-callback-error.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,32 +12,33 @@ 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;

case 'test_callback':
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':
initHooks({
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;
}
Expand Down
14 changes: 8 additions & 6 deletions test/async-hooks/test-emit-before-after.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand All @@ -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({
Expand All @@ -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);

0 comments on commit 9f66f19

Please sign in to comment.