uucore: Remove ARGV cache#13276
Conversation
sylvestre
left a comment
There was a problem hiding this comment.
the AI comment #0 isn't useful.
Please write it for a human as I am not an AI
|
Please update comment 0 |
|
GNU testsuite comparison: |
Merging this PR will improve performance by 5.16%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | hostname_ip_lookup[100000] |
109.1 µs | 100.5 µs | +8.48% |
| ⚡ | Simulation | complex_relative_date |
146.1 µs | 135 µs | +8.22% |
| ⚡ | Simulation | sort_ascii_utf8_locale |
16.1 ms | 15.5 ms | +4.42% |
| ⚡ | Simulation | unexpand_large_file[10] |
291.9 ms | 282.3 ms | +3.41% |
| ⚡ | Simulation | unexpand_many_lines[100000] |
139.3 ms | 134.7 ms | +3.41% |
| ⚡ | Simulation | single_date_now |
85.4 µs | 82.8 µs | +3.18% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing AnuthaDev:arghh-alloc (a952d5e) with main (51529dc)
Footnotes
-
46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
Updated the Summary section in description (if that's what you meant by comment 0) |
|
this one: |
Summary
uucorekept the CLI arguments cached in a static variable, stating that repeatedly callingenv::args_osis expensive. Howeverenv::args_osis only called once during the binary lifecycle and storing the ARGV used extra memory without any benefit. This PR removes that cache to reduce memory allocation.Rationale
The cache bought nothing and cost memory:
args_os()is called once per process:bin_inner!passes it intouumain, and the multicallcoreutilsmain reads it once to dispatch. Caching a once-used value for the process lifetime is pure overhead.Vec<OsString>was a permanent duplicate — one heap allocation per argument plus the vector itself, up to ARG_MAX-sized invocations (e.g.rmfed byxargs)..cloned()) when handing the iterator to clap. The new flow makes a single transient copy that is consumed by argument parsing and freed.Changes
src/uucore/src/lib/lib.rsstatic ARGVand theLazyLockstatics forUTIL_NAME/EXECUTION_PHRASE; replaced withOnceLocks.init_util_name()/init_execution_phrase(). Both are first-write-wins (later calls are ignored), so harnesses that invokeuumainrepeatedly in one process — the fuzz targets — are safe.util_name()/execution_phrase()useget_or_initwith a fallback that reproduces the old lazy derivation for un-wired callers.bin_inner!(every standalone binary'smain) peeks argv[0] from its singleargs_os()read and initializes both values before callinguumain. Deriving from argv[0] preserves the existing symlink/rename behavior (a binary copied tonapstill reportsnap:in errors).args_os()/args_os_filtered()are now thin wrappers overstd::env::args_os()(wild::args_os()on Windows); doc comments updated to say each call copies argv.src/bin/coreutils.rs(multicall): initializes both values in the dispatch arm — from the raw binary path when matched via argv[0] (symlink case), or from<binary> <util>when the utility is the second argument, preserving usage strings likeTry './coreutils ls --help'byte-for-byte.src/bin/uudoc.rs:manpagegeneration initializes the execution phrase inmain(where raw argv is available) and the utility name ingen_manpageafter clap validates the utility, replacing the oldmanpage-skip hack for the wired path.set_utility_is_second_arg()and its getter are retained: the multicall binary anduudocstill set the flag, and the lazy fallback derivation depends on it for un-wired callers.Breaking changes
No API removals — the new
init_*functions are additive, andutil_name()/execution_phrase()/args_os()keep their signatures. Two semantic changes for downstream consumers of theuucorecrate:uucore::args_os()is no longer cheap to call repeatedly. It previously returned clones from a process-lifetime cache; it now copies all of argv (and re-expands globs viawildon Windows) on every call. Call it once and reuse the result. In-tree callers were already single-call.util_name()/execution_phrase()can now be pinned by the embedding binary. If a downstream binary calls the newinit_*functions, those values win over derivation from argv. Callers that never init see identical behavior to before (same derivation, now computed on first use without the persistent argv copy).Measurements
printf "00 %.0s" {1..100000} > args.txtmain:This PR:
The savings scale with argv: total allocations drop by one full argv copy (bytes and one block per argument), and the duplicate no longer persists for the process lifetime — which previously lasted arbitrarily long for utilities like
sleep,tail, orddand could reach ARG_MAX (~2 MB) for glob- or xargs-fed invocations ofls,rm,cp, etc.Testing