Repository navigation
base64: avoid repeated input loads when encoding - #23017
Merged
Merged
Conversation
Pack each input triplet into a uint32_t before writing the four encoded characters. This avoids redundant input loads when the compiler cannot rule out aliasing between input and output. Keep byte-wise reads, both alphabets, padding and tail handling unchanged. Document the load-before-store ordering to preserve the optimization. Extend unit1302 with binary round-trip tests for lengths 1 through 256, including output-length and NUL-termination checks. Closes #23017
cmeister2
force-pushed
the
cm2/b64enc
branch
from
September 21, 2026 09:38
acec0de to
acec0de
Compare
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The optimization preserves encoding behavior and is covered across byte values and remainder lengths.
Review effort: Balanced
Findings: None
What changed in this PR
Optimizes shared Base64 encoding by packing each three-byte group before producing output.
Changes:
- Reduces repeated input loads in standard and URL-safe encoding.
- Adds binary round-trip coverage for lengths 1–256.
| File | Description |
|---|---|
lib/curlx/base64.c |
Uses a packed 24-bit value for encoding. |
tests/unit/unit1302.c |
Adds binary prefix round-trip tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch loads each three-byte input group into a
uint32_tbefore writing the four encoded characters. Both standard and URL-safe base64 use this loop.The ordering matters: it lets the compiler reuse the input in registers without proving that output stores cannot alias the input. The added comment documents that reason for the packed value.
Assembly changes
The September 21 benchmark report includes before/after x86-64 disassembly from GCC 11.5.0 with
-O2 -DNDEBUG. In that build, the original loop loads each input byte twice. These excerpts preserve instruction order but omit intervening instructions, as marked.Before, input loads are interleaved with output stores:
After, each input byte is loaded once, before the first output store:
The negative offsets refer to the current triplet because the loop advances the pointers early. The remaining indices come from register shifts and masks instead of further input loads. The first index does not need the third byte, so GCC calculates it before the final
or. The old loop's twocdqesign-extension instructions also disappear.For the benchmark's 256-byte input, the loop processes 85 triplets; the unchanged tail handles the last byte. That removes 255 input-load instructions and 510 x86 instructions per call. These are instruction counts, not cycle counts or reductions in bytes fetched from RAM.
Why the reloads?
The generated code is consistent with conservative alias analysis. Character-pointer accesses can alias, and allocation in the measured build goes through
Curl_cmalloc, an indirect call that does not provide the compiler with the usual direct-mallocnon-aliasing information.Without proving that the output stores cannot affect the input, the compiler must preserve the original read-after-write behavior. Loading the triplet first makes reuse explicit in the C source. This explanation fits the disassembly; it was not verified with a compiler alias-analysis dump.
Measured result
The report measured 153.359 ns before and 120.116 ns after: 21.68% less time, with the after binary faster in all five paired rounds. Measurements used a static GCC build on an Intel Core Ultra 7 165H under WSL2, pinned to CPU 2, with compilation paused. Allocation and free remained part of the workload.
Those binaries included the report's other optimizations, so this is not an isolated measurement of this PR. Timings also shifted in unchanged routines. The disassembly demonstrates less work in this loop, but neither establishes a portable 22% gain nor attributes the timing improvement to individual instruction changes.
Safety and tests
The implementation still performs three individual byte reads, avoiding alignment requirements, endian dependence, and reading beyond the triplet. Input bounds, allocation size, alphabets, padding, and one-/two-byte tail handling are unchanged. The loop remains scalar.
The added
unit1302coverage checks binary prefixes of lengths 1–256, round-trip correctness, output lengths, and NUL termination. Existing vectors cover both alphabets and all remainder lengths. Local test 1302, source-style checks, and whitespace checks passed; the runner emitted missing-helper warnings forcurlinfoand the server executable. The full suite and cross-platform checks were not run for this patch.