Skip to content

Commit 12a8c0e

Browse files
committed
revert upper bound size hints
Signed-off-by: Connor Tsui <[email protected]>
1 parent 9aa382e commit 12a8c0e

File tree

6 files changed

+36
-35
lines changed

6 files changed

+36
-35
lines changed

vortex-buffer/src/bit/buf_mut.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,8 @@ impl BitBufferMut {
205205

206206
/// Set the bit at `index` to `true` without checking bounds.
207207
///
208+
/// Note: Do not call this in a tight loop. Prefer to use [`set_bit_unchecked`].
209+
///
208210
/// # Safety
209211
///
210212
/// The caller must ensure that `index` does not exceed the largest bit index in the backing buffer.
@@ -229,6 +231,7 @@ impl BitBufferMut {
229231
///
230232
/// - `new_len` must be less than or equal to [`capacity()`](Self::capacity)
231233
/// - The elements at `old_len..new_len` must be initialized
234+
#[inline(always)]
232235
pub unsafe fn set_len(&mut self, new_len: usize) {
233236
debug_assert!(
234237
new_len <= self.capacity(),
@@ -511,14 +514,20 @@ impl FromIterator<bool> for BitBufferMut {
511514
fn from_iter<T: IntoIterator<Item = bool>>(iter: T) -> Self {
512515
let mut iter = iter.into_iter();
513516

514-
// Note that these hints might be incorrect.
515-
let (lower_bound, upper_bound_opt) = iter.size_hint();
516-
let capacity = upper_bound_opt.unwrap_or(lower_bound);
517+
// Since we do not know the length of the iterator, we can only guess how much memory we
518+
// need to reserve. Note that these hints may be inaccurate.
519+
let (lower_bound, _) = iter.size_hint();
517520

518-
let mut buf = BitBufferMut::new_unset(capacity);
521+
// We choose not to use the optional upper bound size hint to match the standard library.
522+
523+
// Initialize all bits to 0 with the given length. By doing this, we only need to set bits
524+
// that are true (and this is faster from benchmarks).
525+
let mut buf = BitBufferMut::new_unset(lower_bound);
526+
assert_eq!(buf.offset, 0);
519527

520528
// Directly write within our known capacity.
521-
for i in 0..capacity {
529+
let ptr = buf.buffer.as_mut_ptr();
530+
for i in 0..lower_bound {
522531
let Some(v) = iter.next() else {
523532
// SAFETY: We are definitely under the capacity and all values are already
524533
// initialized from `new_unset`.
@@ -528,7 +537,7 @@ impl FromIterator<bool> for BitBufferMut {
528537

529538
if v {
530539
// SAFETY: We have ensured that we are within the capacity.
531-
unsafe { buf.set_unchecked(i) }
540+
unsafe { set_bit_unchecked(ptr, i) }
532541
}
533542
}
534543

vortex-buffer/src/buffer.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,10 @@ impl<T> Buffer<T> {
193193
/// Create a buffer with values from the TrustedLen iterator.
194194
/// Should be preferred over `from_iter` when the iterator is known to be `TrustedLen`.
195195
pub fn from_trusted_len_iter<I: TrustedLen<Item = T>>(iter: I) -> Self {
196-
let (_, high) = iter.size_hint();
197-
let mut buffer =
198-
BufferMut::with_capacity(high.vortex_expect("TrustedLen iterator has no upper bound"));
196+
let (_, upper_bound) = iter.size_hint();
197+
let mut buffer = BufferMut::with_capacity(
198+
upper_bound.vortex_expect("TrustedLen iterator has no upper bound"),
199+
);
199200
buffer.extend_trusted(iter);
200201
buffer.freeze()
201202
}

vortex-buffer/src/buffer_mut.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -484,14 +484,11 @@ impl<T> BufferMut<T> {
484484
fn extend_iter(&mut self, mut iter: impl Iterator<Item = T>) {
485485
// Since we do not know the length of the iterator, we can only guess how much memory we
486486
// need to reserve. Note that these hints may be inaccurate.
487-
let (lower_bound, upper_bound_opt) = iter.size_hint();
488-
489-
// In the case that the upper bound is adversarial, we put a hard limit on the amount of
490-
// memory we reserve (and the OS should handle the rest with zero pages).
491-
let reserve_amount = upper_bound_opt
492-
.unwrap_or(lower_bound)
493-
.min(i32::MAX as usize);
494-
self.reserve(reserve_amount);
487+
let (lower_bound, _) = iter.size_hint();
488+
489+
// We choose not to use the optional upper bound size hint to match the standard library.
490+
491+
self.reserve(lower_bound);
495492

496493
let unwritten = self.capacity() - self.len();
497494

vortex-buffer/src/trusted_len.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,17 @@ pub trait TrustedLenExt: Iterator + Sized {
7272
///
7373
/// The caller must guarantee that the iterator does indeed have an exact length.
7474
unsafe fn trusted_len(self) -> TrustedLenAdapter<Self> {
75-
let (lower, maybe_upper) = self.size_hint();
76-
if let Some(upper) = maybe_upper {
75+
let (lower_bound, upper_bound_opt) = self.size_hint();
76+
if let Some(upper_bound) = upper_bound_opt {
7777
assert_eq!(
78-
lower, upper,
78+
lower_bound, upper_bound,
7979
"TrustedLenExt: iterator size hints must match if upper bound is given"
8080
);
8181
}
8282

8383
TrustedLenAdapter {
8484
inner: self,
85-
len: lower,
85+
len: lower_bound,
8686
#[cfg(debug_assertions)]
8787
count: 0,
8888
}

vortex-vector/src/bool/from_iter.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,12 @@ impl FromIterator<Option<bool>> for BoolVectorMut {
2828
let iter = iter.into_iter();
2929
// Since we do not know the length of the iterator, we can only guess how much memory we
3030
// need to reserve. Note that these hints may be inaccurate.
31-
let (lower_bound, upper_bound_opt) = iter.size_hint();
31+
let (lower_bound, _) = iter.size_hint();
3232

33-
// In the case that the upper bound is adversarial, we put a hard limit on the amount of
34-
// memory we reserve (and the OS should handle the rest with zero pages).
35-
let reserve_amount = upper_bound_opt.unwrap_or(lower_bound);
33+
// We choose not to use the optional upper bound size hint to match the standard library.
3634

37-
let mut bits = BitBufferMut::with_capacity(reserve_amount);
38-
let mut validity = MaskMut::with_capacity(reserve_amount);
35+
let mut bits = BitBufferMut::with_capacity(lower_bound);
36+
let mut validity = MaskMut::with_capacity(lower_bound);
3937

4038
for opt_val in iter {
4139
match opt_val {

vortex-vector/src/primitive/from_iter.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,12 @@ impl<T: NativePType> FromIterator<Option<T>> for PVectorMut<T> {
2929
let iter = iter.into_iter();
3030
// Since we do not know the length of the iterator, we can only guess how much memory we
3131
// need to reserve. Note that these hints may be inaccurate.
32-
let (lower_bound, upper_bound_opt) = iter.size_hint();
32+
let (lower_bound, _) = iter.size_hint();
3333

34-
// In the case that the upper bound is adversarial, we put a hard limit on the amount of
35-
// memory we reserve (and the OS should handle the rest with zero pages).
36-
let reserve_amount = upper_bound_opt
37-
.unwrap_or(lower_bound)
38-
.min(i32::MAX as usize);
34+
// We choose not to use the optional upper bound size hint to match the standard library.
3935

40-
let mut elements = BufferMut::with_capacity(reserve_amount);
41-
let mut validity = MaskMut::with_capacity(reserve_amount);
36+
let mut elements = BufferMut::with_capacity(lower_bound);
37+
let mut validity = MaskMut::with_capacity(lower_bound);
4238

4339
for opt_val in iter {
4440
match opt_val {

0 commit comments

Comments
 (0)