safe but infallible extend (wip)

This is a work in progress idea I had to use heap.first.size to cache pending
extend requests that were too small to safely execute. Once there *is* enough
space to align and store a Hole, we can do so.

This does introduce possible "dead_space" between the last allocation and the
newly created Hole after extending, due to the need to align upward before
writing the new Hole. The dead space may be reclaimed sometimes, but perhaps
it's impossible to guarantee that. So this idea might be bad, though it only
affects odd-aligned heap.top() situations.

When we dealloc, would it be sound to *align_up_size* the Layout being free'd,
knowing that we only ever put Holes at aligned addresses?

Example (assume 64-bit arch):
1. New heap: aligned bottom, but size 17
2. Alloc is requested of size 17, align 1, and succeeds. No free holes exist.
3. Heap extend by 15 (to align for Hole) + 16 (Hole size). Extend succeeds.
4. Heap looks like this: [17b in use][15b dead][16b Hole]
5. First allocation is free'd, and the Hole size is rounded up
   Note: we need to check against heap.size() and heap.pending_extend() before
   rounding up all the way
6. Heap looks like this: [32b Hole][16b Hole]

Author: evanrichter

src/hole.rs

@@ -443,29 +443,50 @@ impl HoleList {
         })
     }
 
+    // amount previously extended but not big enough to be claimed by a Hole yet
+    pub(crate) fn pending_extend(&self) -> usize {
+        // this field is not used by anything else
+        self.first.size
+    }
+
     pub(crate) unsafe fn try_extend(&mut self, by: usize) -> Result<(), ExtendError> {
-        if by < Self::min_size() {
-            return Err(ExtendError::SizeTooSmall);
+        let top = self.top;
+        if top.is_null() {
+            // this is the only case where I don't think will need to make/init a new heap...
+            // since we can't do that from here, maybe we just add this to the
+            // documentation to not call .extend() on uninitialized Heaps
+            unreachable!();
         }
-        let layout = Layout::from_size_align(by, 1).unwrap();
-        let aligned = Self::align_layout(layout);
-        if aligned != layout {
-            return Err(ExtendError::OddSize);
+
+        // join this extend request with any pending (but not yet acted on) extensions
+        let by = self.pending_extend() + by;
+
+        let dead_space = top.align_offset(align_of::<Hole>());
+        let minimum_extend = dead_space + Self::min_size();
+
+        if by < minimum_extend {
+            self.first.size = by;
+
+            #[cfg(test)]
+            println!("pending extend size: {} bytes", self.pending_extend());
+
+            return Ok(());
         }
 
-        let top = self.top;
-        if top.is_null() {
-            return Err(ExtendError::EmptyHeap);
+        #[cfg(test)]
+        if dead_space > 0 {
+            println!("dead space created during extend: {} bytes", dead_space);
         }
 
         let aligned_top = align_up(top, align_of::<Hole>());
-        if aligned_top != top {
-            return Err(ExtendError::OddHeapSize);
-        }
+        let layout = Layout::from_size_align(by - dead_space, 1).unwrap();
 
-        self.deallocate(NonNull::new_unchecked(top as *mut u8), layout);
+        self.deallocate(NonNull::new_unchecked(aligned_top as *mut u8), layout);
         self.top = top.add(by);
 
+        // reset pending resizes
+        self.first.size = 0;
+
         Ok(())
     }
 }

src/test.rs

@@ -504,35 +504,36 @@ fn extend_fragmented_heap() {
 fn small_heap_extension() {
     // define an array of `u64` instead of `u8` for alignment
     static mut HEAP: [u64; 5] = [0; 5];
-    let result = unsafe {
+    unsafe {
         let mut heap = Heap::new(HEAP.as_mut_ptr().cast(), 32);
-        heap.try_extend(1)
-    };
-    assert_eq!(result, Err(ExtendError::SizeTooSmall))
+        heap.extend(1);
+        assert_eq!(1, heap.holes.pending_extend());
+    }
 }
 
 /// Ensures that `Heap::extend` fails for sizes that are not a multiple of the hole size.
 #[test]
 fn oddly_sized_heap_extension() {
     // define an array of `u64` instead of `u8` for alignment
     static mut HEAP: [u64; 5] = [0; 5];
-    let result = unsafe {
+    unsafe {
         let mut heap = Heap::new(HEAP.as_mut_ptr().cast(), 16);
-        heap.try_extend(17)
-    };
-    assert_eq!(result, Err(ExtendError::OddSize))
+        heap.extend(17);
+        assert_eq!(0, heap.holes.pending_extend());
+        assert_eq!(16 + 17, heap.size());
+    }
 }
 
 /// Ensures that heap extension fails when trying to extend an empty heap.
 ///
 /// Empty heaps always have a start pointer of 0.
 #[test]
+#[should_panic]
 fn extend_empty() {
-    let result = unsafe {
+    unsafe {
         let mut heap = Heap::empty();
-        heap.try_extend(16)
-    };
-    assert_eq!(result, Err(ExtendError::EmptyHeap))
+        heap.extend(16);
+    }
 }
 
 /// Ensures that heap extension fails when trying to extend an oddly-sized heap.
@@ -543,9 +544,12 @@ fn extend_empty() {
 fn extend_odd_size() {
     // define an array of `u64` instead of `u8` for alignment
     static mut HEAP: [u64; 5] = [0; 5];
-    let result = unsafe {
+    unsafe {
         let mut heap = Heap::new(HEAP.as_mut_ptr().cast(), 17);
-        heap.try_extend(16)
-    };
-    assert_eq!(result, Err(ExtendError::OddHeapSize))
+        heap.extend(16);
+        assert_eq!(16, heap.holes.pending_extend());
+        heap.extend(15);
+        assert_eq!(0, heap.holes.pending_extend());
+        assert_eq!(17 + 16 + 15, heap.size());
+    }
 }