Skip to content

factor:performance tuning#9261

Open
mattsu2020 wants to merge 3 commits intouutils:mainfrom
mattsu2020:factor_fix
Open

factor:performance tuning#9261
mattsu2020 wants to merge 3 commits intouutils:mainfrom
mattsu2020:factor_fix

Conversation

@mattsu2020
Copy link
Contributor

@mattsu2020 mattsu2020 commented Nov 13, 2025

Performance improvement for large numbers

fix this issue
https://bugs.launchpad.net/ubuntu/+source/rust-coreutils/+bug/2131212

related
#10262

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

1 similar comment
@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

could you please run hyperfine with the three programs? gnu, without the patch and with the patch
and share the full results here? thanks :)

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@mattsu2020
Copy link
Contributor Author

could you please run hyperfine with the three programs? gnu, without the patch and with the patch and share the full results here? thanks :)

Implementation Details

GMP 6.3.0 and GNU coreutils 9.5 were built and installed from source

Created factor_numbers_u128_repeat.txt (60 lines) as benchmark input, containing 6 composite numbers ranging from 64 to 128 bits repeated 10 times. Confirmed factorization completion across all 3 implementations and reran Hyperfine.
All commands used the release profile (target/profiling/factor).
Hyperfine execution results
Command: hyperfine --warmup 3 --runs 12 “ < factor_numbers_u128_repeat.txt”

Implementation   Average time (s) Standard deviation (s) Minimum–Maximum (s)
GNU coreutils 9.5 ( local-gnu/bin/factor) 6.718 0.106 6.594 – 7.020
Old implementation (prev_worktree/target/profiling/factor) 6.125 1.942 2.648 – 8.508
After patch application (target/profiling/factor) 6.993 1.585 4.299 – 9.457

To reduce variance, we adjusted to 3 warm-ups + 12 measurements, but the Rust version still shows relatively high dispersion due to its randomized algorithm. For greater stability, consider running at times of low system load or using CPU pinning.
Behavior with inputs exceeding 128 bits

For factor_numbers.txt (max ~260 bits), both the GNU version and the patched version achieved complete factorization. The old implementation returned factor: Factorization incomplete. Remainders exist. and exited with exit code 1. This confirms the improvement in support for large integers.
factor_numbers_u128_repeat.txt

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 16, 2025

Merging this PR will not alter performance

✅ 288 untouched benchmarks
⏩ 38 skipped benchmarks1


Comparing mattsu2020:factor_fix (0b50bcc) with main (289d701)

Open in CodSpeed

Footnotes

  1. 38 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:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@sylvestre
Copy link
Contributor

Any idea why codspeed does not detect it?

@mattsu2020
Copy link
Contributor Author

Any idea why codspeed does not detect it?

If I were to consider it, I would create test cases with large integers.

@github-actions
Copy link

github-actions bot commented Dec 7, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

1 similar comment
@github-actions
Copy link

github-actions bot commented Dec 8, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/tail/follow-name is no longer failing!

@sylvestre
Copy link
Contributor

many jobs are failing

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/sort/sort-stale-thread-mem. tests/sort/sort-stale-thread-mem is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@mattsu2020 mattsu2020 requested a review from sylvestre January 5, 2026 08:52
@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails 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/tail/retry is no longer failing!

@sylvestre
Copy link
Contributor

please add a test to verify that factor 15111234931751377131713914373267893176342349831 is indeed fixed

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/factor/t34 is no longer failing!
Note: The gnu test tests/basenc/bounded-memory is now being skipped but was previously passing.
Note: The gnu test tests/cut/bounded-memory is now being skipped but was previously passing.
Note: The gnu test tests/dd/no-allocate is now being skipped but was previously passing.
Note: The gnu test tests/tail/pipe-f is now being skipped but was previously passing.
Congrats! The gnu test tests/printf/printf-surprise is now passing!

- Add num-integer dependency to support enhanced numeric operations.
- Refactor factorization logic to avoid redundant parsing and optimize u64/u128 paths.
- Improve handling of non-positive and invalid inputs to align with GNU factor behavior.
- Enhance large BigUint factoring with additional algorithms and clearer limitations.
@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cut/bounded-memory is no longer failing!
Note: The gnu test tests/cut/cut-huge-range is now being skipped but was previously passing.
Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing.

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.

2 participants

Comments