uucore: Remove ARGV cache by AnuthaDev · Pull Request #13276 · uutils/coreutils

AnuthaDev · 2026-07-04T22:39:04Z

Problem

uucore keeps the CLI arguments cached in a static variable (ARGV), stating that repeatedly calling env::args_os is expensive. However env::args_os is only called once during the binary lifecycle. Storing the ARGV is useless overhead.

Fix

Removed the ARGV variable and added a different mechanism to initialize UTIL_NAME and EXECUTION_PHRASE.
Added init_util_name() , init_execution_phrase() that use the result of the first args_os call to set these variables. This is to prevent an additional call to args_os for determining these values.
Retained the get_or_init for these values instead of panicking on uninitialized values so any downstream users of uucore keep working, albeit with a performance hit.

Breaking changes

Two semantic changes for downstream consumers of the uucore crate:

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 via wild on Windows) on every call. Call it once and reuse the result.
If a downstream binary calls the new init_* functions, those values win over derivation from argv. Callers that never init see identical behavior to before.

Measurements

Create a test input file: printf "00 %.0s" {1..100000} > args.txt
Run the binary:

valgrind --tool=dhat ./target/release/coreutils echo $(cat args.txt) > /dev/null

main:

Total:     5,286,993 bytes in 200,206 blocks
At t-gmax: 5,000,674 bytes in 100,006 blocks
At t-end:  2,652,146 bytes in 100,083 blocks
Reads:     10,776,549 bytes
Writes:    7,975,882 bytes

This PR:

Total:     2,687,044 bytes in 100,207 blocks
At t-gmax: 2,663,057 bytes in 100,147 blocks
At t-end:  52,103 bytes in 82 blocks
Reads:     10,576,682 bytes
Writes:    5,375,912 bytes

GNU echo:

Total:     8,089 bytes in 31 blocks
At t-gmax: 8,084 bytes in 30 blocks
At t-end:  0 bytes in 0 blocks
Reads:     304,472 bytes
Writes:    303,826 bytes

The uutils version still allocates 332 times more (down from 653 times), still lots of scope for reducing allocations.

Impact

This change reduces memory allocation for all the coreutils binaries. The savings scale with argv: total allocations drop by one full argv copy, and the duplicate no longer persists for the process lifetime.

Practical benefit: Calling ls pattern* in a directory with a large number of items matching the pattern allocates much less.

Testing

All existing tests pass

sylvestre

the AI comment #0 isn't useful.
Please write it for a human as I am not an AI

sylvestre · 2026-07-04T23:14:32Z

Please update comment 0

github-actions · 2026-07-04T23:16:18Z

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/symlink (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/tail/pipe-f2 is no longer failing!

codspeed-hq · 2026-07-04T23:21:09Z

Merging this PR will regress 2 benchmarks

⚠️

Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 4 improved benchmarks
❌ 2 regressed benchmarks
✅ 351 untouched benchmarks
⏩ 46 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Simulation	`ls_recursive_wide_tree[(10000, 1000)]`	84.5 ms	87.4 ms	-3.37%
❌	Simulation	`sort_long_line[10000]`	1.1 ms	1.2 ms	-3.25%
⚡	Simulation	`expand_many_lines[100000]`	116.4 ms	103.5 ms	+12.47%
⚡	Simulation	`expand_custom_tabstops[50000]`	31.4 ms	28.8 ms	+9.19%
⚡	Simulation	`complex_relative_date`	330.6 µs	316.2 µs	+4.55%
⚡	Simulation	`cksum_crc32b`	42.3 ms	40.7 ms	+3.93%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing AnuthaDev:arghh-alloc (9bad806) with main (e1efa7d)}

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. ↩

AnuthaDev · 2026-07-04T23:33:27Z

Updated the Summary section in description (if that's what you meant by comment 0)

sylvestre · 2026-07-04T23:40:35Z

this one:
#13276 (comment)

sylvestre · 2026-07-05T08:51:21Z

i still would like comment #0 to be shorter
also, would it be possible to add a memory benchmark confirming this improvement? (note: should be added in a different pr)
thanks

AnuthaDev · 2026-07-05T09:34:50Z

Improved the comment. Will create a separate PR to add a memory benchmark. Please review.

sylvestre · 2026-07-07T08:08:16Z

Will create a separate PR to add a memory benchmark.

i am waiting for this to review this PR

oech3 · 2026-07-07T14:14:21Z

RAM usage is reduced and already catched by benches https://app.codspeed.io/uutils/coreutils/branches/AnuthaDev%3Aarghh-alloc?utm_source=github&utm_medium=comment-v2&utm_content=button&q=mode%3Amemory

AnuthaDev · 2026-07-08T20:16:12Z

@sylvestre Please let me know if the existing benchmark pointed out by @oech3 is enough or I need to add a separate benchmark

sylvestre · 2026-07-08T20:51:57Z

ok, thanks
there were below the threshold

AnuthaDev · 2026-07-09T12:28:06Z

@sylvestre I can see that you have enabled auto-merge, but its still pending on your approval since you had requested changes earlier.

AnuthaDev · 2026-07-09T13:51:39Z

@sylvestre You need to click on "Approve PR" for it to get merged. Its blocked due to your earlier review.

AnuthaDev · 2026-07-13T19:14:50Z

@sylvestre The merge is blocked on your approval. You need to submit another review with "Approved".

AnuthaDev · 2026-08-12T23:13:30Z

@sylvestre Please review

Copilot

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Removes the lifetime ARGV cache in uucore and replaces it with one-time initialization for UTIL_NAME / EXECUTION_PHRASE, while making args_os() return fresh argv iterators to reduce persistent allocations.

Changes:

Replaced LazyLock-backed ARGV caching with OnceLock for UTIL_NAME and EXECUTION_PHRASE.
Updated bin_inner! and binaries to initialize the utility name / execution phrase from the first argv observation.
Updated args_os() / args_os_filtered() semantics and docs to reflect non-cached argv iteration.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
src/uucore/src/lib/lib.rs	Removes cached `ARGV`; introduces `OnceLock` init APIs and updates argv iteration behavior/docs.
src/bin/uudoc.rs	Initializes `UTIL_NAME` / `EXECUTION_PHRASE` explicitly for the `manpage` flow.
src/bin/coreutils.rs	Initializes `UTIL_NAME` / `EXECUTION_PHRASE` depending on whether the utility is argv[0] or argv[1].

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    let name = std::path::Path::new(input).file_name().unwrap_or(input);
+    let result = UTIL_NAME.set(name.to_string_lossy().into_owned());
+    debug_assert!(
+        result.is_ok(),
+        "init_util_name called after UTIL_NAME was already initialized"
+    );


+    let result = EXECUTION_PHRASE.set(input.as_ref().to_string_lossy().into_owned());
+    debug_assert!(
+        result.is_ok(),
+        "init_execution_phrase called after EXECUTION_PHRASE was already initialized"
+    );


+///
+/// Each call copies all of argv (and, on Windows, re-expands glob patterns),
+/// so call it once and reuse the result rather than calling it repeatedly.
 pub fn args_os() -> impl Iterator<Item = OsString> {


+            if uucore::get_utility_is_second_arg() {
+                uucore::init_util_name(&util_os);
+                let mut phrase = binary.clone().into_os_string();
+                phrase.push(" ");
+                phrase.push(&util_os);
+                uucore::init_execution_phrase(&phrase);
+            } else {
+                uucore::init_util_name(&binary);
+                uucore::init_execution_phrase(&binary);
+            }


sylvestre requested changes Jul 4, 2026

View reviewed changes

sylvestre reviewed Jul 4, 2026

View reviewed changes

Comment thread src/bin/coreutils.rs Outdated

AnuthaDev force-pushed the arghh-alloc branch from ab3b345 to a952d5e Compare July 4, 2026 22:57

AnuthaDev requested a review from sylvestre July 4, 2026 22:58

oech3 reviewed Jul 7, 2026

View reviewed changes

Comment thread src/bin/uudoc.rs Outdated

oech3 reviewed Jul 7, 2026

View reviewed changes

Comment thread src/bin/coreutils.rs Outdated

AnuthaDev force-pushed the arghh-alloc branch from a952d5e to 1155881 Compare July 8, 2026 20:14

sylvestre enabled auto-merge (squash) July 8, 2026 20:52

sylvestre disabled auto-merge July 9, 2026 12:54

sylvestre enabled auto-merge (squash) July 9, 2026 12:54

sylvestre force-pushed the arghh-alloc branch from 3076411 to fff3a87 Compare July 27, 2026 11:57

uucore: Remove ARGV cache

88752a7

auto-merge was automatically disabled August 12, 2026 23:12
Head branch was pushed to by a user without write access

AnuthaDev force-pushed the arghh-alloc branch from fff3a87 to 88752a7 Compare August 12, 2026 23:12

Merge branch 'main' into arghh-alloc

9bad806

Copilot AI lite review requested due to automatic review settings August 13, 2026 09:53

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Copilot started reviewing on behalf of AnuthaDev August 13, 2026 10:11 View session

Uh oh!

Uh oh!

Conversation

AnuthaDev commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Breaking changes

Measurements

Impact

Testing

Uh oh!

sylvestre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sylvestre commented Jul 4, 2026

Uh oh!

github-actions Bot commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq Bot commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will regress 2 benchmarks

Performance Changes

Footnotes

Uh oh!

AnuthaDev commented Jul 4, 2026

Uh oh!

sylvestre commented Jul 4, 2026

Uh oh!

sylvestre commented Jul 5, 2026

Uh oh!

AnuthaDev commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sylvestre commented Jul 7, 2026

Uh oh!

oech3 commented Jul 7, 2026

Uh oh!

AnuthaDev commented Jul 8, 2026

Uh oh!

sylvestre commented Jul 8, 2026

Uh oh!

AnuthaDev commented Jul 9, 2026

Uh oh!

AnuthaDev commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AnuthaDev commented Jul 13, 2026

Uh oh!

AnuthaDev commented Aug 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AnuthaDev commented Jul 4, 2026 •

edited

Loading

github-actions Bot commented Jul 4, 2026 •

edited

Loading

codspeed-hq Bot commented Jul 4, 2026 •

edited

Loading

AnuthaDev commented Jul 5, 2026 •

edited

Loading

AnuthaDev commented Jul 9, 2026 •

edited

Loading