fix(Basenc):fix GNU coreutils test bounded-memory.sh #9536
Conversation
CodSpeed Performance ReportMerging #9536 will degrade performances by 2.88%Comparing Summary
Benchmarks breakdown
Footnotes
|
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
Kudos ! A few remarks however:
|
GNU basenc bounded-memory failed because the Rust impl buffered entire input and exceeded the vmem limit. Stream base32/base64 via BufReader and chunked encode/decode so the working set stays around 8 KiB. Keep base58 buffered to preserve its big-integer semantics. Flush already-decoded bytes before returning errors to match GNU output.
mattsu2020
force-pushed
the
basenc_bounded-memory
branch
from
645bd1e to
d5cc32b
Compare
|
GNU testsuite comparison: |
|
Ok, I like this ! I looked at the benchmark regression, and it seems that the problem come from the new We can merge this as is, because it's not trivial to avoid memsets in Rust and this MR was about fixing a GNU test anyway, but I think we should keep track of this with an issue @mattsu2020 tell me if you'd rather want this to be addressed in another PR. Anyway, thank you for your contribution ! |
I believe it would be more appropriate to address this matter in a separate pull request. Merging it would likely make handling it more difficult. |
|
@mattsu2020 I created the issue: #9621 |
3 participants
Summary
basenc: Stream base32/base64 I/O to honor bounded-memory test
The GNU bounded-memory implementation exceeded memory limits when handling all inputs. Using BufReader with chunked encoding/decoding, we consistently kept the working set under 8KiB. During errors, we flush partial outputs to match GNU's behavior.
Modified the test for bounded-memory.sh in GNU coreutils to pass.
related
#9127