MakeCallback is very slow

[ronag]

Calling into JS from CPP is very slow. Is this due to async hooks? Could we disable that with a global option if the user doesn't need async hooks?

[joyeecheung]

See past discussions nodejs/node#10101 - I think the conclusion was that it wasn't that slow, but that was measured 6 years ago and before V8 rolled out several architectural redesigns. We have also deprecated domains for a long time. Maybe the situations have changed now. Would be good to have some new measurements to see if it is worth a shot making it some kind of optional thing.

[anonrig]

Could we disable that with a global option if the user doesn't need async hooks?

Can you explain this in detail? I'm trying to understand the current implementation, and a quick explanation could/would save a lot of time :)

[ronag]

Could we disable that with a global option if the user doesn't need async hooks?

Can you explain this in detail? I'm trying to understand the current implementation, and a quick explanation could/would save a lot of time :)

I don't know much about the details. Async hooks are used to propagate async context. It has a constant overhead even if you don't use it.

[mcollina]

async_hooks have a tiny overhead if disabled and a significant overhead if enabled.

[ronag]

async_hooks have a tiny overhead if disabled and a significant overhead if enabled.

Is it possible to disable it?

[mcollina]

Not really. That's described in the linked issue. Maybe a compile-time flag could do it but we are talking ns of difference.

[abenhamdine]

see also previous related work in
nodejs/node#41331
nodejs/node#41279
some big picture datapoints here, related to uWebSockets module : uNetworking/uWebSockets.js#341

[benjamingr]

Interesting stuff (though bad manners) at uNetworking/uWebSockets.js#904 namely that experiment (that does break async_hooks but also improves performance)

[H4ad]

I could be wrong, but once this nodejs/node#48528 is merged, we can probably rewrite MakeCallback to use AsyncContextFrame, right? Assuming AsyncContextFrame is faster than async_hooks.

[benjamingr]

@H4ad that would work for people using AsyncLocalStorage but not for people otherwise using async hooks

[Qard]

I've been wanting to split all the async_hooks stuff in C++ out into a delegate class which we could use a no-op version for until a hook is registered. That would make it easier to at least contain the overhead when it's not in-use.

[joyeecheung]

My understanding (or, from nodejs/node#10101) is that async hooks do not actually contribute much to the overhead when it's not enabled - and it's opt-in already so users would be willingly taking the hit. Most of the overhead comes from the tick callback for process.nextTick. While it also only incurs a lot of overhead when actually used, due to the prevalence of its usage in Node.js internals and the user land, the overhead is also much more prevalent. Doing something about async hooks helps a bit, but it's also only a very small piece of the puzzle because it's actually not used that much in the user land. process.nextTick() is the real beast here that affect almost all use cases.

[Qard]

Yes, mostly. Though promises are an order of magnitude larger overhead than tick or timer callbacks. The overhead is non-zero though, so in a hot path which calls MakeCallback often it could make a difference to optimize it with that no-op delegate I mentioned. Not sure if/when I'll get time to do something about that but I'd like to try it at some point. 🤔