From a66b6e5b7a3e2e003e3d154260f195f03bab7459 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Fri, 24 Jan 2020 09:20:52 -0800 Subject: [PATCH 1/2] HeaderMap: Store pos and hash as u16 HeaderMap currently truncates all hashes to 16 bits, and limits its capacity to 32768 elements. The comments say this is done in order to store positions and hashes as u16, to improve cache locality. However, they are currently stored as usize. This patch changes the code to match the comments. This does not appear to cause any measurable improvement or regression in speed in the HeaderMap benchmarks. However, it should at least reduce the memory footprint of every Headermap. --- src/header/map.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/header/map.rs b/src/header/map.rs index 3713e83a..3d907799 100644 --- a/src/header/map.rs +++ b/src/header/map.rs @@ -232,7 +232,7 @@ enum Cursor { /// You may notice that `u16` may represent more than 32,768 values. This is /// true, but 32,768 should be plenty and it allows us to reserve the top bit /// for future usage. -type Size = usize; +type Size = u16; /// This limit falls out from above. const MAX_SIZE: usize = (1 << 15); @@ -252,7 +252,7 @@ struct Pos { /// bits is fine since we know that the `indices` vector will never grow beyond /// that size. #[derive(Debug, Copy, Clone, Eq, PartialEq)] -struct HashValue(usize); +struct HashValue(u16); /// Stores the data associated with a `HeaderMap` entry. Only the first value is /// included in this struct. If a header name has more than one associated @@ -646,7 +646,7 @@ impl HeaderMap { assert!(cap != 0, "header map reserve overflowed"); if self.entries.len() == 0 { - self.mask = cap - 1; + self.mask = cap as Size - 1; self.indices = vec![Pos::none(); cap].into_boxed_slice(); self.entries = Vec::with_capacity(usable_capacity(cap)); } else { @@ -3136,7 +3136,7 @@ impl Pos { #[inline] fn resolve(&self) -> Option<(usize, HashValue)> { if self.is_some() { - Some((self.index, self.hash)) + Some((self.index as usize, self.hash)) } else { None } @@ -3192,13 +3192,13 @@ fn to_raw_capacity(n: usize) -> usize { #[inline] fn desired_pos(mask: Size, hash: HashValue) -> usize { - (hash.0 & mask) + (hash.0 & mask) as usize } /// The number of steps that `current` is forward of the desired position for hash #[inline] fn probe_distance(mask: Size, hash: HashValue, current: usize) -> usize { - current.wrapping_sub(desired_pos(mask, hash)) & mask + current.wrapping_sub(desired_pos(mask, hash)) & mask as usize } fn hash_elem_using(danger: &Danger, k: &K) -> HashValue @@ -3224,7 +3224,7 @@ where } }; - HashValue((hash & MASK) as usize) + HashValue((hash & MASK) as u16) } /* From 5197f93d6755c92e6c9831e9e7eae6d74111f53c Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Fri, 24 Jan 2020 09:52:26 -0800 Subject: [PATCH 2/2] Fix overflow check for raw capacity. The actual maximum capacity is 24578 (i.e. `32768 * 3/4`). A higher number would cause the raw capacity (i.e. `capacity + capacity/3` rounded to the next power of 2) to overflow `u16`. --- src/header/map.rs | 7 ++++--- tests/header_map.rs | 12 ++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/header/map.rs b/src/header/map.rs index 3d907799..58349fa8 100644 --- a/src/header/map.rs +++ b/src/header/map.rs @@ -465,8 +465,6 @@ impl HeaderMap { /// assert_eq!(12, map.capacity()); /// ``` pub fn with_capacity(capacity: usize) -> HeaderMap { - assert!(capacity <= MAX_SIZE, "requested capacity too large"); - if capacity == 0 { HeaderMap { mask: 0, @@ -477,6 +475,7 @@ impl HeaderMap { } } else { let raw_cap = to_raw_capacity(capacity).next_power_of_two(); + assert!(raw_cap <= MAX_SIZE, "requested capacity too large"); debug_assert!(raw_cap > 0); HeaderMap { @@ -642,7 +641,7 @@ impl HeaderMap { if cap > self.indices.len() { let cap = cap.next_power_of_two(); - assert!(cap < MAX_SIZE, "header map reserve over max capacity"); + assert!(cap <= MAX_SIZE, "header map reserve over max capacity"); assert!(cap != 0, "header map reserve overflowed"); if self.entries.len() == 0 { @@ -1543,6 +1542,7 @@ impl HeaderMap { #[inline] fn grow(&mut self, new_raw_cap: usize) { + assert!(new_raw_cap <= MAX_SIZE, "requested capacity too large"); // This path can never be reached when handling the first allocation in // the map. @@ -3109,6 +3109,7 @@ impl ops::IndexMut for RawLinks { impl Pos { #[inline] fn new(index: usize, hash: HashValue) -> Self { + debug_assert!(index < MAX_SIZE); Pos { index: index as Size, hash: hash, diff --git a/tests/header_map.rs b/tests/header_map.rs index 91c8f3f6..39893568 100644 --- a/tests/header_map.rs +++ b/tests/header_map.rs @@ -43,6 +43,18 @@ fn reserve_over_capacity() { headers.reserve(50_000); // over MAX_SIZE } +#[test] +fn with_capacity_max() { + // The largest capacity such that (cap + cap / 3) < MAX_SIZE. + HeaderMap::::with_capacity(24_576); +} + +#[test] +#[should_panic] +fn with_capacity_overflow() { + HeaderMap::::with_capacity(24_577); +} + #[test] #[should_panic] fn reserve_overflow() {