From 58f011ffdd2535312c7e3c2b93892cfea6afcc86 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 18 Jun 2017 02:36:37 +0200 Subject: [PATCH 1/7] async_hooks: reduce duplication with factory --- lib/async_hooks.js | 79 +++++++++++++--------------------------------- 1 file changed, 22 insertions(+), 57 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index e1029c97a57eec..faa02c17d60326 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -49,6 +49,9 @@ const init_symbol = Symbol('init'); const before_symbol = Symbol('before'); const after_symbol = Symbol('after'); const destroy_symbol = Symbol('destroy'); +const emitBeforeN = hookFactory(before_symbol); +const emitAfterN = hookFactory(after_symbol); +const emitDestroyN = hookFactory(destroy_symbol); // Setup the callbacks that node::AsyncWrap will call when there are hooks to // process. They use the same functions as the JS embedder API. These callbacks @@ -342,24 +345,28 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) { } } - -function emitBeforeN(asyncId) { - processing_hook = true; - // Use a single try/catch for all hook to avoid setting up one per iteration. - try { - for (var i = 0; i < active_hooks_array.length; i++) { - if (typeof active_hooks_array[i][before_symbol] === 'function') { - active_hooks_array[i][before_symbol](asyncId); +function hookFactory(symbol) { + // Called from native. The asyncId stack handling is taken care of there + // before this is called. + return function(asyncId) { + processing_hook = true; + // Use a single try/catch for all hook to avoid setting up one per + // iteration. + try { + for (var i = 0; i < active_hooks_array.length; i++) { + if (typeof active_hooks_array[i][symbol] === 'function') { + active_hooks_array[i][symbol](asyncId); + } } + } catch (e) { + fatalError(e); } - } catch (e) { - fatalError(e); - } - processing_hook = false; + processing_hook = false; - if (tmp_active_hooks_array !== null) { - restoreTmpHooks(); - } + if (tmp_active_hooks_array !== null) { + restoreTmpHooks(); + } + }; } @@ -383,28 +390,6 @@ function emitBeforeS(asyncId, triggerAsyncId = asyncId) { } -// Called from native. The asyncId stack handling is taken care of there before -// this is called. -function emitAfterN(asyncId) { - processing_hook = true; - // Use a single try/catch for all hook to avoid setting up one per iteration. - try { - for (var i = 0; i < active_hooks_array.length; i++) { - if (typeof active_hooks_array[i][after_symbol] === 'function') { - active_hooks_array[i][after_symbol](asyncId); - } - } - } catch (e) { - fatalError(e); - } - processing_hook = false; - - if (tmp_active_hooks_array !== null) { - restoreTmpHooks(); - } -} - - // TODO(trevnorris): Calling emitBefore/emitAfter from native can't adjust the // kIdStackIndex. But what happens if the user doesn't have both before and // after callbacks. @@ -425,26 +410,6 @@ function emitDestroyS(asyncId) { } -function emitDestroyN(asyncId) { - processing_hook = true; - // Use a single try/catch for all hook to avoid setting up one per iteration. - try { - for (var i = 0; i < active_hooks_array.length; i++) { - if (typeof active_hooks_array[i][destroy_symbol] === 'function') { - active_hooks_array[i][destroy_symbol](asyncId); - } - } - } catch (e) { - fatalError(e); - } - processing_hook = false; - - if (tmp_active_hooks_array !== null) { - restoreTmpHooks(); - } -} - - // Emit callbacks for native calls. Since some state can be setup directly from // C++ there's no need to perform all the work here. From 4c00eee74233e2423b3c53f0815b27d447310ba4 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 18 Jun 2017 03:25:41 +0200 Subject: [PATCH 2/7] async_hooks: add TODO entry --- lib/async_hooks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index faa02c17d60326..7b08e8fc8fe4e9 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -328,8 +328,8 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) { triggerAsyncId = initTriggerId(); } - // I'd prefer allowing these checks to not exist, or only throw in a debug - // build, in order to improve performance. + // TODO(trevnorris): I'd prefer allowing these checks to not exist, or only + // throw in a debug build, in order to improve performance. if (!Number.isSafeInteger(asyncId) || asyncId < 0) throw new RangeError('asyncId must be an unsigned integer'); if (typeof type !== 'string' || type.length <= 0) From 8a8117ef07d1c3da2a5b704dca0781941985675b Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 21 Jun 2017 14:20:43 +0200 Subject: [PATCH 3/7] rename factory --- lib/async_hooks.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 7b08e8fc8fe4e9..ac6c8ee5729c63 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -49,9 +49,9 @@ const init_symbol = Symbol('init'); const before_symbol = Symbol('before'); const after_symbol = Symbol('after'); const destroy_symbol = Symbol('destroy'); -const emitBeforeN = hookFactory(before_symbol); -const emitAfterN = hookFactory(after_symbol); -const emitDestroyN = hookFactory(destroy_symbol); +const emitBeforeN = emitHookFactory(before_symbol); +const emitAfterN = emitHookFactory(after_symbol); +const emitDestroyN = emitHookFactory(destroy_symbol); // Setup the callbacks that node::AsyncWrap will call when there are hooks to // process. They use the same functions as the JS embedder API. These callbacks @@ -345,7 +345,7 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) { } } -function hookFactory(symbol) { +function emitHookFactory(symbol) { // Called from native. The asyncId stack handling is taken care of there // before this is called. return function(asyncId) { From 8918fc88481fbbf2a249c2f44cb4aca476103548 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 30 Jun 2017 14:36:17 +0200 Subject: [PATCH 4/7] address comment: use named functions --- lib/async_hooks.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index ac6c8ee5729c63..513e5afa888f7d 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -49,9 +49,9 @@ const init_symbol = Symbol('init'); const before_symbol = Symbol('before'); const after_symbol = Symbol('after'); const destroy_symbol = Symbol('destroy'); -const emitBeforeN = emitHookFactory(before_symbol); -const emitAfterN = emitHookFactory(after_symbol); -const emitDestroyN = emitHookFactory(destroy_symbol); +const emitBeforeN = emitHookFactory(before_symbol, 'emitBeforeN'); +const emitAfterN = emitHookFactory(after_symbol, 'emitAfterN'); +const emitDestroyN = emitHookFactory(destroy_symbol, 'emitDestroyN'); // Setup the callbacks that node::AsyncWrap will call when there are hooks to // process. They use the same functions as the JS embedder API. These callbacks @@ -345,10 +345,10 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) { } } -function emitHookFactory(symbol) { +function emitHookFactory(symbol, name) { // Called from native. The asyncId stack handling is taken care of there // before this is called. - return function(asyncId) { + const fn = function(asyncId) { processing_hook = true; // Use a single try/catch for all hook to avoid setting up one per // iteration. @@ -367,6 +367,10 @@ function emitHookFactory(symbol) { restoreTmpHooks(); } }; + Object.defineProperty(fn, 'name', { + value: name + }); + return fn; } From e4cd11029f725b848235aa7331cdd82607da33fb Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 3 Jul 2017 14:16:42 +0200 Subject: [PATCH 5/7] fixup func-style --- lib/async_hooks.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 513e5afa888f7d..4afb3bba72bcc0 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -348,6 +348,7 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) { function emitHookFactory(symbol, name) { // Called from native. The asyncId stack handling is taken care of there // before this is called. + // eslint-disable-next-line func-style const fn = function(asyncId) { processing_hook = true; // Use a single try/catch for all hook to avoid setting up one per From 795358aa31f7a7cf3f5154440b266a3fd3c0054a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 3 Jul 2017 14:15:45 +0200 Subject: [PATCH 6/7] fixup rename function --- lib/async_hooks.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 4afb3bba72bcc0..e10a7e0edc06de 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -49,18 +49,18 @@ const init_symbol = Symbol('init'); const before_symbol = Symbol('before'); const after_symbol = Symbol('after'); const destroy_symbol = Symbol('destroy'); -const emitBeforeN = emitHookFactory(before_symbol, 'emitBeforeN'); -const emitAfterN = emitHookFactory(after_symbol, 'emitAfterN'); -const emitDestroyN = emitHookFactory(destroy_symbol, 'emitDestroyN'); +const emitBeforeNative = emitHookFactory(before_symbol, 'emitBeforeNative'); +const emitAfterNative = emitHookFactory(after_symbol, 'emitAfterNative'); +const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative'); // Setup the callbacks that node::AsyncWrap will call when there are hooks to // process. They use the same functions as the JS embedder API. These callbacks // are setup immediately to prevent async_wrap.setupHooks() from being hijacked // and the cost of doing so is negligible. async_wrap.setupHooks({ init, - before: emitBeforeN, - after: emitAfterN, - destroy: emitDestroyN }); + before: emitBeforeNative, + after: emitAfterNative, + destroy: emitDestroyNative }); // Used to fatally abort the process if a callback throws. function fatalError(e) { @@ -391,7 +391,7 @@ function emitBeforeS(asyncId, triggerAsyncId = asyncId) { if (async_hook_fields[kBefore] === 0) return; - emitBeforeN(asyncId); + emitBeforeNative(asyncId); } @@ -400,7 +400,7 @@ function emitBeforeS(asyncId, triggerAsyncId = asyncId) { // after callbacks. function emitAfterS(asyncId) { if (async_hook_fields[kAfter] > 0) - emitAfterN(asyncId); + emitAfterNative(asyncId); popAsyncIds(asyncId); } From fc397f81eb2f3be34d154a1d3bff8e04d33e9e11 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 3 Jul 2017 14:17:01 +0200 Subject: [PATCH 7/7] fixup add comment about naming fn --- lib/async_hooks.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index e10a7e0edc06de..d5c7c116434c0b 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -368,6 +368,8 @@ function emitHookFactory(symbol, name) { restoreTmpHooks(); } }; + // Set the name property of the anonymous function as it looks good in the + // stack trace. Object.defineProperty(fn, 'name', { value: name });