From 12a2a95d750aa5ae309ecd8374ba9f6348087495 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Tue, 20 Aug 2019 17:15:57 -0400 Subject: [PATCH 1/4] maintain a free slot index set in TLS InstanceImpl to allocate in O(1) time Signed-off-by: Xin Zhuang --- .../common/thread_local/thread_local_impl.cc | 20 ++++++++++--------- .../common/thread_local/thread_local_impl.h | 5 +++++ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index 4e8c32fed776..d9b8469bd06d 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -24,16 +24,16 @@ SlotPtr InstanceImpl::allocateSlot() { ASSERT(std::this_thread::get_id() == main_thread_id_); ASSERT(!shutdown_); - for (uint64_t i = 0; i < slots_.size(); i++) { - if (slots_[i] == nullptr) { - std::unique_ptr slot(new SlotImpl(*this, i)); - slots_[i] = slot.get(); - return slot; - } + if (free_slot_indexes_.empty()) { + std::unique_ptr slot(new SlotImpl(*this, slots_.size())); + slots_.push_back(slot.get()); + return slot; } - - std::unique_ptr slot(new SlotImpl(*this, slots_.size())); - slots_.push_back(slot.get()); + uint32_t idx = *free_slot_indexes_.begin(); + free_slot_indexes_.erase(idx); + ASSERT(idx < slots_.size()); + std::unique_ptr slot(new SlotImpl(*this, idx)); + slots_[idx] = slot.get(); return slot; } @@ -73,6 +73,8 @@ void InstanceImpl::removeSlot(SlotImpl& slot) { const uint64_t index = slot.index_; slots_[index] = nullptr; + auto ret = free_slot_indexes_.emplace(index); + ASSERT(ret.second, fmt::format("slot index {} already in free slot set!", index)); runOnAllThreads([index]() -> void { // This runs on each thread and clears the slot, making it available for a new allocations. // This is safe even if a new allocation comes in, because everything happens with post() and diff --git a/source/common/thread_local/thread_local_impl.h b/source/common/thread_local/thread_local_impl.h index 3e8e39c8fa89..73b5433134d5 100644 --- a/source/common/thread_local/thread_local_impl.h +++ b/source/common/thread_local/thread_local_impl.h @@ -9,6 +9,8 @@ #include "common/common/logger.h" +#include "absl/container/flat_hash_set.h" + namespace Envoy { namespace ThreadLocal { @@ -57,6 +59,9 @@ class InstanceImpl : Logger::Loggable, public Instance { static thread_local ThreadLocalData thread_local_data_; std::vector slots_; + // A set of index of freed slots. + absl::flat_hash_set free_slot_indexes_; + std::list> registered_threads_; std::thread::id main_thread_id_; Event::Dispatcher* main_thread_dispatcher_{}; From 5708a82ce668f131c29c4cf36b87465b802e121c Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Wed, 21 Aug 2019 13:33:14 -0400 Subject: [PATCH 2/4] change from hash set to list Signed-off-by: Xin Zhuang --- source/common/thread_local/thread_local_impl.cc | 11 +++++++---- source/common/thread_local/thread_local_impl.h | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index d9b8469bd06d..b7c632761707 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -1,5 +1,6 @@ #include "common/thread_local/thread_local_impl.h" +#include #include #include #include @@ -29,8 +30,8 @@ SlotPtr InstanceImpl::allocateSlot() { slots_.push_back(slot.get()); return slot; } - uint32_t idx = *free_slot_indexes_.begin(); - free_slot_indexes_.erase(idx); + uint32_t idx = free_slot_indexes_.front(); + free_slot_indexes_.pop_front(); ASSERT(idx < slots_.size()); std::unique_ptr slot(new SlotImpl(*this, idx)); slots_[idx] = slot.get(); @@ -73,8 +74,10 @@ void InstanceImpl::removeSlot(SlotImpl& slot) { const uint64_t index = slot.index_; slots_[index] = nullptr; - auto ret = free_slot_indexes_.emplace(index); - ASSERT(ret.second, fmt::format("slot index {} already in free slot set!", index)); + ASSERT(std::find(free_slot_indexes_.begin(), free_slot_indexes_.end(), index) == + free_slot_indexes_.end(), + fmt::format("slot index {} already in free slot set!", index)); + free_slot_indexes_.push_back(index); runOnAllThreads([index]() -> void { // This runs on each thread and clears the slot, making it available for a new allocations. // This is safe even if a new allocation comes in, because everything happens with post() and diff --git a/source/common/thread_local/thread_local_impl.h b/source/common/thread_local/thread_local_impl.h index 73b5433134d5..a57ecc21ccfb 100644 --- a/source/common/thread_local/thread_local_impl.h +++ b/source/common/thread_local/thread_local_impl.h @@ -59,8 +59,8 @@ class InstanceImpl : Logger::Loggable, public Instance { static thread_local ThreadLocalData thread_local_data_; std::vector slots_; - // A set of index of freed slots. - absl::flat_hash_set free_slot_indexes_; + // A list of index of freed slots. + std::list free_slot_indexes_; std::list> registered_threads_; std::thread::id main_thread_id_; From b63620281b70067486345b54d8a311044a0bb44f Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Wed, 21 Aug 2019 14:22:47 -0400 Subject: [PATCH 3/4] fix nit Signed-off-by: Xin Zhuang --- source/common/thread_local/thread_local_impl.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/common/thread_local/thread_local_impl.h b/source/common/thread_local/thread_local_impl.h index a57ecc21ccfb..39a0f12a3e4f 100644 --- a/source/common/thread_local/thread_local_impl.h +++ b/source/common/thread_local/thread_local_impl.h @@ -9,8 +9,6 @@ #include "common/common/logger.h" -#include "absl/container/flat_hash_set.h" - namespace Envoy { namespace ThreadLocal { From d1d43be82596e5fd03ee261a65e4c9c093ae74cc Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Wed, 21 Aug 2019 14:24:37 -0400 Subject: [PATCH 4/4] fix nit, one at a time Signed-off-by: Xin Zhuang --- source/common/thread_local/thread_local_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index b7c632761707..9781db07797b 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -30,7 +30,7 @@ SlotPtr InstanceImpl::allocateSlot() { slots_.push_back(slot.get()); return slot; } - uint32_t idx = free_slot_indexes_.front(); + const uint32_t idx = free_slot_indexes_.front(); free_slot_indexes_.pop_front(); ASSERT(idx < slots_.size()); std::unique_ptr slot(new SlotImpl(*this, idx));