Expensive isTraceHTTPEnable

[H4ad]

Problem

I was doing some performance analysis on my lib and I found the following trace:

The method isTraceHTTPEnable is consuming 58ms to check if the trace is enabled when I process 100k requests with total time of ~7s.

To check if tracing is enabled, this method calls a method inside internalBinding('trace_events') to return a boolean, so this slowdown is likely caused by data being passed from the C++ layer to JavaScript.

Reproduction

You can reproduce this CPU Profile and learn how to visualize in this repo.

Solutions

The first solution I thought of was to use v8 Fast APIs, I don't know much about that, but from what I've read in the docs, since the return value is just a boolean, I think it's eligible to be used.

I also thought about caching these values but that won't work since the user can enable and disable tracing whenever he wants, as per the documentation

If you think is possible to use v8 Fast APIs, I can try to work on a PR.

[ronag]

@nodejs/performance @nodejs/http

[ronag]

I don't know if I'd consider it that expensive. But yea, that could be faster.

[mcollina]

This can be cached/hoisted easily in jsland (by patching the enable function). It would be a good PR.

Good finding @ronag!

[tniessen]

To check if tracing is enabled, this method calls a method inside internalBinding('trace_events') to return a boolean, so this slowdown is likely caused by data being passed from the C++ layer to JavaScript.

The first solution I thought of was to use v8 Fast APIs, I don't know much about that, but from what I've read in the docs, since the return value is just a boolean, I think it's eligible to be used.

FWIW, the function is not really part of internalBinding('trace_events') but rather part of V8. I am not aware of a way to use V8 Fast API calls with V8 built-ins, but if it is possible, maybe V8 would appreciate a patch upstream.

[bnoordhuis]

This can be cached/hoisted easily in jsland (by patching the enable function)

That's probably not sound in general. For example, an embedder can enable the node.http trace category directly through V8's C++ API.

W.r.t. V8's fast API: isTraceCategoryEnabled() and trace() are already V8 builtins. There's probably not much room for improvement there.

What should work is expose the uint8_t pointer from platform->GetTracingController()->GetCategoryGroupEnabled("node.http") as a single-element Uint8Array and change isTraceHTTPEnabled() to:

function isTraceHTTPEnabled() {
  return array[0] > 0; // where |array| is that Uint8Array
}

[H4ad]

@tniessen I am not aware of a way to use V8 Fast API calls with V8 built-ins

I tried code search but I didn't find any use inside built-ins so maybe not a use -case for that.