Update random-access-storage to 3.0.0 #15
Conversation
This bumps `random-access-storage` to 3.0.0, and fixes compilation errors necessary on the process. This commit implements `sync_all` using `Ok(())` as suggested on the PR introducing the method as I've understood we are using `Vec` and they are not buffered. Related to datrs/random-access-storage#6 Related to datrs/random-access-storage#18 Related to datrs/random-access-storage#17 Closes datrs#14
When running 'rustup component add' we are receiveing a message that it is not available on nightly. > component 'rustfmt' for target 'x86_64-unknown-linux-gnu' is unavailable for download for channel 'nightly' This commit changes it to run using stable channel instead.
src/lib.rs
Outdated
| @@ -16,20 +16,19 @@ use std::io; | |||
| #[derive(Debug)] | |||
| pub struct RandomAccessMemory { | |||
| /// The length length of each buffer. | |||
| page_size: usize, | |||
| page_size: u64, | |||
There was a problem hiding this comment.
At first, I didn't quite understand the change, and I've changed all usize items used on the function to be u64. After doing that, and trying to understand it better, it might not be correct, right?
Given we are using a Vec as our backing store, should the page_size should remain usize?
Should length also be usize to pass to the user of the API that this crate only handles 4G as memory?
There was a problem hiding this comment.
Yes, I think page_size should remain usize as it must be less than usize::max_value() to fit into the address space. Also, I guess in practice it will be much smaller than usize::max_value().
As for length, it should be u64 to handle large (sparse) storage. The current implementation doesn't seem to handle it properly, but it could (see #17).
|
I've applied the changes on |
|
This looks great, thank you! |
Choose one: 🐛 + 🙋
Checklist
Context
This bumps
random-access-storageto 3.0.0, and fixes compilationerrors necessary on the process.
This commit implements
sync_allusingOk(())as suggested on the PRintroducing the method as I've understood we are using
Vecand theyare not buffered.
Related to datrs/random-access-storage#6
Related to datrs/random-access-storage#18
Related to datrs/random-access-storage#17
Closes #14
Semver Changes
Major bump, as signatures have changed