Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-07-23 07:13:52 +01:00
Code Issues Projects Releases Packages Wiki Activity

Revert "Assert more things in hashtables."

Browse source 
This reverts commit e5023a3eb4.
This commit is contained in:
Manish Goregaokar 2017-10-23 13:45:57 -07:00
parent fde96d0ff5
commit f7ad19f500
1 changed files with 26 additions and 73 deletions
Download patch file Download diff file Expand all files Collapse all files

99
components/hashglobe/src/table.rs
View file

@ -43,7 +43,7 @@ struct TaggedHashUintPtr(Unique<HashUint>);
impl TaggedHashUintPtr { impl TaggedHashUintPtr {
#[inline] #[inline]
unsafe fn new(ptr: *mut HashUint) -> Self { unsafe fn new(ptr: *mut HashUint) -> Self {
assert!(ptr as usize & 1 == 0 || ptr as usize == EMPTY as usize); debug_assert!(ptr as usize & 1 == 0 || ptr as usize == EMPTY as usize);
TaggedHashUintPtr(Unique::new_unchecked(ptr)) TaggedHashUintPtr(Unique::new_unchecked(ptr))
} }
@ -117,7 +117,6 @@ pub struct RawTable<K, V> {
capacity_mask: usize, capacity_mask: usize,
size: usize, size: usize,
hashes: TaggedHashUintPtr, hashes: TaggedHashUintPtr,
bytes_allocated: usize,
// Because K/V do not appear directly in any of the types in the struct, // Because K/V do not appear directly in any of the types in the struct,
// inform rustc that in fact instances of K and V are reachable from here. // inform rustc that in fact instances of K and V are reachable from here.
@ -246,15 +245,6 @@ impl<K, V> RawBucket<K, V> {
unsafe fn hash_pair(&self) -> (*mut HashUint, *mut (K, V)) { unsafe fn hash_pair(&self) -> (*mut HashUint, *mut (K, V)) {
(self.hash(), self.pair()) (self.hash(), self.pair())
} }
fn assert_bounds(&self, bytes_allocated: usize) {
let base = self.hash_start as *mut u8;
let (h, p) = unsafe { self.hash_pair() };
assert!((h as *mut u8) < (p as *mut u8), "HashMap Corruption - hash offset not below pair offset");
let end = unsafe { p.offset(1) } as *mut u8;
assert!(end > base, "HashMap Corruption - end={:?}, base={:?}", end, base);
assert!(end <= unsafe { base.offset(bytes_allocated as isize) }, "HashMap Corruption - end={:?}, base={:?}", end, base);
}
} }
// Buckets hold references to the table. // Buckets hold references to the table.
@ -358,7 +348,8 @@ impl<K, V, M: Deref<Target = RawTable<K, V>>> Bucket<K, V, M> {
pub fn at_index(table: M, ib_index: usize) -> Bucket<K, V, M> { pub fn at_index(table: M, ib_index: usize) -> Bucket<K, V, M> {
// if capacity is 0, then the RawBucket will be populated with bogus pointers. // if capacity is 0, then the RawBucket will be populated with bogus pointers.
// This is an uncommon case though, so avoid it in release builds. // This is an uncommon case though, so avoid it in release builds.
assert!(table.capacity() > 0, "HashMap Corruption - Table should have capacity at this point"); debug_assert!(table.capacity() > 0,
"Table should have capacity at this point");
let ib_index = ib_index & table.capacity_mask; let ib_index = ib_index & table.capacity_mask;
Bucket { Bucket {
raw: table.raw_bucket_at(ib_index), raw: table.raw_bucket_at(ib_index),
@ -431,13 +422,11 @@ impl<K, V, M: Deref<Target = RawTable<K, V>>> Bucket<K, V, M> {
/// Modifies the bucket in place to make it point to the next slot. /// Modifies the bucket in place to make it point to the next slot.
pub fn next(&mut self) { pub fn next(&mut self) {
self.raw.idx = self.raw.idx.wrapping_add(1) & self.table.capacity_mask; self.raw.idx = self.raw.idx.wrapping_add(1) & self.table.capacity_mask;
self.raw.assert_bounds(self.table.bytes_allocated);
} }
/// Modifies the bucket in place to make it point to the previous slot. /// Modifies the bucket in place to make it point to the previous slot.
pub fn prev(&mut self) { pub fn prev(&mut self) {
self.raw.idx = self.raw.idx.wrapping_sub(1) & self.table.capacity_mask; self.raw.idx = self.raw.idx.wrapping_sub(1) & self.table.capacity_mask;
self.raw.assert_bounds(self.table.bytes_allocated);
} }
} }
@ -560,7 +549,7 @@ impl<'t, K, V> FullBucket<K, V, &'t mut RawTable<K, V>> {
/// This works similarly to `put`, building an `EmptyBucket` out of the /// This works similarly to `put`, building an `EmptyBucket` out of the
/// taken bucket. /// taken bucket.
pub fn take(self) -> (EmptyBucket<K, V, &'t mut RawTable<K, V>>, K, V) { pub fn take(self) -> (EmptyBucket<K, V, &'t mut RawTable<K, V>>, K, V) {
self.table.size = self.table.size.checked_sub(1).unwrap(); self.table.size -= 1;
unsafe { unsafe {
*self.raw.hash() = EMPTY_BUCKET; *self.raw.hash() = EMPTY_BUCKET;
@ -675,7 +664,7 @@ impl<K, V, M> GapThenFull<K, V, M>
/// Panics if `target_alignment` is not a power of two. /// Panics if `target_alignment` is not a power of two.
#[inline] #[inline]
fn round_up_to_next(unrounded: usize, target_alignment: usize) -> usize { fn round_up_to_next(unrounded: usize, target_alignment: usize) -> usize {
assert!(target_alignment.is_power_of_two(), "HashMap Corruption - alignment not power of two"); assert!(target_alignment.is_power_of_two());
(unrounded + target_alignment - 1) & !(target_alignment - 1) (unrounded + target_alignment - 1) & !(target_alignment - 1)
} }
@ -745,11 +734,9 @@ impl<K, V> RawTable<K, V> {
size: 0, size: 0,
capacity_mask: capacity.wrapping_sub(1), capacity_mask: capacity.wrapping_sub(1),
hashes: TaggedHashUintPtr::new(EMPTY as *mut HashUint), hashes: TaggedHashUintPtr::new(EMPTY as *mut HashUint),
bytes_allocated: 0,
marker: marker::PhantomData, marker: marker::PhantomData,
}); });
} }
assert!(capacity.is_power_of_two(), "HashMap Corruption - capacity not power of two");
// No need for `checked_mul` before a more restrictive check performed // No need for `checked_mul` before a more restrictive check performed
// later in this method. // later in this method.
@ -801,13 +788,11 @@ impl<K, V> RawTable<K, V> {
ptr::write_bytes(buffer, 0xe7, size); ptr::write_bytes(buffer, 0xe7, size);
let hashes = buffer.offset(hash_offset as isize) as *mut HashUint; let hashes = buffer.offset(hash_offset as isize) as *mut HashUint;
assert!(hashes as *mut u8 == buffer, "HashMap Corruption - Nonzero hash_offset");
Ok(RawTable { Ok(RawTable {
capacity_mask: capacity.wrapping_sub(1), capacity_mask: capacity.wrapping_sub(1),
size: 0, size: 0,
hashes: TaggedHashUintPtr::new(hashes), hashes: TaggedHashUintPtr::new(hashes),
bytes_allocated: size,
marker: marker::PhantomData, marker: marker::PhantomData,
}) })
} }
@ -818,23 +803,17 @@ impl<K, V> RawTable<K, V> {
let (pairs_offset, _, oflo) = let (pairs_offset, _, oflo) =
calculate_offsets(hashes_size, pairs_size, align_of::<(K, V)>()); calculate_offsets(hashes_size, pairs_size, align_of::<(K, V)>());
assert!(!oflo, "HashMap Corruption - capacity overflow"); debug_assert!(!oflo, "capacity overflow");
assert!(pairs_offset as isize > 0, "HashMap Corruption - pairs offset={}", pairs_offset);
assert!(index as isize >= 0, "HashMap Corruption - index={}", index);
assert!(index < self.capacity(), "HashMap Corruption - index={}", index);
let buffer = self.hashes.ptr() as *mut u8; let buffer = self.hashes.ptr() as *mut u8;
let bucket = unsafe { unsafe {
RawBucket { RawBucket {
hash_start: buffer as *mut HashUint, hash_start: buffer as *mut HashUint,
pair_start: buffer.offset(pairs_offset as isize) as *const (K, V), pair_start: buffer.offset(pairs_offset as isize) as *const (K, V),
idx: index, idx: index,
_marker: marker::PhantomData, _marker: marker::PhantomData,
} }
}; }
bucket.assert_bounds(self.bytes_allocated);
bucket
} }
/// Returns a raw pointer to the table's buffer. /// Returns a raw pointer to the table's buffer.
@ -866,9 +845,8 @@ impl<K, V> RawTable<K, V> {
fn raw_buckets(&self) -> RawBuckets<K, V> { fn raw_buckets(&self) -> RawBuckets<K, V> {
RawBuckets { RawBuckets {
raw: if self.capacity() == 0 { None } else { Some(self.raw_bucket_at(0)) }, raw: self.raw_bucket_at(0),
elems_left: self.size, elems_left: self.size,
bytes_allocated: self.bytes_allocated,
marker: marker::PhantomData, marker: marker::PhantomData,
} }
} }
@ -887,13 +865,12 @@ impl<K, V> RawTable<K, V> {
} }
pub fn into_iter(self) -> IntoIter<K, V> { pub fn into_iter(self) -> IntoIter<K, V> {
let RawBuckets { raw, elems_left, bytes_allocated, .. } = self.raw_buckets(); let RawBuckets { raw, elems_left, .. } = self.raw_buckets();
// Replace the marker regardless of lifetime bounds on parameters. // Replace the marker regardless of lifetime bounds on parameters.
IntoIter { IntoIter {
iter: RawBuckets { iter: RawBuckets {
raw, raw,
elems_left, elems_left,
bytes_allocated,
marker: marker::PhantomData, marker: marker::PhantomData,
}, },
table: self, table: self,
@ -901,13 +878,12 @@ impl<K, V> RawTable<K, V> {
} }
pub fn drain(&mut self) -> Drain<K, V> { pub fn drain(&mut self) -> Drain<K, V> {
let RawBuckets { raw, elems_left, bytes_allocated, .. } = self.raw_buckets(); let RawBuckets { raw, elems_left, .. } = self.raw_buckets();
// Replace the marker regardless of lifetime bounds on parameters. // Replace the marker regardless of lifetime bounds on parameters.
Drain { Drain {
iter: RawBuckets { iter: RawBuckets {
raw, raw,
elems_left, elems_left,
bytes_allocated,
marker: marker::PhantomData, marker: marker::PhantomData,
}, },
table: Shared::from(self), table: Shared::from(self),
@ -919,22 +895,18 @@ impl<K, V> RawTable<K, V> {
/// state and should only be used for dropping the table's remaining /// state and should only be used for dropping the table's remaining
/// entries. It's used in the implementation of Drop. /// entries. It's used in the implementation of Drop.
unsafe fn rev_drop_buckets(&mut self) { unsafe fn rev_drop_buckets(&mut self) {
// initialize the raw bucket past the end of the table
let mut raw = self.raw_bucket_at(self.capacity());
let mut elems_left = self.size; let mut elems_left = self.size;
if elems_left == 0 {
return; while elems_left != 0 {
} raw.idx -= 1;
let mut raw = self.raw_bucket_at(self.capacity() - 1);
loop {
if *raw.hash() != EMPTY_BUCKET { if *raw.hash() != EMPTY_BUCKET {
elems_left -= 1;
ptr::drop_in_place(raw.pair()); ptr::drop_in_place(raw.pair());
elems_left = elems_left.checked_sub(1).unwrap();
if elems_left == 0 {
return;
} }
} }
raw.idx = raw.idx.checked_sub(1).unwrap();
raw.assert_bounds(self.bytes_allocated);
}
} }
/// Set the table tag /// Set the table tag
@ -951,11 +923,8 @@ impl<K, V> RawTable<K, V> {
/// A raw iterator. The basis for some other iterators in this module. Although /// A raw iterator. The basis for some other iterators in this module. Although
/// this interface is safe, it's not used outside this module. /// this interface is safe, it's not used outside this module.
struct RawBuckets<'a, K, V> { struct RawBuckets<'a, K, V> {
// We use an Option here to avoid ever constructing a RawBucket for raw: RawBucket<K, V>,
// invalid memory.
raw: Option<RawBucket<K, V>>,
elems_left: usize, elems_left: usize,
bytes_allocated: usize,
// Strictly speaking, this should be &'a (K,V), but that would // Strictly speaking, this should be &'a (K,V), but that would
// require that K:'a, and we often use RawBuckets<'static...> for // require that K:'a, and we often use RawBuckets<'static...> for
@ -971,7 +940,6 @@ impl<'a, K, V> Clone for RawBuckets<'a, K, V> {
RawBuckets { RawBuckets {
raw: self.raw, raw: self.raw,
elems_left: self.elems_left, elems_left: self.elems_left,
bytes_allocated: self.bytes_allocated,
marker: marker::PhantomData, marker: marker::PhantomData,
} }
} }
@ -988,17 +956,12 @@ impl<'a, K, V> Iterator for RawBuckets<'a, K, V> {
loop { loop {
unsafe { unsafe {
let item = self.raw.unwrap(); let item = self.raw;
self.raw.idx += 1;
if *item.hash() != EMPTY_BUCKET { if *item.hash() != EMPTY_BUCKET {
self.elems_left = self.elems_left.checked_sub(1).unwrap(); self.elems_left -= 1;
if self.elems_left != 0 {
self.raw.as_mut().unwrap().idx += 1;
self.raw.as_ref().unwrap().assert_bounds(self.bytes_allocated);
}
return Some(item); return Some(item);
} }
self.raw.as_mut().unwrap().idx += 1;
self.raw.as_ref().unwrap().assert_bounds(self.bytes_allocated);
} }
} }
} }
@ -1133,7 +1096,7 @@ impl<K, V> Iterator for IntoIter<K, V> {
fn next(&mut self) -> Option<(SafeHash, K, V)> { fn next(&mut self) -> Option<(SafeHash, K, V)> {
self.iter.next().map(|raw| { self.iter.next().map(|raw| {
self.table.size = self.table.size.checked_sub(1).unwrap(); self.table.size -= 1;
unsafe { unsafe {
let (k, v) = ptr::read(raw.pair()); let (k, v) = ptr::read(raw.pair());
(SafeHash { hash: *raw.hash() }, k, v) (SafeHash { hash: *raw.hash() }, k, v)
@ -1159,7 +1122,7 @@ impl<'a, K, V> Iterator for Drain<'a, K, V> {
fn next(&mut self) -> Option<(SafeHash, K, V)> { fn next(&mut self) -> Option<(SafeHash, K, V)> {
self.iter.next().map(|raw| { self.iter.next().map(|raw| {
unsafe { unsafe {
self.table.as_mut().size = self.table.as_mut().size.checked_sub(1).unwrap(); self.table.as_mut().size -= 1;
let (k, v) = ptr::read(raw.pair()); let (k, v) = ptr::read(raw.pair());
(SafeHash { hash: ptr::replace(&mut *raw.hash(), EMPTY_BUCKET) }, k, v) (SafeHash { hash: ptr::replace(&mut *raw.hash(), EMPTY_BUCKET) }, k, v)
} }
@ -1188,28 +1151,18 @@ impl<K: Clone, V: Clone> Clone for RawTable<K, V> {
unsafe { unsafe {
let cap = self.capacity(); let cap = self.capacity();
let mut new_ht = RawTable::new_uninitialized(cap); let mut new_ht = RawTable::new_uninitialized(cap);
if cap == 0 {
return new_ht;
}
let mut new_buckets = new_ht.raw_bucket_at(0); let mut new_buckets = new_ht.raw_bucket_at(0);
let mut buckets = self.raw_bucket_at(0); let mut buckets = self.raw_bucket_at(0);
loop { while buckets.idx < cap {
*new_buckets.hash() = *buckets.hash(); *new_buckets.hash() = *buckets.hash();
if *new_buckets.hash() != EMPTY_BUCKET { if *new_buckets.hash() != EMPTY_BUCKET {
let pair_ptr = buckets.pair(); let pair_ptr = buckets.pair();
let kv = ((*pair_ptr).0.clone(), (*pair_ptr).1.clone()); let kv = ((*pair_ptr).0.clone(), (*pair_ptr).1.clone());
ptr::write(new_buckets.pair(), kv); ptr::write(new_buckets.pair(), kv);
} }
if buckets.idx == cap - 1 {
break;
}
buckets.idx += 1; buckets.idx += 1;
buckets.assert_bounds(self.bytes_allocated);
new_buckets.idx += 1; new_buckets.idx += 1;
new_buckets.assert_bounds(new_ht.bytes_allocated);
} }
new_ht.size = self.size(); new_ht.size = self.size();
@ -1248,7 +1201,7 @@ impl<K, V> Drop for RawTable<K, V> {
pairs_size, pairs_size,
align_of::<(K, V)>()); align_of::<(K, V)>());
assert!(!oflo, "HashMap Corruption - should be impossible"); debug_assert!(!oflo, "should be impossible");
unsafe { unsafe {
dealloc(self.hashes.ptr() as *mut u8, align); dealloc(self.hashes.ptr() as *mut u8, align);