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.

Sign up for GitHub

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

Optimized SipHash implementation #13114

Closed
wants to merge 5 commits into from
Closed

Optimized SipHash implementation #13114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3167e86
Optimized SipHash implementation
Mar 23, 2014
aa61854
Reverted change to u8to64_le due to it being invalid on unaligned arc…
Mar 28, 2014
5bfe692
Switched to a macro for the write_le functions
Mar 29, 2014
7a99a1b
Added u64_from_le_bytes to io::extensions and used it in the SipHash …
Mar 30, 2014
5dba2cc
Switched wholly over to u64_from_le_bytes
Mar 30, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 78 additions & 42 deletions src/libstd/hash/sip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ use clone::Clone;
use container::Container;
use default::Default;
use io::{IoResult, Writer};
use io::extensions::u64_from_le_bytes;
use iter::Iterator;
use mem::size_of_val;
use result::Ok;
use slice::ImmutableVector;

Expand All @@ -43,26 +45,14 @@ pub struct SipState {
priv v1: u64,
priv v2: u64,
priv v3: u64,
priv tail: [u8, ..8], // unprocessed bytes
priv tail: u64, // unprocessed bytes, stored in little endian format
priv ntail: uint, // how many bytes in tail are valid
}

// sadly, these macro definitions can't appear later,
// because they're needed in the following defs;
// this design could be improved.

macro_rules! u8to64_le (
($buf:expr, $i:expr) =>
($buf[0+$i] as u64 |
$buf[1+$i] as u64 << 8 |
$buf[2+$i] as u64 << 16 |
$buf[3+$i] as u64 << 24 |
$buf[4+$i] as u64 << 32 |
$buf[5+$i] as u64 << 40 |
$buf[6+$i] as u64 << 48 |
$buf[7+$i] as u64 << 56)
)

macro_rules! rotl (
($x:expr, $b:expr) =>
(($x << $b) | ($x >> (64 - $b)))
Expand All @@ -80,6 +70,34 @@ macro_rules! compress (
})
)

macro_rules! make_write_le (
() =>
({
self.tail |= n as u64 << 8*self.ntail;
self.ntail += size_of_val(&n);

if self.ntail >= 8 {
let m = self.tail;

self.v3 ^= m;
compress!(self.v0, self.v1, self.v2, self.v3);
compress!(self.v0, self.v1, self.v2, self.v3);
self.v0 ^= m;

self.ntail -= 8;
if self.ntail == 0 {
self.tail = 0;
} else {
self.tail = n as u64 >> 64 - 8*self.ntail;
}
}

self.length += size_of_val(&n);

Ok(())
})
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, future hygienic changes will prevent this macro from compiling. Right now I think that function arguments are not hygienic (which is a bug).

To fix this, the macro just needs to take the enclosed scope as arguments instead of using the explicit identifiers. This means that the macro will need to look something like:

macro_rules!( ($self:expr, $n:expr) => ({ ... }) )


impl SipState {
/// Create a `SipState` that is keyed off the provided keys.
#[inline]
Expand All @@ -98,7 +116,7 @@ impl SipState {
v1: 0,
v2: 0,
v3: 0,
tail: [ 0, 0, 0, 0, 0, 0, 0, 0 ],
tail: 0,
ntail: 0,
};
state.reset();
Expand All @@ -114,6 +132,7 @@ impl SipState {
self.v2 = self.k0 ^ 0x6c7967656e657261;
self.v3 = self.k1 ^ 0x7465646279746573;
self.ntail = 0;
self.tail = 0;
}

/// Return the computed hash.
Expand All @@ -124,15 +143,7 @@ impl SipState {
let mut v2 = self.v2;
let mut v3 = self.v3;

let mut b : u64 = (self.length as u64 & 0xff) << 56;

if self.ntail > 0 { b |= self.tail[0] as u64 << 0; }
if self.ntail > 1 { b |= self.tail[1] as u64 << 8; }
if self.ntail > 2 { b |= self.tail[2] as u64 << 16; }
if self.ntail > 3 { b |= self.tail[3] as u64 << 24; }
if self.ntail > 4 { b |= self.tail[4] as u64 << 32; }
if self.ntail > 5 { b |= self.tail[5] as u64 << 40; }
if self.ntail > 6 { b |= self.tail[6] as u64 << 48; }
let b : u64 = ((self.length as u64 & 0xff) << 56) | self.tail;

v3 ^= b;
compress!(v0, v1, v2, v3);
Expand Down Expand Up @@ -161,29 +172,20 @@ impl Writer for SipState {
needed = 8 - self.ntail;

if length < needed {
let mut t = 0;
while t < length {
self.tail[self.ntail+t] = msg[t];
t += 1;
}
self.tail |= u64_from_le_bytes(msg, 0, length) << 8*self.ntail;
self.ntail += length;
return Ok(());
}

let mut t = 0;
while t < needed {
self.tail[self.ntail+t] = msg[t];
t += 1;
}

let m = u8to64_le!(self.tail, 0);
let m = self.tail | u64_from_le_bytes(msg, 0, needed) << 8*self.ntail;

self.v3 ^= m;
compress!(self.v0, self.v1, self.v2, self.v3);
compress!(self.v0, self.v1, self.v2, self.v3);
self.v0 ^= m;

self.ntail = 0;
self.tail = 0;
}

// Buffered tail is now flushed, process new input.
Expand All @@ -193,7 +195,7 @@ impl Writer for SipState {

let mut i = needed;
while i < end {
let mi = u8to64_le!(msg, i);
let mi = u64_from_le_bytes(msg, i, 8);

self.v3 ^= mi;
compress!(self.v0, self.v1, self.v2, self.v3);
Expand All @@ -203,15 +205,35 @@ impl Writer for SipState {
i += 8;
}

let mut t = 0u;
while t < left {
self.tail[t] = msg[i+t];
t += 1
}
self.tail = u64_from_le_bytes(msg, i, left);
self.ntail = left;

Ok(())
}

// We override these functions because by default, they convert `n` to bytes (possibly with an
// expensive byte swap) then convert those bytes back into a `u64` (possibly with another byte swap which
// reverses the first). Additionally, in these cases, we know that at most one flush will be
// needed, and so can optimize appropriately.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you cite the performance numbers in this comment?

#[inline]
fn write_u8(&mut self, n: u8) -> IoResult<()> {
make_write_le!()
}

#[inline]
fn write_le_u16(&mut self, n: u16) -> IoResult<()> {
make_write_le!()
}

#[inline]
fn write_le_u32(&mut self, n: u32) -> IoResult<()> {
make_write_le!()
}

#[inline]
fn write_le_u64(&mut self, n: u64) -> IoResult<()> {
make_write_le!()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this overwrite the big endian versions? (the comment could explain why)

}

impl Clone for SipState {
Expand Down Expand Up @@ -288,6 +310,7 @@ pub fn hash_with_keys<T: Hash<SipState>>(k0: u64, k1: u64, value: &T) -> u64 {
mod tests {
extern crate test;
use io::Writer;
use io::extensions::u64_from_le_bytes;
use iter::Iterator;
use num::ToStrRadix;
use option::{Some, None};
Expand Down Expand Up @@ -417,7 +440,7 @@ mod tests {

while t < 64 {
debug!("siphash test {}", t);
let vec = u8to64_le!(vecs[t], 0);
let vec = u64_from_le_bytes(vecs[t], 0, 8);
let out = hash_with_keys(k0, k1, &Bytes(buf.as_slice()));
debug!("got {:?}, expected {:?}", out, vec);
assert_eq!(vec, out);
Expand Down Expand Up @@ -523,6 +546,19 @@ mod tests {
})
}

#[bench]
fn bench_long_str(bh: &mut BenchHarness) {
let s = "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor \
incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud \
exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute \
irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla \
pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui \
officia deserunt mollit anim id est laborum.";
bh.iter(|| {
assert_eq!(hash(&s), 17717065544121360093);
})
}

struct Compound {
x: u8,
y: u16,
Expand Down
60 changes: 60 additions & 0 deletions src/libstd/io/extensions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,37 @@ pub fn u64_from_be_bytes(data: &[u8], start: uint, size: uint) -> u64 {
}
}

/// Extracts an 8-bit to 64-bit unsigned little-endian value from the given byte
/// buffer and returns it as a 64-bit value.
///
/// Arguments:
///
/// * `data`: The buffer in which to extract the value.
/// * `start`: The offset at which to extract the value.
/// * `size`: The size of the value in bytes to extract. This must be 8 or
/// less, or task failure occurs. If this is less than 8, then only
/// that many bytes are parsed. For example, if `size` is 4, then a
/// 32-bit value is parsed.
pub fn u64_from_le_bytes(data: &[u8], start: uint, size: uint) -> u64 {
use ptr::{copy_nonoverlapping_memory};
use mem::from_le64;
use slice::MutableVector;

assert!(size <= 8u);

if data.len() - start < size {
fail!("index out of bounds");
}

let mut buf = [0u8, ..8];
unsafe {
let ptr = data.as_ptr().offset(start as int);
let out = buf.as_mut_ptr();
copy_nonoverlapping_memory(out, ptr, size);
from_le64(*(out as *i64)) as u64
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the above function look quite similar, perhaps they could be refactored? Could the byte-swapping intrinsics be used in combination with reading using the big endian function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather define the big endian function in terms of the little endian function, as the little endian function is slightly simpler, but merging seems reasonable, even if it breaks symmetry.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way is fine by me, I'd just shoot for less duplication.


#[cfg(test)]
mod test {
use prelude::*;
Expand Down Expand Up @@ -498,6 +529,35 @@ mod test {
assert_eq!(u64_from_be_bytes(buf, 1, 7), 0x02030405060708);
assert_eq!(u64_from_be_bytes(buf, 1, 8), 0x0203040506070809);
}

#[test]
fn test_u64_from_le_bytes() {
use super::u64_from_le_bytes;

let buf = [0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09];

// Aligned access
assert_eq!(u64_from_le_bytes(buf, 0, 0), 0);
assert_eq!(u64_from_le_bytes(buf, 0, 1), 0x01);
assert_eq!(u64_from_le_bytes(buf, 0, 2), 0x0201);
assert_eq!(u64_from_le_bytes(buf, 0, 3), 0x030201);
assert_eq!(u64_from_le_bytes(buf, 0, 4), 0x04030201);
assert_eq!(u64_from_le_bytes(buf, 0, 5), 0x0504030201);
assert_eq!(u64_from_le_bytes(buf, 0, 6), 0x060504030201);
assert_eq!(u64_from_le_bytes(buf, 0, 7), 0x07060504030201);
assert_eq!(u64_from_le_bytes(buf, 0, 8), 0x0807060504030201);

// Unaligned access
assert_eq!(u64_from_le_bytes(buf, 1, 0), 0);
assert_eq!(u64_from_le_bytes(buf, 1, 1), 0x02);
assert_eq!(u64_from_le_bytes(buf, 1, 2), 0x0302);
assert_eq!(u64_from_le_bytes(buf, 1, 3), 0x040302);
assert_eq!(u64_from_le_bytes(buf, 1, 4), 0x05040302);
assert_eq!(u64_from_le_bytes(buf, 1, 5), 0x0605040302);
assert_eq!(u64_from_le_bytes(buf, 1, 6), 0x070605040302);
assert_eq!(u64_from_le_bytes(buf, 1, 7), 0x08070605040302);
assert_eq!(u64_from_le_bytes(buf, 1, 8), 0x0908070605040302);
}
}

#[cfg(test)]
Expand Down