See if estimateLengthForUTF8 costs more than it provides

[Jarred-Sumner]

What does this PR do?

How did you verify your code works?

[robobun]

^{Updated 8:08 PM PT - Oct 11th, 2025}

❌ @Jarred-Sumner, your commit 343b21f has 2 failures in Build #28931 (All Failures):

• test/cli/run/require-cache.test.ts - 1 failing on 🍎 13 aarch64
• test/js/bun/test/parallel/test-http-7480-should-not-emit-continue-event.ts - core dumped on 🐧 12 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 23512

That installs a local version of the PR into your bun-23512 executable, so you can run:

bun-23512 --bun

[coderabbitai]

Walkthrough

Updated src/js_printer.zig: quoteForJSON now preallocates a fixed buffer size of 2 + text.len instead of using estimateLengthForUTF8(text, ascii_only, '"'), with control flow and error handling otherwise unchanged. No public API changes.

Changes

Cohort / File(s)	Change summary
JSON quoting buffer preallocation src/js_printer.zig	In quoteForJSON, buffer growth changed from estimateLengthForUTF8(..., '"') to fixed 2 + text.len before appending the opening quote; logic and error handling unchanged; no exported declarations altered.

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description includes the required section headings but contains no content explaining the change or how it was verified, leaving both template questions unanswered.	Please fill in the “What does this PR do?” section with a summary of the code changes and complete the “How did you verify your code works?” section with details of your testing or validation steps.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title correctly identifies that the change revolves around evaluating whether estimateLengthForUTF8 is worth its cost, which matches the core modification in quoteForJSON; while it uses a question format rather than an imperative summary, it still directly relates to the primary change.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f0807e2 and 343b21f.

📒 Files selected for processing (1)

• src/js_printer.zig (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Files:

• src/js_printer.zig

src/**/js_*.zig

📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)

src/**/js_*.zig: Implement JavaScript bindings in a Zig file named with a js_ prefix (e.g., js_smtp.zig, js_your_feature.zig)
Handle reference counting correctly with ref()/deref() in JS-facing Zig code
Always implement proper cleanup in deinit() and finalize() for JS-exposed types

Files:

• src/js_printer.zig

src/{**/js_*.zig,bun.js/api/**/*.zig}

📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)

Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions

Files:

• src/js_printer.zig

src/**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, manage memory carefully and use defer for cleanup of allocations/resources

src/**/*.zig: Use the # prefix to declare private fields in Zig structs (e.g., struct { #foo: u32 })
Prefer decl literals when initializing values in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 })
Place @import directives at the bottom of Zig files
Use @import("bun") instead of @import("root").bun

When adding debug logs in Zig, create a scoped logger and log via Bun APIs: const log = bun.Output.scoped(.${SCOPE}, .hidden); then log("...", .{})

Files:

• src/js_printer.zig

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Format

🔇 Additional comments (1)

src/js_printer.zig (1)

344-344: Simplified pre-allocation trade-off may regress performance
Changing estimateLengthForUTF8(text, ascii_only, '"') to 2 + text.len (src/js_printer.zig:344) under-allocates for escaped/non-ASCII characters, causing extra reallocations.

• Benchmark typical vs heavy-escape and non-ASCII (ascii_only = true) workloads
• Add a comment explaining this trade-off above bytes.growIfNeeded

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}