Skip to content

factor: deduplicate code, refactor & optimize parsing#11292

Merged
cakebaker merged 3 commits intouutils:mainfrom
Alonely0:factorio-2
Mar 12, 2026
Merged

factor: deduplicate code, refactor & optimize parsing#11292
cakebaker merged 3 commits intouutils:mainfrom
Alonely0:factorio-2

Conversation

@Alonely0
Copy link
Contributor

Replaces the various write_result_* for a single, generic one. On the parsing side, UTF-8 checks are now performed strictly once, the worst-case of BigUints needs not be parsed & allocated every time, and error handling has been tightened.

@Alonely0 Alonely0 changed the title factor: deduplidate code, refactor & optimize parsing factor: deduplicate code, refactor & optimize parsing Mar 11, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 11, 2026

Merging this PR will improve performance by 6.16%

⚡ 1 improved benchmark
✅ 297 untouched benchmarks
⏩ 48 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation factor_multiple_u64s[2] 35.2 ms 33.2 ms +6.16%

Comparing Alonely0:factorio-2 (fdf46e4) with main (d916606)

Open in CodSpeed

Footnotes

  1. 48 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.

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/symlink (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.
Congrats! The gnu test tests/seq/seq-epipe is now passing!
Note: The gnu test tests/misc/write-errors was skipped on 'main' but is now failing.

@Alonely0
Copy link
Contributor Author

The macOS x64 CI fail is unrelated, as per usual sigh.

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tty/tty-eof (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.
Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

@github-actions
Copy link

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/tty/tty-eof (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/follow-name (passes in this run but fails in the 'main' branch)
Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

@Alonely0
Copy link
Contributor Author

@cakebaker Is there any other change you would like to see? Lmk if there's anything else I can do :). One macOS CI is still experiencing internal errors, sadly...

Copy link
Contributor

@cakebaker cakebaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work :)

@Alonely0
Copy link
Contributor Author

Lemme fix the lockfile in a second lol

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tty/tty-eof (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/printf/printf-surprise is now being skipped but was previously passing.
Congrats! The gnu test tests/pr/bounded-memory is now passing!

@Alonely0 Alonely0 force-pushed the factorio-2 branch 2 times, most recently from 4e4318e to c39b06d Compare March 11, 2026 17:38
@Alonely0
Copy link
Contributor Author

@cakebaker In order to pass cargo-deny we probably need to pin ctrlc's nix to the previous version, as they have updated but we've chosen not to yet (due to a performance issue iirc). I can do it ad-hoc or submit another PR, as you prefer.

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/misc/io-errors. tests/misc/io-errors is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tty/tty-eof (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/csplit/csplit-heap is now being skipped but was previously passing.
Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

@xtqqczze
Copy link
Contributor

Can we avoid duplicating ctrlc and nix dependencies until we can merge #10869 and #11063?

@cakebaker
Copy link
Contributor

@Alonely0 I would only add the one-line change in Cargo.lock related to the removal of num-traits. Updating the other dependencies with cargo update is outside the scope of this PR.

@Alonely0
Copy link
Contributor Author

@Alonely0 I would only add the one-line change in Cargo.lock related to the removal of num-traits. Updating the other dependencies with cargo update is outside the scope of this PR.

Sure thing, I'll get that reverted in a second.

@oech3
Copy link
Contributor

oech3 commented Mar 12, 2026

Cargo.lock is outdated

@Alonely0
Copy link
Contributor Author

Cargo.lock is outdated

Yeah, tho upon updating it we get other CI failures because we end up with duplicated nix pkgs, as it was updated as a transitive dep of ctrlc. For us, this is essentially blocked on #11063; I think @xtqqczze would prefer to handle this.

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (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)
Congrats! The gnu test tests/cp/link-heap is now passing!

Now there is a single polymorphic function that just takes displayable data, not concrete datatypes. It's two opaque Display types since one of the cases has mismatched number types.
Now UTF-8 checks are performed strictly once, the worst-case of BigUints needs not be parsed & allocated every time, and error handling has been tightened.
@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/uniq/uniq-perf. tests/uniq/uniq-perf is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/cp/link-heap is now passing!

@cakebaker
Copy link
Contributor

Ok, I added the missing Cargo.lock and did a rebase.

@cakebaker cakebaker merged commit 686bf6a into uutils:main Mar 12, 2026
167 of 181 checks passed
@cakebaker
Copy link
Contributor

Thanks!

@Alonely0
Copy link
Contributor Author

Thanks!

Thank you too :)

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.

4 participants