-
Notifications
You must be signed in to change notification settings - Fork 61
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
Force alignment for all chunk buffers #225
Conversation
src/index.rs
Outdated
|
||
pub type Chunk = [u8; CHUNK_LEN]; | ||
#[repr(C, align(8))] | ||
#[derive(PartialEq, Eq, Clone, Copy, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the impact of not being Copy
big ? (but not sure if we want to force explicit clone here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made it explicit. Also changed transmute_chunk
to operate on references, as it allows to eliminate a copy in some places.
@@ -35,3 +35,6 @@ pub type Key = [u8; KEY_SIZE]; | |||
|
|||
#[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))] | |||
compile_error!("parity-db only supports x86_64 and aarch64"); | |||
|
|||
#[cfg(not(target_endian = "little"))] | |||
compile_error!("parity-db only supports little-endian platforms"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense (keeping support would require a CI test indeed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it is not tested properly. The platform restriction of x86_64 and aarch64 limits it to little endian anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closes #223