Skip to content

cgen: avoid copies in builder interpolation parts - #32179

Open
trufae wants to merge 1 commit into
vlang:masterfrom
trufae:builder-interpolation-buffers
Open

trufae wants to merge 1 commit into
vlang:masterfrom
trufae:builder-interpolation-buffers

Conversation

@trufae

@trufae trufae commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Move the interpolation buffer sizing out of str_intp.v into a shared interpolation_buffers.v, so cleanc.v, str_intp.v and the new builder path agree on when a numeric conversion may return a view of a stack buffer.

Add a strings.Builder.write_string fast path that fills the parts array with those views instead of converting each part to an owning string, keeping every buffer alive until all operands are copied.

What this does

strings.Builder.write_string used to turn every interpolation part into an
owning string, even when the part was a numeric formatting call whose result
already lives in a caller-provided stack buffer. Each of those conversions
costs an allocation and a copy, and the copy is discarded as soon as the
builder snapshots its argument.

This reworks that path. Buffer sizing moves out of str_intp.v into a shared
gen/c/interpolation_buffers.v, so cleanc.v, str_intp.v and the new builder
path agree on when a numeric conversion may return a view into a buffer and on
how much stack is safe to reserve. On top of that, a new
gen/c/builder_interpolation.v fills the parts array with those views instead
of owning strings when the argument is a join, a nested +, or a single
buffered conversion. Parts are all evaluated before any write happens, and
every buffer stays alive until string_plus_many has copied the operands.

Everything outside that set keeps the old path: user str() methods,
dynamically sized formats, -autofree, and nested joins whose bytes may be
snapshotted before later side effects.

Benchmarks

One million writes, ten paired runs, median wall time:

case before after
${name}:${n} 60.8 ms 31.5 ms
mixed integer/float formatting 1,377 ms 1,144 ms

Testing

New vlib/v/compiler_tests/builder_interpolation_test.v checks the generated C
and then runs the compiled program under default, -gc none, -prod and
-autofree. Existing interpolation and exponent tests pass. The compiler
benchmark and full suite are still running; I'll report back here.

Copy link
Copy Markdown
Member

Reviewed 1689c7c10e76e32df631179b00e2534e231e82df. No blocking issues found.

The shared sizing logic keeps numeric buffers within their bounds and stack budget, and their lifetimes cover the full operand evaluation and copy. Evaluating all parts before writing, together with the builder buffer/header overlap snapshot, preserves the relevant aliasing and side-effect behavior.

Local validation passed: compiler self-build and ./v -new-compiler -gc none -nocache vlib/v/compiler_tests/builder_interpolation_test.v. This checked generated C and the fixture under default, -gc none, -prod, and -autofree modes, including numeric formatting and aliasing cases.

Looks good to merge from this review. CI was not considered.

Copy link
Copy Markdown
Member

The code-review verdict still stands, but this branch now conflicts with master at the early optimization hooks in vlib/v/gen/c/fn.v.

I resolved the conflict locally against master d85740b3639ac1f748ec60bf4d28a8e2aca2abff by retaining both complete guarded blocks in gen_call: this PR's builder-interpolation hook, followed by the already-merged temporary-bytes-chain hook from #32183:

if !callee_is_fn_value && g.gen_builder_interpolation_call(node, target_name, resolved_target_name) {
    return
}
if node.children_count == 2 && target_name.len in [12, 13] && !callee_is_fn_value
    && target_name in ['string.bytes', 'string__bytes']
    && g.gen_temporary_bytes_chain(node, resolved_target_name) {
    return
}

The resulting change relative to master is still the original seven-file PR scope (+429/-44); the added/deleted source lines match the approved change. The resolved tree passed:

  • ./v -gc none self
  • ./v -new-compiler -gc none -nocache vlib/v/compiler_tests/builder_interpolation_test.v
  • ./v -new-compiler -gc none -nocache vlib/v/compiler_tests/conversion_chains_test.v

GitHub rejected publishing the resolution to the fork with 403 Resource not accessible by integration, so I could not update this PR branch or complete its squash merge. Please update the branch preserving both hooks; the source review and focused local checks are clear. CI was not considered.

Move the interpolation buffer sizing out of str_intp.v into a shared
interpolation_buffers.v, so cleanc.v, str_intp.v and the new builder path
agree on when a numeric conversion may return a view of a stack buffer.

Add a strings.Builder.write_string fast path that fills the parts array
with those views instead of converting each part to an owning string,
keeping every buffer alive until all operands are copied.
@trufae
trufae force-pushed the builder-interpolation-buffers branch from 1689c7c to 7a1b22f Compare November 13, 2026 12:42

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants