From 9f3a7179988fe4595f95b236aef0fb17fefecb21 Mon Sep 17 00:00:00 2001 From: Greg Greenway Date: Tue, 26 Jan 2021 10:30:56 -0800 Subject: [PATCH] optimize: only lookup thread_local once per reservation Signed-off-by: Greg Greenway --- source/common/buffer/buffer_impl.cc | 2 +- source/common/buffer/buffer_impl.h | 72 +++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/source/common/buffer/buffer_impl.cc b/source/common/buffer/buffer_impl.cc index 8231535ca31b..3db10686ea7b 100644 --- a/source/common/buffer/buffer_impl.cc +++ b/source/common/buffer/buffer_impl.cc @@ -343,7 +343,7 @@ Reservation OwnedImpl::reserveWithMaxLength(uint64_t max_length) { break; } - Slice slice(size); + Slice slice(size, slices_owner->free_list_); reservation_slices.push_back(slice.reserve(size)); slices_owner->owned_slices_.emplace_back(std::move(slice)); ASSERT(slice.dataSize() == 0); diff --git a/source/common/buffer/buffer_impl.h b/source/common/buffer/buffer_impl.h index fb7859bbaab5..92d8f540e457 100644 --- a/source/common/buffer/buffer_impl.h +++ b/source/common/buffer/buffer_impl.h @@ -35,6 +35,15 @@ class Slice { using Reservation = RawSlice; using StoragePtr = std::unique_ptr; + static constexpr uint32_t free_list_max_ = Buffer::Reservation::MAX_SLICES_; + using FreeListType = absl::InlinedVector; + class FreeListReference { + private: + FreeListReference(FreeListType& free_list) : free_list_(free_list) {} + FreeListType& free_list_; + friend class Slice; + }; + /** * Create an empty Slice with 0 capacity. */ @@ -45,9 +54,9 @@ class Slice { * @param min_capacity number of bytes of space the slice should have. Actual capacity is rounded * up to the next multiple of 4kb. */ - Slice(uint64_t min_capacity) - : capacity_(sliceSize(min_capacity)), storage_(newStorage(capacity_)), base_(storage_.get()), - data_(0), reservable_(0) {} + Slice(uint64_t min_capacity, absl::optional free_list = absl::nullopt) + : capacity_(sliceSize(min_capacity)), storage_(newStorage(capacity_, free_list)), + base_(storage_.get()), data_(0), reservable_(0) {} /** * Create an immutable Slice that refers to an external buffer fragment. @@ -100,6 +109,11 @@ class Slice { freeStorage(std::move(storage_), capacity_); } + void freeStorage(FreeListReference free_list) { + callAndClearDrainTrackers(); + freeStorage(std::move(storage_), capacity_, free_list); + } + /** * @return true if the data in the slice is mutable */ @@ -292,6 +306,8 @@ class Slice { static constexpr uint32_t default_slice_size_ = 16384; + static FreeListReference freeList() { return FreeListReference(free_list_); } + protected: /** * Compute a slice size big enough to hold a specified amount of data. @@ -304,40 +320,48 @@ class Slice { return num_pages * PageSize; } - static StoragePtr newStorage(uint64_t capacity) { + static StoragePtr newStorage(uint64_t capacity, absl::optional free_list_opt) { ASSERT(sliceSize(default_slice_size_) == default_slice_size_, "default_slice_size_ incompatible with sliceSize()"); ASSERT(sliceSize(capacity) == capacity, "newStorage should only be called on values returned from sliceSize()"); + ASSERT(!free_list_opt.has_value() || &free_list_opt->free_list_ == &free_list_); StoragePtr storage; - if (capacity == default_slice_size_ && !free_list_.empty()) { - storage = std::move(free_list_.back()); - ASSERT(storage != nullptr); - ASSERT(free_list_.back() == nullptr); - free_list_.pop_back(); - } else { - storage.reset(new uint8_t[capacity]); + if (capacity == default_slice_size_ && free_list_opt.has_value()) { + FreeListType& free_list = free_list_opt->free_list_; + if (!free_list.empty()) { + storage = std::move(free_list.back()); + ASSERT(storage != nullptr); + ASSERT(free_list.back() == nullptr); + free_list.pop_back(); + return storage; + } } + storage.reset(new uint8_t[capacity]); return storage; } - static void freeStorage(StoragePtr storage, uint64_t capacity) { + static void freeStorage(StoragePtr storage, uint64_t capacity, + absl::optional free_list_opt = absl::nullopt) { if (storage == nullptr) { return; } - if (capacity == default_slice_size_ && free_list_.size() < free_list_max_) { - free_list_.emplace_back(std::move(storage)); - ASSERT(storage == nullptr); - } else { - storage.reset(); + if (capacity == default_slice_size_ && free_list_opt.has_value()) { + FreeListType& free_list = free_list_opt->free_list_; + if (free_list.size() < free_list_max_) { + free_list.emplace_back(std::move(storage)); + ASSERT(storage == nullptr); + return; + } } + + storage.reset(); } - static constexpr uint32_t free_list_max_ = Buffer::Reservation::MAX_SLICES_; - static thread_local absl::InlinedVector free_list_; + static thread_local FreeListType free_list_; /** Length of the byte array that base_ points to. This is also the offset in bytes from the start * of the slice to the end of the Reservable section. */ @@ -711,8 +735,18 @@ class OwnedImpl : public LibEventInstance { }; struct OwnedImplReservationSlicesOwnerMultiple : public OwnedImplReservationSlicesOwner { + // Optimization: get the thread_local freeList() once per Reservation, outside the loop. + OwnedImplReservationSlicesOwnerMultiple() : free_list_(Slice::freeList()) {} + + ~OwnedImplReservationSlicesOwnerMultiple() { + while (!owned_slices_.empty()) { + owned_slices_.back().freeStorage(free_list_); + owned_slices_.pop_back(); + } + } absl::Span ownedSlices() override { return absl::MakeSpan(owned_slices_); } + Slice::FreeListReference free_list_; absl::InlinedVector owned_slices_; };