GeezerVison

CI: https://ci.nodejs.org/job/node-test-pull-request/55358/

CI: https://ci.nodejs.org/job/node-test-pull-request/55372/

I haven't kept up with fast calls lately but originally the double return type was only supported on some architectures. Do we know that this is actually optimizing the call everywhere?

Haven’t really looked into if it’s always optimized but I think fast APIs are not guaranteed to always be hit anyway, that’s why the slow fallback callbacks are required.

@joyeecheung i think my only concern would be whether this was, in practice, a regression on any of our supported platforms. maybe a benchmark run could double check?

We only have benchmark CI on Ubuntu x64 but here is a CI run https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1469/

i think my only concern would be whether this was, in practice, a regression on any of our supported platforms. maybe a benchmark run could double check?

JSCallReducer should just skip generating fast calls for signatures that are not supported on the running platform, that's why the slow callback is required. If a regression can be caused by running a fast call on a platform it cannot optimize, that sounds more like an internal bug of V8...

From benchmark CI:

10:04:57                             confidence improvement accuracy (*)    (**)   (***)
10:04:57 perf_hooks/now.js n=1000000         **     12.90 %       ±8.03% ±10.75% ±14.13%

CI: https://ci.nodejs.org/job/node-test-pull-request/55440/

CI: https://ci.nodejs.org/job/node-test-pull-request/55444/

CI: https://ci.nodejs.org/job/node-test-pull-request/55458/

CI: https://ci.nodejs.org/job/node-test-pull-request/55471/

CI: https://ci.nodejs.org/job/node-test-pull-request/55648/

CI: https://ci.nodejs.org/job/node-test-pull-request/55659/

CI: https://ci.nodejs.org/job/node-test-pull-request/55666/

CI: https://ci.nodejs.org/job/node-test-pull-request/55726/

Landed in 8c7fe47