[runtime/storage] Optimize unix Performance + Consistent resize Behavior
#1167
Conversation
| (sender, receiver, msg) | ||
| }, | ||
| |(mut sender, msg)| { | ||
| |(mut sender, _receiver, msg)| { |
There was a problem hiding this comment.
Dropping the receiver here caused the benchmark to silently fail.
| //! _To ensure the integrity of the data, a CRC32 checksum is appended to the end of the blob. | ||
| //! This ensures that partial writes are detected before any data is relied on._ | ||
| //! | ||
| //! _In the unlikely event that the current timestamp since the last `sync` is unchanged (as measured |
There was a problem hiding this comment.
On Windows, SystemTime is backed by a FILETIME whose native resolution is 100 ns. When creating a time that differs from the Unix epoch by only 1 ns it is rounded down to the closest 100-ns tick.
This means this trick isn't really guaranteed to work (it was jank anyways).
| assert_eq!(time.epoch_millis(), u64::MAX); | ||
| let time = time + Duration::from_millis(1); | ||
| assert_eq!(time.epoch_millis(), u64::MAX); | ||
| // Add 5 minutes |
There was a problem hiding this comment.
Windows’ FILETIME is a 64-bit unsigned count of 100-ns ticks since 1601-01-01, so the largest representable Unix-epoch time is ≈ year +60056:
https://learn.microsoft.com/en-us/windows/win32/sysinfo/file-times
|
|
||
| Fuzz: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 |
There was a problem hiding this comment.
.github/workflows/fast.yml
Outdated
| run: cargo +nightly fuzz build --fuzz-dir cryptography/fuzz | ||
| - name: Build stream | ||
| run: cargo +nightly fuzz build --fuzz-dir stream/fuzz | ||
| - name: Test all targets |
There was a problem hiding this comment.
Run fuzz tests for a short duration to make sure they actually work (rather than just compile).
…nt `truncate` (now `resize`) behavior (#1169)
unix Performanceunix Performance + Consistent resize Behavior
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes UNIX storage performance and standardizes blob resizing behavior by replacing various truncate operations with a unified resize method, and updates metadata to use versioning instead of timestamps.
- Replaces blob.truncate calls with blob.resize across multiple storage modules.
- Updates Metadata to use a u64 version field rather than a u128 timestamp and removes the last_update method.
- Updates tests, benchmarks, and GitHub workflow configurations to reflect these API changes.
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| utils/src/time.rs | Updates time tests: saturation test removed in favor of a 5-minute test. |
| utils/src/futures.rs | Minor comment and lifetime clarification updates in future polling. |
| utils/src/bitvec.rs | Adds lifetime parameter for BitIterator for improved type safety. |
| utils/src/array/u64.rs | Introduces a From implementation for U64. |
| stream/src/public_key/benches/sender_send.rs | Adjusts connection splitting to include receiver in the tuple. |
| stream/fuzz/fuzz_targets/handshake.rs | Modifies error handling in fuzz targets to simply await outcomes. |
| storage/src/mmr/journaled.rs | Replaces blob.truncate with blob.resize in MMR tests. |
| storage/src/metadata/storage.rs | Changes blob metadata representation from u128 to u64 and removes last_update. |
| storage/src/metadata/mod.rs | Updates documentation to reflect version-based metadata format. |
| storage/src/journal/* | Replaces truncate calls with resize for variable and fixed journals. |
| runtime/src/utils/buffer/* | Converts truncate logic to a unified resize approach in buffer utilities. |
| runtime/src/storage/* | Updates all storage backends (tokio, metered, memory, iouring, audited) to use resize. |
| runtime/src/lib.rs | Updates error messages and Blob trait to reflect resize behavior. |
| .github/workflows/* | Adjusts workflow configurations, timeouts, and caching to support new features. |
Comments suppressed due to low confidence (3)
utils/src/time.rs:73
- The saturation behavior test for SystemTime has been removed in favor of a 5-minute test. Please verify that this change is intentional and that no edge-case behavior is being lost.
assert_eq!(time.epoch_millis(), 300_000);
storage/src/metadata/storage.rs:20
- The change from a u128 timestamp to a u64 version for blob metadata is significant; please ensure that this new version scheme is sufficient for future requirements and that all dependent components are updated accordingly.
blobs: [(E::Blob, u64, u64); 2],
| run: cargo +nightly install cargo-fuzz --version ${{ env.FUZZ_VERSION }} | ||
| - name: Test all targets | ||
| env: | ||
| ASAN_OPTIONS: "detect_leaks=0" |
There was a problem hiding this comment.
Some of the fuzzing targets leak memory on some inputs. We primarily care about panics, so we just ignore this for now.
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #1167 +/- ##
==========================================
+ Coverage 91.02% 91.03% +0.01%
==========================================
Files 215 216 +1
Lines 57409 57447 +38
==========================================
+ Hits 52256 52297 +41
+ Misses 5153 5150 -3
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Related: #1135
Changes
fuzzfor 60s instead of just compilingfuzztimeout onmain: https://github.com/commonwarexyz/monorepo/actions/runs/15935358573/job/44953850493truncateconsistency: [runtime/storage] Useresizeinstead oftruncate+ Fix inconsistenttruncate(nowresize) behavior #1169versioninstead oftimestampfor metadata