From a8e62dfc7a7f6c7334402d9f844f1d7434c9a5c0 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 27 Apr 2018 10:51:09 -0700 Subject: [PATCH 1/5] Bump version to v0.4.7 --- CHANGELOG.md | 6 ++++++ Cargo.toml | 2 +- src/lib.rs | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67b41f7..73f99ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# 0.4.7 (April 27, 2018) + +* Make `Buf` and `BufMut` usable as trait objects (#186). +* impl BorrowMut for BytesMut (#185). +* Improve accessor performance (#195). + # 0.4.6 (Janary 8, 2018) * Implement FromIterator for Bytes/BytesMut (#148). diff --git a/Cargo.toml b/Cargo.toml index 553e558..a33dfba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "bytes" -version = "0.4.6" # don't forget to update html_root_url +version = "0.4.7" # don't forget to update html_root_url license = "MIT/Apache-2.0" authors = ["Carl Lerche "] description = "Types and traits for working with bytes" diff --git a/src/lib.rs b/src/lib.rs index 68cdbee..e912b7d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -69,7 +69,7 @@ //! and `BufMut` are infallible. #![deny(warnings, missing_docs, missing_debug_implementations)] -#![doc(html_root_url = "https://docs.rs/bytes/0.4.6")] +#![doc(html_root_url = "https://docs.rs/bytes/0.4.7")] extern crate byteorder; extern crate iovec; From 13aaeee3457acaef086263ee97606df5dc975b92 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 11 May 2018 08:45:04 -0700 Subject: [PATCH 2/5] Fix panic in FromIterator for BytesMut --- src/bytes.rs | 1 + tests/test_bytes.rs | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/bytes.rs b/src/bytes.rs index 3228474..9aa24af 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -869,6 +869,7 @@ impl FromIterator for BytesMut { let mut out = BytesMut::with_capacity(maybe_max.unwrap_or(min)); for i in iter { + out.reserve(1); out.put(i); } diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index 15c3157..c940f01 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -674,3 +674,24 @@ fn unsplit_two_split_offs() { buf.unsplit(buf2); assert_eq!(b"aaaabbbbccccdddd", &buf[..]); } + +#[test] +fn from_iter_no_size_hint() { + use std::iter; + + let mut expect = vec![]; + + let actual: Bytes = iter::repeat(b'x') + .scan(100, |cnt, item| { + if *cnt >= 1 { + *cnt -= 1; + expect.push(item); + Some(item) + } else { + None + } + }) + .collect(); + + assert_eq!(&actual[..], &expect[..]); +} From 2491e5102de73d65d0d5bc6971f51664bf0cd825 Mon Sep 17 00:00:00 2001 From: Noah Zentzis Date: Thu, 24 May 2018 14:50:31 -0700 Subject: [PATCH 3/5] Recycle space when reserving from Vec-backed Bytes (#197) * Recycle space when reserving from Vec-backed Bytes BytesMut::reserve, when called on a BytesMut instance which is backed by a non-shared Vec, would previously just delegate to Vec::reserve regardless of the current location in the buffer. If the Bytes is actually the trailing component of a larger Vec, then the unused space won't be recycled. In applications which continually move the pointer forward to consume data as it comes in, this can cause the underlying buffer to get extremely large. This commit checks whether there's extra space at the start of the backing Vec in this case, and reuses the unused space if possible instead of allocating. * Avoid excessive copying when reusing Vec space Only reuse space in a Vec-backed Bytes when doing so would gain back more than half of the current capacity. This avoids excessive copy operations when a large buffer is almost (but not completely) full. --- src/bytes.rs | 42 ++++++++++++++++++++++++++++++++---------- tests/test_bytes.rs | 15 +++++++++++++++ 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/bytes.rs b/src/bytes.rs index 9aa24af..1022f00 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -2165,20 +2165,42 @@ impl Inner { } if kind == KIND_VEC { - // Currently backed by a vector, so just use `Vector::reserve`. + // If there's enough free space before the start of the buffer, then + // just copy the data backwards and reuse the already-allocated + // space. + // + // Otherwise, since backed by a vector, use `Vec::reserve` unsafe { - let (off, _) = self.uncoordinated_get_vec_pos(); - let mut v = rebuild_vec(self.ptr, self.len, self.cap, off); - v.reserve(additional); + let (off, prev) = self.uncoordinated_get_vec_pos(); - // Update the info - self.ptr = v.as_mut_ptr().offset(off as isize); - self.len = v.len() - off; - self.cap = v.capacity() - off; + // Only reuse space if we stand to gain at least capacity/2 + // bytes of space back + if off >= additional && off >= (self.cap / 2) { + // There's space - reuse it + // + // Just move the pointer back to the start after copying + // data back. + let base_ptr = self.ptr.offset(-(off as isize)); + ptr::copy(self.ptr, base_ptr, self.len); + self.ptr = base_ptr; + self.uncoordinated_set_vec_pos(0, prev); - // Drop the vec reference - mem::forget(v); + // Length stays constant, but since we moved backwards we + // can gain capacity back. + self.cap += off; + } else { + // No space - allocate more + let mut v = rebuild_vec(self.ptr, self.len, self.cap, off); + v.reserve(additional); + // Update the info + self.ptr = v.as_mut_ptr().offset(off as isize); + self.len = v.len() - off; + self.cap = v.capacity() - off; + + // Drop the vec reference + mem::forget(v); + } return; } } diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index c940f01..03da9dd 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -378,6 +378,21 @@ fn reserve_max_original_capacity_value() { assert_eq!(bytes.capacity(), 64 * 1024); } +// Without either looking at the internals of the BytesMut or doing weird stuff +// with the memory allocator, there's no good way to automatically verify from +// within the program that this actually recycles memory. Instead, just exercise +// the code path to ensure that the results are correct. +#[test] +fn reserve_vec_recycling() { + let mut bytes = BytesMut::from(Vec::with_capacity(16)); + assert_eq!(bytes.capacity(), 16); + bytes.put("0123456789012345"); + bytes.advance(10); + assert_eq!(bytes.capacity(), 6); + bytes.reserve(8); + assert_eq!(bytes.capacity(), 16); +} + #[test] fn reserve_in_arc_unique_does_not_overallocate() { let mut bytes = BytesMut::with_capacity(1000); From 40ebcd1cfe7f5b271e508ef678b63619e9b6de21 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 25 May 2018 12:40:37 -0700 Subject: [PATCH 4/5] Use sanitizers in CI (#204) --- .travis.yml | 18 ++++++++++++++++++ ci/tsan | 17 +++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 ci/tsan diff --git a/.travis.yml b/.travis.yml index 8600e8f..dbc744e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,6 +42,24 @@ matrix: - rustup target add wasm32-unknown-unknown - cargo build --target=wasm32-unknown-unknown + # Sanitizers + - rust: nightly + os: linux + script: + - | + set -e + + export RUST_TEST_THREADS=1 + export ASAN_OPTIONS="detect_odr_violation=0 detect_leaks=0" + export TSAN_OPTIONS="suppressions=`pwd`/ci/tsan" + + # Run address sanitizer + RUSTFLAGS="-Z sanitizer=address" \ + cargo test --tests --target x86_64-unknown-linux-gnu + + # Run thread sanitizer + RUSTFLAGS="-Z sanitizer=thread" \ + cargo test --tests --target x86_64-unknown-linux-gnu before_install: set -e diff --git a/ci/tsan b/ci/tsan new file mode 100644 index 0000000..34f76ab --- /dev/null +++ b/ci/tsan @@ -0,0 +1,17 @@ +# TSAN suppressions file for `bytes` + +# TSAN does not understand fences and `Arc::drop` is implemented using a fence. +# This causes many false positives. +race:Arc*drop +race:arc*Weak*drop + +# `std` mpsc is not used in any Bytes code base. This race is triggered by some +# rust runtime logic. +race:std*mpsc_queue + +# Probably more fences in std. +race:__call_tls_dtors + +# `is_inline` is explicitly called concurrently without synchronization. The +# safety explanation can be found in a comment. +race:Inner::is_inline From 3c717b9e31baa0e0ac9018fa9b662ba14f20e3e1 Mon Sep 17 00:00:00 2001 From: Luke Horsley Date: Fri, 25 May 2018 21:54:32 +0100 Subject: [PATCH 5/5] Added a resize function for BytesMut (#203) --- src/bytes.rs | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/bytes.rs b/src/bytes.rs index 1022f00..cbcf58b 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -1276,6 +1276,32 @@ impl BytesMut { self.truncate(0); } + /// Resizes the buffer so that `len` is equal to `new_len`. + /// + /// If `new_len` is greater than `len`, the buffer is extended by the + /// difference with each additional byte set to `value`. If `new_len` is + /// less than `len`, the buffer is simply truncated. + /// + /// # Examples + /// + /// ``` + /// use bytes::BytesMut; + /// + /// let mut buf = BytesMut::new(); + /// + /// buf.resize(3, 0x1); + /// assert_eq!(&buf[..], &[0x1, 0x1, 0x1]); + /// + /// buf.resize(2, 0x2); + /// assert_eq!(&buf[..], &[0x1, 0x1]); + /// + /// buf.resize(4, 0x3); + /// assert_eq!(&buf[..], &[0x1, 0x1, 0x3, 0x3]); + /// ``` + pub fn resize(&mut self, new_len: usize, value: u8) { + self.inner.resize(new_len, value); + } + /// Sets the length of the buffer. /// /// This will explicitly set the size of the buffer without actually @@ -1890,6 +1916,21 @@ impl Inner { } } + fn resize(&mut self, new_len: usize, value: u8) { + let len = self.len(); + if new_len > len { + let additional = new_len - len; + self.reserve(additional); + unsafe { + let dst = self.as_raw()[len..].as_mut_ptr(); + ptr::write_bytes(dst, value, additional); + self.set_len(new_len); + } + } else { + self.truncate(new_len); + } + } + unsafe fn set_start(&mut self, start: usize) { // Setting the start to 0 is a no-op, so return early if this is the // case.