-
Notifications
You must be signed in to change notification settings - Fork 3.8k
reduce unnecessary ffi boundary crossing #25428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
a0edecd to
d0243ea
Compare
WalkthroughReplaces an external C binding for base64 URL encoding with an internal simdutf-based implementation. The public function signature remains unchanged; only the internal implementation is modified to use the internal library instead of external bindings. Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (6)test/**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{cpp,zig}📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Files:
src/**/*.zig📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Files:
**/*.zig📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
Files:
🧠 Learnings (16)📓 Common learnings📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:35:08.612ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-11-24T18:35:39.205ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-11-24T18:35:08.612ZApplied to files:
📚 Learning: 2025-11-24T18:35:50.422ZApplied to files:
📚 Learning: 2025-10-18T05:23:24.403ZApplied to files:
📚 Learning: 2025-10-25T17:20:19.041ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-10-19T02:44:46.354ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test doesn't pass btw
bun test v1.3.5-canary.1 (d0243ea8)
2 |
3 | describe("base64 URL-safe encoding with simdutf", () => {
4 | test("encodes simple strings correctly", () => {
5 | const input = new TextEncoder().encode("Man");
6 | const output = new Uint8Array(100);
7 | const len = Bun.unsafe.base64.encodeURLSafe(output, input);
^
TypeError: undefined is not an object (evaluating 'Bun.unsafe.base64.encodeURLSafe')
at <anonymous> (/usr/local/etc/buildkite-agent/builds/macOS-13-x64-1/bun/bun/test/js/bun/util/base64-url-safe-simdutf.test.ts:7:28)
✗ base64 URL-safe encoding with simdutf > encodes simple strings correctly
12 | });
13 |
14 | test("encodes without padding", () => {
15 | const input = new TextEncoder().encode("Woman");
16 | const output = new Uint8Array(100);
17 | const len = Bun.unsafe.base64.encodeURLSafe(output, input);
^
TypeError: undefined is not an object (evaluating 'Bun.unsafe.base64.encodeURLSafe')
at <anonymous> (/usr/local/etc/buildkite-agent/builds/macOS-13-x64-1/bun/bun/test/js/bun/util/base64-url-safe-simdutf.test.ts:17:28)
✗ base64 URL-safe encoding with simdutf > encodes without padding
24 |
25 | test("uses URL-safe alphabet (- and _ instead of + and /)", () => {
26 | // Input that produces + and / in standard base64
27 | const input = new Uint8Array([0xff, 0xff, 0xbe, 0xff, 0xef, 0xbf, 0xfb, 0xef, 0xff]);
28 | const output = new Uint8Array(100);
29 | const len = Bun.unsafe.base64.encodeURLSafe(output, input);
^
TypeError: undefined is not an object (evaluating 'Bun.unsafe.base64.encodeURLSafe')
at <anonymous> (/usr/local/etc/buildkite-agent/builds/macOS-13-x64-1/bun/bun/test/js/bun/util/base64-url-safe-simdutf.test.ts:29:28)
✗ base64 URL-safe encoding with simdutf > uses URL-safe alphabet (- and _ instead of + and /) [0.20ms]
47 | "continued and indefatigable generation of knowledge, " +
48 | "exceeds the short vehemence of any carnal pleasure.";
49 |
50 | const input = new TextEncoder().encode(quote);
51 | const output = new Uint8Array(1000);
52 | const len = Bun.unsafe.base64.encodeURLSafe(output, input);
^
TypeError: undefined is not an object (evaluating 'Bun.unsafe.base64.encodeURLSafe')
at <anonymous> (/usr/local/etc/buildkite-agent/builds/macOS-13-x64-1/bun/bun/test/js/bun/util/base64-url-safe-simdutf.test.ts:52:28)
✗ base64 URL-safe encoding with simdutf > handles large strings [0.10ms]
62 | });
63 |
64 | test("handles empty input", () => {
65 | const input = new Uint8Array(0);
66 | const output = new Uint8Array(10);
67 | const len = Bun.unsafe.base64.encodeURLSafe(output, input);
^
TypeError: undefined is not an object (evaluating 'Bun.unsafe.base64.encodeURLSafe')
at <anonymous> (/usr/local/etc/buildkite-agent/builds/macOS-13-x64-1/bun/bun/test/js/bun/util/base64-url-safe-simdutf.test.ts:67:28)
✗ base64 URL-safe encoding with simdutf > handles empty input [0.08ms]
70 | });
71 |
72 | test("handles single byte", () => {
73 | const input = new Uint8Array([0x41]); // 'A'
74 | const output = new Uint8Array(10);
75 | const len = Bun.unsafe.base64.encodeURLSafe(output, input);
^
TypeError: undefined is not an object (evaluating 'Bun.unsafe.base64.encodeURLSafe')
at <anonymous> (/usr/local/etc/buildkite-agent/builds/macOS-13-x64-1/bun/bun/test/js/bun/util/base64-url-safe-simdutf.test.ts:75:28)
✗ base64 URL-safe encoding with simdutf > handles single byte [0.07ms]
88 | ];
89 |
90 | for (const testCase of testCases) {
91 | const input = typeof testCase === "string" ? new TextEncoder().encode(testCase) : testCase;
92 | const output = new Uint8Array(1000);
93 | const len = Bun.unsafe.base64.encodeURLSafe(output, input);
^
TypeError: undefined is not an object (evaluating 'Bun.unsafe.base64.encodeURLSafe')
at <anonymous> (/usr/local/etc/buildkite-agent/builds/macOS-13-x64-1/bun/bun/test/js/bun/util/base64-url-safe-simdutf.test.ts:93:30)
✗ base64 URL-safe encoding with simdutf > produces same output as Buffer.toString('base64url') [0.14ms]
0 pass
7 fail
Ran 7 tests across 1 file. [31.00ms]
What does this PR do?
Removes FFI boundary crossing - direct Zig → simdutf instead of Zig → C++ → simdutf
How did you verify your code works?
Existing tests should be sufficient