shuf: fix memory and CPU usage with -i LOW-HIGH and small -n COUNT - #126
shuf: fix memory and CPU usage with -i LOW-HIGH and small -n COUNT#126xelan wants to merge 1 commit into
Conversation
shuf -i L-H creates an in-memory array with a "virtual line" for every number in the range, even if -n COUNT asks for only a few of them: "shuf -i 1-2222222222 -n 1" dies trying to allocate ~17 gigabytes, and "shuf -i 1-99999999 -n 1" needs ~800 megabytes and takes seconds where GNU shuf needs a millisecond. If COUNT is small enough (outlines^2 / 2 < numlines), pick COUNT distinct random numbers from the range instead of creating and shuffling the array: the expected cost of the duplicate checking is then lower than the cost of creating the array. Otherwise keep the old array method, so full-range permutations behave as before. Fixes mirror#109 function old new delta shuf_main 557 698 +141 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 1/0 up/down: 141/0) Total: 141 bytes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
While running time issue is solved in 2023, these tests fails by allocating too much memory, then |
Tests provided by Andreas Erhard: - https://github.com/xelan In an attempt to fix this issue: - mirror#109 Provided by this pull: - mirror#126 Some tests has been put under `SKIP=` rule by RAF: Apparently, the implementation of the shuf command requires a lot of memory, as it creates a "virtual line" for each number in the range. For larger numbers (but well below INT_MAX) the command crashes with an "out of memory" error: # about half of 32-bit INT_MAX $ shuf -i 1-2222222222 -n 1 shuf: out of memory Original commit pull was not reporting the Andreas's e-mail but only the co-authored field related to Claude by Antrophic Therefore, this addition which doesn't impact on the codebase but only on the testsuite is signed by who integrated the file. Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
rand() uniformity issue in shuf.c as noted by the TODOThis can be moved in libbb.a and serves others applets like zcip.c, awk.c, ntpd.c, telnetd.c and tls.c Please, consider that rand() isn't strong in terms of PractRand testing and potentially should be replaced with umkaos.c internal function. Moreover, the integration of umkaos internals (from uchaosys/kdev) in busybox support the idea to replace ./miscutils/seedrng.c also commit 451122a (HEAD -> uchaosys) shuf: random non-uniformity fix (todo->done), v4 |
|
Requires:
Please, provide a patch with your e-mail address as authorship signing-off:
The integrated patch is on the bugfixes branch: |
Tests provided by Andreas Erhard: - https://github.com/xelan In an attempt to fix this issue: - mirror#109 Provided by this pull: - mirror#126 Some tests has been put under `SKIP=` rule by RAF: Apparently, the implementation of the shuf command requires a lot of memory, as it creates a "virtual line" for each number in the range. For larger numbers (but well below INT_MAX) the command crashes with an "out of memory" error: # about half of 32-bit INT_MAX $ shuf -i 1-2222222222 -n 1 shuf: out of memory Original commit pull was not reporting the Andreas's e-mail but only the co-authored field related to Claude by Antrophic Therefore, this addition which doesn't impact on the codebase but only on the testsuite is signed by who integrated the file. Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
shuf -i L-H used to create an in-memory array with one slot for every
number in the range, then shuffle it. For large ranges with a small
-n COUNT this is wasteful and can OOM:
shuf -i 1-99999999 -n 1 # ~800 MB, seconds
shuf -i 1-2222222222 -n 1 # ~17 GB, dies
Instead, when outlines^2 / 2 < numlines, pick COUNT distinct random
numbers directly from the range. The expected number of duplicate
checks is less than outlines^2 / 2, which is cheaper than allocating
and shuffling the full array. For full-range permutations the old
array method is kept so behaviour is unchanged.
This makes "shuf -i 1-2222222222 -n 1" run in milliseconds with
negligible memory use, while large -n values still use the fast
Fisher-Yates path.
text data bss dec hex filename
758 0 0 758 2f6 coreutils/shuf.o
860 0 0 860 35c coreutils/shuf.o
References:
- mirror#126
- mirror#109
Requires:
- shuf: random non-uniformity fix (todo->done), v4
This patch can be applied once having reasonably fixed the isse
of non-uniformity rand() issue otherwise the goto again can create
an (almost) infinite loop. Since the Montercarlo precision is lower
than 0.1% by now, it meas that the new random() function is uniform
enough to be trusted in exit from the again-loop.
Original commit pull was not reporting the Andreas's e-mail but
only the co-authored field related to Claude by Antrophic
Therefore, this addition which doesn't impact on the codebase
but only on the testsuite is signed by who integrated the patch.
Original author of the patch: Andreas Erhard <github.com/xelan>
Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
shuf -i L-H used to create an in-memory array with one slot for every
number in the range, then shuffle it. For large ranges with a small
-n COUNT this is wasteful and can OOM:
shuf -i 1-99999999 -n 1 # ~800 MB, seconds
shuf -i 1-2222222222 -n 1 # ~17 GB, dies
Instead, when outlines^2 / 2 < numlines, pick COUNT distinct random
numbers directly from the range. The expected number of duplicate
checks is less than outlines^2 / 2, which is cheaper than allocating
and shuffling the full array. For full-range permutations the old
array method is kept so behaviour is unchanged.
This makes "shuf -i 1-2222222222 -n 1" run in milliseconds with
negligible memory use, while large -n values still use the fast
Fisher-Yates path.
text data bss dec hex filename
758 0 0 758 2f6 coreutils/shuf.o
860 0 0 860 35c coreutils/shuf.o
References:
- mirror#126
- mirror#109
Requires:
- shuf: random non-uniformity fix (todo->done), v5
This patch can be applied once having reasonably fixed the isse
of non-uniformity rand() issue otherwise the goto again can create
an (almost) infinite loop. Since the Montercarlo precision error is
assesed by now, it meas that the new random() function is uniform
enough to be trusted in exit from the again-loop.
Original commit pull was not reporting the Andreas's e-mail but
only the co-authored field related to Claude by Antrophic
Therefore, this addition which doesn't impact on the codebase
but only on the testsuite is signed by who integrated the patch.
Original author of the patch: Andreas Erhard <github.com/xelan>
Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
shuf -i L-H used to create an in-memory array with one slot for every
number in the range, then shuffle it. For large ranges with a small
-n COUNT this is wasteful and can OOM:
shuf -i 1-99999999 -n 1 # ~800 MB, seconds
shuf -i 1-2222222222 -n 1 # ~17 GB, dies
Instead, when outlines^2 / 2 < numlines, pick COUNT distinct random
numbers directly from the range. The expected number of duplicate
checks is less than outlines^2 / 2, which is cheaper than allocating
and shuffling the full array. For full-range permutations the old
array method is kept so behaviour is unchanged.
This makes "shuf -i 1-2222222222 -n 1" run in milliseconds with
negligible memory use, while large -n values still use the fast
Fisher-Yates path.
text data bss dec hex filename
758 0 0 758 2f6 coreutils/shuf.o
860 0 0 860 35c coreutils/shuf.o
References:
- mirror#126
- mirror#109
Requires:
- shuf: random non-uniformity fix (todo->done), v5
This patch can be applied once having reasonably fixed the isse
of non-uniformity rand() issue otherwise the goto again can create
an (almost) infinite loop. Since the Montercarlo precision error is
assesed by now, it meas that the new random() function is uniform
enough to be trusted in exit from the again-loop.
Original commit pull was not reporting the Andreas's e-mail but
only the co-authored field related to Claude by Antrophic
Therefore, this addition which doesn't impact on the codebase
but only on the testsuite is signed by who integrated the patch.
Original author of the patch: Andreas Erhard <github.com/xelan>
Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
|
updated |
shuf -i L-Hcreates an in-memory array with a "virtual line" for every number in the range, even if -n COUNT asks for only a few of them: "shuf -i 1-2222222222 -n 1" dies trying to allocate ~17 gigabytes, and "shuf -i 1-99999999 -n 1" needs ~800 megabytes and takes seconds where GNU shuf needs a millisecond.If COUNT is small enough (outlines^2 / 2 < numlines), pick COUNT distinct random numbers from the range instead of creating and shuffling the array: the expected cost of the duplicate checking is then lower than the cost of creating the array. Otherwise keep the old array method, so full-range permutations behave as before.
This PR was made with the help of Claude Fable, as C is not my main programming language. Added tests and refined the code to keep it compatible while fixing the issue, but please double-check.
Fixes #109
Changeset
coreutils/shuf.ctestsuite/shuf.tests(new)Code size
shuf_main