Skip to content

ffi: accept safe integer numbers for 64-bit arguments - #66197

Open
HoonDongKang wants to merge 3 commits into
nodejs:mainfrom
HoonDongKang:ffi/accept-safe-integer-for-64-bit
Open

HoonDongKang wants to merge 3 commits into
nodejs:mainfrom
HoonDongKang:ffi/accept-safe-integer-for-64-bit

Conversation

@HoonDongKang

@HoonDongKang HoonDongKang commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Accept safe integer numbers for int64/uint64 arguments alongside bigint values, allowing buffer lengths to be passed without explicit BigInt() conversions.

-functions.sum_buffer(bytes, BigInt(bytes.byteLength));
+functions.sum_buffer(bytes, bytes.byteLength);

Changes

  • Accept safe integer numbers for int64 and non-negative safe integer numbers for uint64.
  • Validate and convert arguments across Fast API, SharedBuffer, and generic C++ conversion paths.
  • Preserve existing bigint range checks and bigint return values.
  • Document the accepted number ranges.

Tests

The tests check that number and bigint arguments produce the same results, including when used together. A new single-argument test covers Fast API conversion before and after requesting V8 optimization.

Separate nine-argument tests exercise SharedBuffer conversion and generic fallback. Both check Number and BigInt boundaries, invalid inputs, and error messages.

// SharedBuffer: validates and converts the integer in JavaScript.
functions.passthrough_i64_9(0n, value, ...zeros);

// Generic fallback: validates and converts the integer in C++.
functions.passthrough_i64_9(buffer, value, ...zeros);

Validation

54 tests passed across the following files. Local lint and whitespace checks also passed.

./node --test \
  test/ffi/test-ffi-calls.js \
  test/ffi/test-ffi-fast-integer-validation.js \
  test/ffi/test-ffi-shared-buffer.js


β„Ή tests 54
β„Ή suites 0
β„Ή pass 54
β„Ή fail 0
β„Ή cancelled 0
β„Ή skipped 0
β„Ή todo 0
β„Ή duration_ms 934.82625

Refs: #66198
Assisted-by: Codex:Astra-medium

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run. labels Sep 22, 2026
nguyensyquan731-arch

This comment was marked as off-topic.

@codecov

codecov Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 90.37%. Comparing base (75e4bbe) to head (b0a3f6d).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66197   +/-   ##
=======================================
  Coverage   90.36%   90.37%           
=======================================
  Files         792      792           
  Lines      275398   275461   +63     
  Branches    52776    52802   +26     
=======================================
+ Hits       248877   248955   +78     
- Misses      16937    16938    +1     
+ Partials     9584     9568   -16     
Files with missing lines Coverage Ξ”
lib/internal/ffi-shared-buffer.js 60.11% <100.00%> (+4.36%) ⬆️
lib/internal/ffi/fast-api.js 94.67% <100.00%> (+0.17%) ⬆️
src/ffi/types.cc 63.19% <100.00%> (+6.92%) ⬆️

... and 31 files with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nguyensyquan731-arch

This comment was marked as spam.

@HoonDongKang
HoonDongKang force-pushed the ffi/accept-safe-integer-for-64-bit branch 2 times, most recently from ac792ef to b25043f Compare September 28, 2026 04:28
Allow safe integer numbers as int64 and uint64 arguments alongside
bigint values. Reject negative numbers for uint64 and numbers outside
the safe integer range. Keep 64-bit return values as bigint.

Apply validation and conversion across the Fast API, shared-buffer,
and generic argument conversion paths.

Add coverage for Number and BigInt boundaries, invalid inputs, and
single-argument calls before and after optimization.

Signed-off-by: HoonDongKang <d159123@naver.com>
Assisted-by: Codex:Astra-medium
Signed-off-by: HoonDongKang <d159123@naver.com>
Assisted-by: Codex:Astra-medium
@HoonDongKang
HoonDongKang force-pushed the ffi/accept-safe-integer-for-64-bit branch from b25043f to bf23996 Compare September 28, 2026 04:31
@ShogunPanda

Copy link
Copy Markdown
Contributor

Do you mind running benchmarks?
While I like the idea, I want to make sure it's not too expensive.

Validate and convert integer arguments in a single helper to avoid
repeated type metadata lookups and conversion checks.

Handle safe integer Number inputs for 64-bit arguments separately,
while preserving the existing type and range checks for BigInt inputs.
Return other argument types unchanged for subsequent pointer conversion.

Signed-off-by: HoonDongKang <d159123@naver.com>
Assisted-by: Codex:Astra-medium
@HoonDongKang
HoonDongKang force-pushed the ffi/accept-safe-integer-for-64-bit branch from 28b2721 to b0a3f6d Compare September 28, 2026 08:00
@HoonDongKang

HoonDongKang commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor Author

I updated fast-api.js in b0a3f6d to combine integer validation and conversion into validateAndConvertFastIntegerArg().

In the previous revision of this PR, validation and conversion were separate steps, each looking up the argument’s type metadata. The combined helper performs that lookup once, validates and converts safe integer "number" inputs to "bigint", and preserves the existing type and range checks for "bigint" inputs.

I measured two aspects of performance, with 30 runs per configuration:

1. Existing BigInt inputs: baseline vs. updated implementation

                            confidence   improvement   accuracy (*)    (**)   (***)
ffi/add-i64.js n=10000000  ***            +29.99 %   Β±4.82%  Β±6.41%  Β±8.36%
ffi/add-u64.js n=10000000  ***            +35.96 %   Β±5.13%  Β±6.83%  Β±8.89%

                              -41.1%             0%             +41.1%
ffi/add-i64.js n=10000000                      |β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–‘β–‘     +29.99% ***
ffi/add-u64.js n=10000000                      |β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–‘β–‘  +35.96% ***

2. BigInt vs. Number inputs in the updated implementation

I added an input: ['bigint', 'number'] configuration to both benchmarks and used scatter.js to measure the cost of passing numbers instead of pre-created bigints.

Benchmark BigInt throughput (ops/s) Number throughput (ops/s) Difference
add-i64.js 29,288,918.5 19,523,825.1 βˆ’33.34%
add-u64.js 28,502,908.3 19,819,141.9 βˆ’30.47%

For the first comparison, both versions perform the same type and range checks for BigInt inputs, and the Number conversion branch is not taken. The improvement may therefore reflect differences in V8’s generated code rather than less validation work.

The second comparison shows the cost of accepting Number inputs: throughput was approximately 30–33% lower than with pre-created BigInt inputs in these benchmarks. This measures the additional cost of internal validation and conversion when callers use the convenience provided by this PR.

In absolute terms, this corresponds to approximately 15–17 ns of additional time per call in these benchmarks, which pass two 64-bit integer arguments. Do you think this overhead is acceptable given the convenience of passing Number values directly?

==
Update: I compared the same binaries with and without TurboFan inlining to investigate the unexpected BigInt improvement.

With the --no-turbo-inlining flag, which disables TurboFan inlining, I got the following results:

                            confidence   improvement   accuracy (*)    (**)   (***)
ffi/add-i64.js n=10000000                  +0.49 %   Β±1.57%  Β±2.08%  Β±2.71%
ffi/add-u64.js n=10000000                  -0.10 %   Β±1.59%  Β±2.12%  Β±2.76%

                              -2.1%              0%              +2.1%
ffi/add-i64.js n=10000000            β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘|β–“β–“β–“β–“β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘  +0.49% 
ffi/add-u64.js n=10000000      β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–“|β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘       -0.10% 

With TurboFan inlining disabled, both performance differences were below 1%. This suggests that the previously observed BigInt improvement is related to TurboFan inlining and its effects on the generated code, rather than reduced validation work.

@HoonDongKang

HoonDongKang commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor Author

I ran additional benchmarks to follow up on test 2("2. BigInt vs. Number inputs in the updated implementation") in my previous comment, comparing three ways of preparing the arguments. Each comparison used the same PR binary, with 30 runs per configuration.

1. Pre-created BigInt values vs. Number values (the original test)
The arguments were prepared before timing:

const a = input === 'number' ? 20 : 20n;
const b = input === 'number' ? 22 : 22n;

bench.start();
for (let i = 0; i < n; ++i)
  add(a, b);
bench.end(n);

2. Explicit conversion of Number constants vs. direct Number arguments
Each case ran in a separate timed loop:

// input === 'bigint'
add(BigInt(20), BigInt(22));

// input === 'number'
add(20, 22);

3. Explicit conversion of buffer lengths vs. direct Number arguments
The buffers were allocated before timing:

const a = Buffer.alloc(64);
const b = Buffer.alloc(256);

Each case ran in a separate timed loop:

// input === 'bigint'
add(BigInt(a.length), BigInt(b.length));

// input === 'number'
add(a.length, b.length);

The results below show mean throughput measured using scatter.js. The percentage change is for Number inputs relative to BigInt inputs; positive values mean Number inputs had higher throughput.

Argument preparation Benchmark BigInt (ops/s) Number (ops/s) Change
Pre-created values add-i64 29,288,918.5 19,523,825.1 βˆ’33.34%
Pre-created values add-u64 28,502,908.3 19,819,141.9 βˆ’30.47%
BigInt(20), BigInt(22) add-i64 29,191,962.6 24,779,345.4 βˆ’15.12%
BigInt(20), BigInt(22) add-u64 28,580,807.5 25,475,704.5 βˆ’10.86%
BigInt(a.length), BigInt(b.length) add-i64 18,111,529.1 20,226,464.4 +11.68%
BigInt(a.length), BigInt(b.length) add-u64 17,549,515.7 19,566,290.2 +11.49%

The original 30–33% throughput reduction was relative to passing pre-created BigInt values. It does not represent the cost of replacing an explicit BigInt(length) conversion with a direct Number argument. In the buffer-length benchmarks, passing Numbers directly had approximately 11–12% higher throughput. This case more closely reflects the intended usage of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants