Skip to content

Conversation

alexs-sh
Copy link
Contributor

@alexs-sh alexs-sh commented May 11, 2025

About

This PR continues the work on issue #7870. It adds 4K alignment of output data to better matching with GNU shred behavior. Updating the alignment, but not the application's block size, allows to avoid the performance degradation observed in shred: use 4K block PR.

Before

echo a > foo && cargo run -q --features shred shred -v -n1 foo && du -b foo
shred: foo: pass 1/1 (random)...
65536   foo

After

echo a > foo && cargo run -q --features shred shred -v -n1 foo && du -b foo
shred: foo: pass 1/1 (random)...
4096    foo

Expected

echo a > foo && shred -v -n1 foo && du -b foo
shred: foo: pass 1/1 (random)...
4096    foo

I also tested a few more control values (1,4095,4096,4097,65535,65536,65537) to compare GNU shred with uutils shred. With these changes, both apps produce the same output.

GNU shred

Running /usr/bin/shred
In: 1   foo
shred: foo: pass 1/1 (random)...
Out: 4096       foo

In: 4095        foo
shred: foo: pass 1/1 (random)...
Out: 4096       foo

In: 4096        foo
shred: foo: pass 1/1 (random)...
Out: 4096       foo

In: 4097        foo
shred: foo: pass 1/1 (random)...
Out: 8192       foo

In: 65535       foo
shred: foo: pass 1/1 (random)...
Out: 65536      foo

In: 65536       foo
shred: foo: pass 1/1 (random)...
Out: 65536      foo

In: 65537       foo
shred: foo: pass 1/1 (random)...
Out: 69632      foo

Modified uutils shred

Running ./target/release/shred
In: 1   foo
./target/release/shred: foo: pass 1/1 (random)...
Out: 4096       foo

In: 4095        foo
./target/release/shred: foo: pass 1/1 (random)...
Out: 4096       foo

In: 4096        foo
./target/release/shred: foo: pass 1/1 (random)...
Out: 4096       foo

In: 4097        foo
./target/release/shred: foo: pass 1/1 (random)...
Out: 8192       foo

In: 65535       foo
./target/release/shred: foo: pass 1/1 (random)...
Out: 65536      foo

In: 65536       foo
./target/release/shred: foo: pass 1/1 (random)...
Out: 65536      foo

In: 65537       foo
./target/release/shred: foo: pass 1/1 (random)...
Out: 69632      foo

This commit allows aligning output data by 4K to better match GNU shred.

The 4K block size is configured as a constant because it provides a
widely used value in the simplest way. However, there is a chance that
some systems may use a different value. So far, I haven't encountered
anything other than 4K, I decided not to overcomplicate the approach for
now.
@alexs-sh alexs-sh mentioned this pull request May 11, 2025
Copy link

GNU testsuite comparison:

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

This commit improves the reliability of the writing logic and removes
implicit dependencies between the preferred I/O size and the block size.

For example, in earlier versions, using BLOCK_SIZE != N * IO_SIZE could
lead to overflows due to alignment with values larger than the buffer
size.
Copy link

GNU testsuite comparison:

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

Copy link
Collaborator

@BenWiederhake BenWiederhake left a comment

Choose a reason for hiding this comment

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

Lovely! Shorter and more self-explanatory than the previous version, commented, more correct, and even tests for many (edge)cases. Awesome! :D

@BenWiederhake BenWiederhake merged commit a73c0ea into uutils:main May 11, 2025
70 checks passed
@alexs-sh
Copy link
Contributor Author

Thank you for your feedback and quick review

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