Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor writer process to support direct disk writing #126

Merged
merged 4 commits into from
May 20, 2024

Conversation

ifd3f
Copy link
Owner

@ifd3f ifd3f commented May 20, 2024

This closes #10, #115, and #124:

We also significantly improve the unit testing of the writer process.

Test plan

Test plan, multiple screenshots

Aligned file, Linux

> cargo run -- burn test.img -z none -s none -f --root always
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.10s
     Running `target/debug/caligula burn test.img -z none -s none -f --root always`
Input file: test.img
Detected compression format: no compression
> Select target disk sdc | STORAGE DEVICE - 62.6 GB (disk, remov
able: yes)

image

Unaligned file, Linux

> cargo run -- burn LICENSE -z none -s none -f --root always
   Compiling caligula v0.4.4 (/home/astrid/Documents/caligula)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.65s
     Running `target/debug/caligula burn LICENSE -z none -s none -f --root always`
Input file: LICENSE
Detected compression format: no compression
> Select target disk sdc | STORAGE DEVICE - 62.6 GB (disk, removable: yes)
[sudo] password for astrid: 

image

Aligned compressed file, Linux

> cargo run -- burn duo256_sd.img.lz4 -s none -f --root always
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.09s
     Running `target/debug/caligula burn duo256_sd.img.lz4 -s none -f --root always`
Input file: duo256_sd.img.lz4
Detected compression format: lz4
> Select target disk sdc | STORAGE DEVICE - 62.6 GB (disk, remov
able: yes)

image

Aligned file, MacOS

> cargo run -- burn test.img -z none -s none -f --root always
warning: caligula@0.4.4: native/darwin/REDiskList.m:30:13: warning: 'NSArray' may not respond to 'sortUsingSelector:'
warning: caligula@0.4.4:     [_disks sortUsingSelector:@selector(localizedCaseInsensitiveCompare:)];
warning: caligula@0.4.4:      ~~~~~~ ^
warning: caligula@0.4.4: 1 warning generated.
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.16s
     Running `target/debug/caligula burn test.img -z none -s none -f --root always`
Input file: test.img
Detected compression format: no compression
> Select target disk disk2 | Generic STORAGE DEVICE Media - 62.6 GB (disk, remov
able: yes)
image

Unaligned file, MacOS

cargo run -- burn LICENSE -z none -s none -f --root always
warning: caligula@0.4.4: native/darwin/REDiskList.m:30:13: warning: 'NSArray' may not respond to 'sortUsingSelector:'
warning: caligula@0.4.4: [_disks sortUsingSelector:@selector(localizedCaseInsensitiveCompare:)];
warning: caligula@0.4.4: ~~~~~~ ^
warning: caligula@0.4.4: 1 warning generated.
Finished dev profile [unoptimized + debuginfo] target(s) in 0.17s
Running target/debug/caligula burn LICENSE -z none -s none -f --root always
Input file: LICENSE
Detected compression format: no compression
Select target disk disk2 | Generic STORAGE DEVICE Media - 62.6 GB (disk, remov
able: yes)

image

Performance impact?

As it turns out, negligible!

The following tests are run on the main branch, with uncompressed 1GB files.

Naively running Caligula on the base branch on Linux seems to indicate that this PR causes performance degradation.

image

However, that's because we aren't properly syncing the file, since std::io::File::flush is a no-op. Actually adding in a .sync_data() at the end of the write indicates that the disk is technically not fully written by that point, so the actual write speed is slower than it looks from the program's end.

image

These speeds are more in line with what we saw above.

@ifd3f
Copy link
Owner Author

ifd3f commented May 20, 2024

giving it a look-over and thorough testing in the morning when i'm not tired. we wouldn't want to merge bad code here :)

@ifd3f ifd3f self-assigned this May 20, 2024
@ifd3f ifd3f force-pushed the refactor/writer-process-o-direct branch from 9b4b1c2 to f0ab52c Compare May 20, 2024 13:45
@ifd3f ifd3f enabled auto-merge May 20, 2024 13:53
@ifd3f ifd3f merged commit 392c739 into main May 20, 2024
32 checks passed
@ifd3f ifd3f deleted the refactor/writer-process-o-direct branch May 21, 2024 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant