Skip to content

Commit

Permalink
Revert "argon2: align blocks to 128-byte boundaries for modern x86-64…
Browse files Browse the repository at this point in the history
… and aarch64"

This reverts commit 1342037.

For reasons not yet understood, aligning blocks to 128-byte boundaries
results in worse performance than using 64-byte alignment.

Looking into this with perf suggests that it is *not* a cache problem,
but rather that the generated code is different and results in
substantially more instructions being executed when the blocks are
128-byte aligned.

For now, revert the alignment back to 64 bytes. While we're at it, also
remove the comment that suggests alignment is only needed to prevent
false sharing: it's possible that other places in the crate, which I
haven't checked, required (for correctness or best performance) the
64-byte alignment we're reverting back to.

It's worth noting that false sharing isn't generally a major issue in
Argon2: due to how memory is accessed, only the first and last few words
of a segment can (and most of the time probably still won't) experience
some false sharing with reads from other lanes.

Finally, changing the alignment with 1342037 would have a major
SemVer change:

https://doc.rust-lang.org/cargo/reference/semver.html#repr-align-n-change
  • Loading branch information
jonasmalacofilho committed Jan 21, 2025
1 parent 5eefce1 commit c0ce7f9
Showing 1 changed file with 1 addition and 5 deletions.
6 changes: 1 addition & 5 deletions argon2/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,8 @@ macro_rules! permute {
}

/// Structure for the (1 KiB) memory block implemented as 128 64-bit words.
//
// Blocks are 128-byte aligned to prevent false sharing. The specific alignment value is based on
// the notes in `crossbeam-utils::CachePadded` for modern architectures, which either use 128-byte
// cache lines (aarch64) or pull 64-byte cache lines in pairs (x86-64).
#[derive(Copy, Clone, Debug)]
#[repr(align(128))]
#[repr(align(64))]
pub struct Block([u64; Self::SIZE / 8]);

impl Block {
Expand Down

0 comments on commit c0ce7f9

Please sign in to comment.