Timeline for V8 fast-calls header exposed for native addons?

uNetworkingAB commented 5 months ago

What is the problem this feature will solve?

Fast calls in V8 was introduced 4 years ago, soon to be half a decade ago. These features are still not exposed to native addon authors due to "it may change"-argumentation (see previous threads).

When can these features be freely used by the ecosystem as they see fit? Are we supposed to wait half a decade more? There clearly are many useful cases where it dramatically helps performance.

Why not just let the ecosystem adopt them as they see usable? We already have gone through 10+ years of V8 breaking API changes from version to version so none of this is news to any of us making native addons - V8 has never been stable in its API, not even the early wrappers were ever stable.

It always changed over time, so to treat fast calls differently than any other potential API change makes no sense. Esp. when it holds back performance innovation that technically already is available.

What is the feature you are proposing to solve the problem?

To include the v8-fast-api-calls.h so that native addons can use them.

What alternatives have you considered?

Hacking it in myself, on my own end

joyeecheung commented 5 months ago

I am pretty sure it's not distributed in the tarball simply because no-one remembered to add them to the list in https://github.com/nodejs/node/blob/dab85bdc066f4fb8cbf1ca5db5d0254898edb7d7/tools/install.py#L199 ? But @targos may know more.

targos commented 5 months ago

It was asked by the V8 team that we didn't expose it: https://github.com/nodejs/node/pull/37570#issuecomment-793436245

I don't object to add it if the situation is better now.

joyeecheung commented 5 months ago

Although speaking of stability it seems the V8 team is planning to do some changes to it again (there are 3 design docs in flight right now) although I am not sure if any of them lead to API/ABI breakages:

https://docs.google.com/document/d/1mrhpObtu1M8fKn76i18bVflYGO7Tjh36yNeQtT04aAs/edit?resourcekey=0-O-kp9c945JKQP-sPaOUT0Q#heading=h.i5ibcxqde9h2

https://docs.google.com/document/d/1QAibq4jcYc46pwBUi4VzkAABpTYD78XJSvEWpmn7GHE/edit#heading=h.n1atlriavj6v

benjamingr commented 5 months ago

Hey Alex,

I believe the project is in a much more performance-oriented state than it was when you last engaged and there are more people like Yagiz and Daniel involved invested in improving performance related C++ stuff than before.

That said, this is still an alt account after your primary one was blocked. This whole saga is needlessly adversarial. We are happy to interact constructively towards a faster better Node.

I see your edits on the original issue. I recommend you:

Apologize for past behavior (almost a decade ago at this point) and state you won't violate the CoC and engage constructively.
Set up a meeting with the Node.js performance people like Yagiz and Daniel. I am sure you will be able to teach them a lot about the current bottlenecks and how to improve them. You will need to be patient as most of them don't have the context you do.

I believe you will be surprised at what can be accomplished at the current state quickly. Performance is getting a lot of love and things that got pushback in the past are a lot more likely to land with current project leadership.

In particular I think opting-out of async_hooks related overhead in MakeCallback and other "dangerous" changes from the past are on the table.

As for the actual change, V8 had 2 years to bake. Maya is no longer at Google IIRC, maybe @syg can assist with regards to if this is fine to expose?

uNetworkingAB commented 5 months ago

Exposing this header would unlock major improvements to addon performance for several cases.

The volatility/instability argument makes no sense in my book, as I've already seen V8 APIs massively break, continuously, for 10 years. So why would it be any different this time? It wouldn't.

We all know V8 is an unstable API and always has been. Its part of the charm.

uNetworkingAB commented 5 months ago

@joyeecheung Those are interesting reads but don't change the fact V8 has always been API-breaking. This measurement they did is interesting:

// M1 Start measuring noop_maybe_fast: noop_maybe_fast: 103.16700000000003 ←—-- Start measuring noop_always_slow: noop_always_slow: 299.08399999999995

A noop function is 3x the performance with fast calls vs. ordinary ones.

joyeecheung commented 5 months ago

I don’t think there is need to explain the performance need here since that’s already used in Node.js. I would say it’s less up to Node.js but more up to the V8 team to gauge whether it’s fine to be included in the tarball now, as the maintenance burden of doing so is more on their side (there is another burden of ABI stability on Node.js’s side but AFAICT there are many other parts that make it difficult to upgrade V8 in a release line already).

uNetworkingAB commented 5 months ago

Simply copying the singular v8-fast-api-calls.h for the major Node.js version into the repo worked and I now have precompiled binaries with fast API calls confirmed working.

Technically I could keep doing this, having each major version of v8-fast-api-calls.h per each major version of Node.js built for, but it would be a lot simpler if the header dist just included it.

nodejs / node