From 7110d57b2f0bc1e4dc21988f2e395ba1478eb230 Mon Sep 17 00:00:00 2001 From: Stepan Koltsov Date: Mon, 15 May 2017 21:28:12 +0300 Subject: [PATCH] BytesMut::reserve should not overallocate (#117) Round up to power of 2 is not necessary, because `reserve` already doubles previous capacity in ``` new_cap = cmp::max( cmp::max(v.capacity() << 1, new_cap), original_capacity); ``` which makes `reserve` calls constant in average. Avoiding rounding up prevents `reserve` from wasting space when caller knows exactly what space they need. Patch adds three tests which would fail before this test. The most important is this: ``` #[test] fn reserve_in_arc_unique_does_not_overallocate() { let mut bytes = BytesMut::with_capacity(1000); bytes.take(); // now bytes is Arc and refcount == 1 assert_eq!(1000, bytes.capacity()); bytes.reserve(2001); assert_eq!(2001, bytes.capacity()); } ``` It asserts that when user requests more than double of current capacity, exactly the requested amount of memory is allocated and is not wasted to next power of two. --- src/bytes.rs | 2 +- tests/test_bytes.rs | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/bytes.rs b/src/bytes.rs index 297659d..31dec20 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -2019,7 +2019,7 @@ impl Inner { } // Create a new vector to store the data - let mut v = Vec::with_capacity(new_cap.next_power_of_two()); + let mut v = Vec::with_capacity(new_cap); // Copy the bytes v.extend_from_slice(self.as_ref()); diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index 5c30475..af481a2 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -302,7 +302,7 @@ fn reserve_convert() { let a = bytes.split_to(30); bytes.reserve(128); - assert_eq!(bytes.capacity(), (bytes.len() + 128).next_power_of_two()); + assert!(bytes.capacity() >= bytes.len() + 128); drop(a); } @@ -347,6 +347,42 @@ fn reserve_max_original_capacity_value() { assert_eq!(bytes.capacity(), 64 * 1024); } +#[test] +fn reserve_in_arc_unique_does_not_overallocate() { + let mut bytes = BytesMut::with_capacity(1000); + bytes.take(); + + // now bytes is Arc and refcount == 1 + + assert_eq!(1000, bytes.capacity()); + bytes.reserve(2001); + assert_eq!(2001, bytes.capacity()); +} + +#[test] +fn reserve_in_arc_unique_doubles() { + let mut bytes = BytesMut::with_capacity(1000); + bytes.take(); + + // now bytes is Arc and refcount == 1 + + assert_eq!(1000, bytes.capacity()); + bytes.reserve(1001); + assert_eq!(2000, bytes.capacity()); +} + +#[test] +fn reserve_in_arc_nonunique_does_not_overallocate() { + let mut bytes = BytesMut::with_capacity(1000); + let _copy = bytes.take(); + + // now bytes is Arc and refcount == 2 + + assert_eq!(1000, bytes.capacity()); + bytes.reserve(2001); + assert_eq!(2001, bytes.capacity()); +} + #[test] fn inline_storage() { let mut bytes = BytesMut::with_capacity(inline_cap());