Skip to content

Commit f200f3a

Browse files
committed
perf(buffer): exclude alignment slack from growth
Signed-off-by: Nicholas Gates <nick@nickgates.com>
1 parent ef98cb9 commit f200f3a

2 files changed

Lines changed: 81 additions & 5 deletions

File tree

vortex-buffer/src/buffer.rs

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ pub struct Buffer<T> {
3535
pub(crate) length: usize,
3636
pub(crate) alignment: Alignment,
3737
pub(crate) physical_alignment: Alignment,
38+
// One physical-alignment block is reserved outside the logical capacity.
39+
pub(crate) overallocated: bool,
3840
pub(crate) backing: Arc<BufferBacking>,
3941
}
4042

@@ -67,6 +69,7 @@ impl<T> Default for Buffer<T> {
6769
length: 0,
6870
alignment: Alignment::of::<T>(),
6971
physical_alignment: Alignment::MAX,
72+
overallocated: false,
7073
backing: EMPTY_BACKING.clone(),
7174
}
7275
}
@@ -109,6 +112,7 @@ impl<T> Buffer<T> {
109112
length: usize,
110113
alignment: Alignment,
111114
physical_alignment: Alignment,
115+
overallocated: bool,
112116
) -> Self {
113117
// SAFETY: BufferMut keeps offset within allocation, including for empty buffers.
114118
let ptr = unsafe { allocation.ptr().add(offset).cast() };
@@ -117,6 +121,7 @@ impl<T> Buffer<T> {
117121
length,
118122
alignment,
119123
physical_alignment,
124+
overallocated,
120125
backing: Arc::new(BufferBacking::Owned(allocation)),
121126
}
122127
}
@@ -134,6 +139,7 @@ impl<T> Buffer<T> {
134139
length,
135140
alignment,
136141
physical_alignment: alignment,
142+
overallocated: false,
137143
backing: Arc::new(BufferBacking::External { _owner: owner }),
138144
}
139145
}
@@ -229,6 +235,7 @@ impl<T> Buffer<T> {
229235
length: 0,
230236
alignment,
231237
physical_alignment: Alignment::MAX,
238+
overallocated: false,
232239
backing: EMPTY_BACKING.clone(),
233240
}
234241
}
@@ -289,6 +296,7 @@ impl<T> Buffer<T> {
289296
length: buffer.length / size_of::<T>(),
290297
alignment,
291298
physical_alignment: buffer.physical_alignment,
299+
overallocated: buffer.overallocated,
292300
backing: buffer.backing,
293301
}
294302
}
@@ -494,6 +502,7 @@ impl<T> Buffer<T> {
494502
length: end - begin,
495503
alignment,
496504
physical_alignment: self.physical_alignment,
505+
overallocated: self.overallocated,
497506
backing: Arc::clone(&self.backing),
498507
}
499508
}
@@ -548,6 +557,7 @@ impl<T> Buffer<T> {
548557
length: subset.len(),
549558
alignment,
550559
physical_alignment: self.physical_alignment,
560+
overallocated: self.overallocated,
551561
backing: Arc::clone(&self.backing),
552562
}
553563
}
@@ -568,6 +578,7 @@ impl<T> Buffer<T> {
568578
length: self.length * size_of::<T>(),
569579
alignment: self.alignment,
570580
physical_alignment: self.physical_alignment,
581+
overallocated: self.overallocated,
571582
backing: self.backing,
572583
}
573584
}
@@ -579,17 +590,25 @@ impl<T> Buffer<T> {
579590
length,
580591
alignment,
581592
physical_alignment,
593+
overallocated,
582594
backing,
583595
} = self;
584596
match Arc::try_unwrap(backing) {
585597
Ok(BufferBacking::Owned(allocation)) => {
586598
let offset = ptr.addr().get() - allocation.ptr().addr().get();
599+
let overallocated = overallocated
600+
&& offset
601+
== allocation
602+
.ptr()
603+
.as_ptr()
604+
.align_offset(physical_alignment.as_usize());
587605
Ok(BufferMut {
588606
allocation,
589607
offset,
590608
length,
591609
alignment,
592610
physical_alignment,
611+
overallocated,
593612
_marker: Default::default(),
594613
})
595614
}
@@ -598,13 +617,15 @@ impl<T> Buffer<T> {
598617
length,
599618
alignment,
600619
physical_alignment,
620+
overallocated,
601621
backing: Arc::new(backing),
602622
}),
603623
Err(backing) => Err(Self {
604624
ptr,
605625
length,
606626
alignment,
607627
physical_alignment,
628+
overallocated,
608629
backing,
609630
}),
610631
}
@@ -677,6 +698,7 @@ impl<T> Buffer<T> {
677698
length: self.length,
678699
alignment: self.alignment,
679700
physical_alignment: self.physical_alignment,
701+
overallocated: self.overallocated,
680702
backing: self.backing,
681703
}
682704
}
@@ -782,7 +804,14 @@ where
782804
if std::mem::needs_drop::<T>() {
783805
Self::from_owner(Wrapper(value), alignment)
784806
} else {
785-
Self::from_allocation(Allocation::from_vec(value), 0, length, alignment, alignment)
807+
Self::from_allocation(
808+
Allocation::from_vec(value),
809+
0,
810+
length,
811+
alignment,
812+
alignment,
813+
false,
814+
)
786815
}
787816
}
788817
}
@@ -1048,6 +1077,34 @@ mod test {
10481077
assert_eq!(buffer.allocation.alignment(), align_of::<u32>());
10491078
}
10501079

1080+
#[test]
1081+
fn from_u8_vec_preserves_capacity() {
1082+
let mut vec = Vec::with_capacity(16);
1083+
vec.extend([1u8, 2, 3]);
1084+
1085+
let buffer = Buffer::from(vec);
1086+
let Ok(buffer) = buffer.try_into_mut() else {
1087+
panic!("Vec-backed buffer should be uniquely owned")
1088+
};
1089+
assert_eq!(buffer.capacity(), 16);
1090+
}
1091+
1092+
#[test]
1093+
fn sliced_buffer_into_mut_has_safe_capacity() {
1094+
let mut original = crate::BufferMut::with_capacity(128);
1095+
original.extend(0u32..100);
1096+
let original = original.freeze();
1097+
let sliced = original.slice(64..96);
1098+
drop(original);
1099+
1100+
let Ok(mut sliced) = sliced.try_into_mut() else {
1101+
panic!("uniquely owned slice should become mutable")
1102+
};
1103+
let capacity = sliced.capacity();
1104+
sliced.push_n(0, capacity - sliced.len());
1105+
assert_eq!(sliced.len(), capacity);
1106+
}
1107+
10511108
#[test]
10521109
fn from_vec_preserves_drop_glue() {
10531110
struct DropValue(Arc<AtomicUsize>);

vortex-buffer/src/buffer_mut.rs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ pub struct BufferMut<T> {
3333
pub(crate) length: usize,
3434
pub(crate) alignment: Alignment,
3535
pub(crate) physical_alignment: Alignment,
36+
// One physical-alignment block is reserved outside the logical capacity.
37+
pub(crate) overallocated: bool,
3638
pub(crate) _marker: std::marker::PhantomData<T>,
3739
}
3840

@@ -134,6 +136,7 @@ impl<T> BufferMut<T> {
134136
length: 0,
135137
alignment,
136138
physical_alignment: actual,
139+
overallocated: true,
137140
_marker: Default::default(),
138141
}
139142
}
@@ -222,6 +225,7 @@ impl<T> BufferMut<T> {
222225
length: len,
223226
alignment,
224227
physical_alignment: actual_alignment,
228+
overallocated: true,
225229
_marker: Default::default(),
226230
}
227231
}
@@ -387,7 +391,15 @@ impl<T> BufferMut<T> {
387391
/// Returns the capacity of the buffer.
388392
#[inline]
389393
pub fn capacity(&self) -> usize {
390-
(self.allocation.size() - self.offset) / size_of::<T>()
394+
if self.allocation.size() == 0 {
395+
return 0;
396+
}
397+
398+
if !self.overallocated {
399+
return (self.allocation.size() - self.offset) / size_of::<T>();
400+
}
401+
402+
(self.allocation.size() - self.physical_alignment.as_usize()) / size_of::<T>()
391403
}
392404

393405
/// Returns a raw pointer to the buffer's data.
@@ -461,7 +473,10 @@ impl<T> BufferMut<T> {
461473
.checked_mul(size_of::<T>())
462474
.vortex_expect("buffer capacity overflow");
463475
let physical_alignment = max(self.alignment, self.physical_alignment);
464-
let current_size = self.allocation.size() - self.offset;
476+
let current_size = self
477+
.capacity()
478+
.checked_mul(size_of::<T>())
479+
.vortex_expect("buffer capacity overflow");
465480
let logical_size = required_size
466481
.max(current_size.saturating_mul(2))
467482
.max(physical_alignment.as_usize());
@@ -517,6 +532,7 @@ impl<T> BufferMut<T> {
517532
};
518533
self.offset = new_offset;
519534
self.physical_alignment = physical_alignment;
535+
self.overallocated = true;
520536
}
521537

522538
/// Returns the spare capacity of the buffer as a slice of `MaybeUninit<T>`.
@@ -666,6 +682,7 @@ impl<T> BufferMut<T> {
666682
length: self.length * size_of::<T>(),
667683
alignment: self.alignment,
668684
physical_alignment: self.physical_alignment,
685+
overallocated: self.overallocated,
669686
_marker: Default::default(),
670687
}
671688
}
@@ -678,6 +695,7 @@ impl<T> BufferMut<T> {
678695
self.length,
679696
self.alignment,
680697
self.physical_alignment,
698+
self.overallocated,
681699
)
682700
}
683701

@@ -742,6 +760,7 @@ impl<T> BufferMut<T> {
742760
length: self.length,
743761
alignment: self.alignment,
744762
physical_alignment: self.physical_alignment,
763+
overallocated: self.overallocated,
745764
_marker: std::marker::PhantomData,
746765
}
747766
}
@@ -1093,10 +1112,10 @@ mod test {
10931112

10941113
buffer.push(0);
10951114
let capacity = buffer.capacity();
1096-
assert!(capacity >= alignment.as_usize());
1115+
assert_eq!(capacity, Alignment::DEFAULT_ALIGNMENT.as_usize());
10971116

10981117
buffer.reserve(capacity);
1099-
assert!(buffer.capacity() >= capacity * 2);
1118+
assert_eq!(buffer.capacity(), capacity * 2);
11001119
}
11011120

11021121
#[test]

0 commit comments

Comments
 (0)