From 1dee433c3440fe58e046de3b2e59dfd7f1a5cb35 Mon Sep 17 00:00:00 2001 From: Nathan Bronson Date: Tue, 13 Oct 2015 14:07:12 -0700 Subject: [PATCH] never destroy LifoSem's wait node pool Summary: Some LifoSem-s survive late into program execution, which means that destroying the IndexedMemPool that holds LifoSem waiter nodes can cause crashes during program shutdown. Reviewed By: @chadparry Differential Revision: D2536597 fb-gh-sync-id: c9b3b73b61f2b8bdcb00d03f4c6d3daf24b267c3 --- folly/LifoSem.h | 28 +++++++++++++++----------- folly/test/LifoSemTests.cpp | 39 +++++++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/folly/LifoSem.h b/folly/LifoSem.h index b0af517c68b..4aa1cc88820 100644 --- a/folly/LifoSem.h +++ b/folly/LifoSem.h @@ -131,15 +131,21 @@ struct LifoSemRawNode { typedef folly::IndexedMemPool,32,200,Atom> Pool; /// Storage for all of the waiter nodes for LifoSem-s that use Atom - static Pool pool; + static Pool& pool(); }; /// Use this macro to declare the static storage that backs the raw nodes /// for the specified atomic type -#define LIFOSEM_DECLARE_POOL(Atom, capacity) \ - template<> \ - folly::detail::LifoSemRawNode::Pool \ - folly::detail::LifoSemRawNode::pool((capacity)); +#define LIFOSEM_DECLARE_POOL(Atom, capacity) \ + namespace folly { \ + namespace detail { \ + template <> \ + LifoSemRawNode::Pool& LifoSemRawNode::pool() { \ + static Pool* instance = new Pool((capacity)); \ + return *instance; \ + } \ + } \ + } /// Handoff is a type not bigger than a void* that knows how to perform a /// single post() -> wait() communication. It must have a post() method. @@ -180,8 +186,8 @@ template class Atom> struct LifoSemNodeRecycler { void operator()(LifoSemNode* elem) const { elem->destroy(); - auto idx = LifoSemRawNode::pool.locateElem(elem); - LifoSemRawNode::pool.recycleIndex(idx); + auto idx = LifoSemRawNode::pool().locateElem(elem); + LifoSemRawNode::pool().recycleIndex(idx); } }; @@ -478,14 +484,14 @@ struct LifoSemBase { /// Returns a node that can be passed to decrOrLink template UniquePtr allocateNode(Args&&... args) { - auto idx = LifoSemRawNode::pool.allocIndex(); + auto idx = LifoSemRawNode::pool().allocIndex(); if (idx != 0) { auto& node = idxToNode(idx); node.clearShutdownNotice(); try { node.init(std::forward(args)...); } catch (...) { - LifoSemRawNode::pool.recycleIndex(idx); + LifoSemRawNode::pool().recycleIndex(idx); throw; } return UniquePtr(&node); @@ -515,12 +521,12 @@ struct LifoSemBase { static LifoSemNode& idxToNode(uint32_t idx) { - auto raw = &LifoSemRawNode::pool[idx]; + auto raw = &LifoSemRawNode::pool()[idx]; return *static_cast*>(raw); } static uint32_t nodeToIdx(const LifoSemNode& node) { - return LifoSemRawNode::pool.locateElem(&node); + return LifoSemRawNode::pool().locateElem(&node); } /// Either increments by n and returns 0, or pops a node and returns it. diff --git a/folly/test/LifoSemTests.cpp b/folly/test/LifoSemTests.cpp index 85e67c8bae0..6006c10da30 100644 --- a/folly/test/LifoSemTests.cpp +++ b/folly/test/LifoSemTests.cpp @@ -401,31 +401,40 @@ static void contendedUse(uint n, int posters, int waiters) { BENCHMARK_DRAW_LINE() BENCHMARK_NAMED_PARAM(contendedUse, 1_to_1, 1, 1) +BENCHMARK_NAMED_PARAM(contendedUse, 1_to_4, 1, 4) BENCHMARK_NAMED_PARAM(contendedUse, 1_to_32, 1, 32) +BENCHMARK_NAMED_PARAM(contendedUse, 4_to_1, 4, 1) +BENCHMARK_NAMED_PARAM(contendedUse, 4_to_24, 4, 24) +BENCHMARK_NAMED_PARAM(contendedUse, 8_to_100, 8, 100) BENCHMARK_NAMED_PARAM(contendedUse, 32_to_1, 31, 1) BENCHMARK_NAMED_PARAM(contendedUse, 16_to_16, 16, 16) BENCHMARK_NAMED_PARAM(contendedUse, 32_to_32, 32, 32) BENCHMARK_NAMED_PARAM(contendedUse, 32_to_1000, 32, 1000) -// sudo nice -n -20 folly/test/LifoSemTests --benchmark --bm_min_iters=10000000 +// sudo nice -n -20 _build/opt/folly/test/LifoSemTests \ +// --benchmark --bm_min_iters=10000000 --gtest_filter=-\* // ============================================================================ // folly/test/LifoSemTests.cpp relative time/iter iters/s // ============================================================================ -// lifo_sem_pingpong 396.84ns 2.52M -// lifo_sem_oneway 88.52ns 11.30M -// single_thread_lifo_post 14.78ns 67.67M -// single_thread_lifo_wait 13.53ns 73.90M -// single_thread_lifo_postwait 28.91ns 34.59M -// single_thread_lifo_trywait 670.13ps 1.49G -// single_thread_posix_postwait 24.12ns 41.46M -// single_thread_posix_trywait 6.76ns 147.88M +// lifo_sem_pingpong 1.31us 762.40K +// lifo_sem_oneway 193.89ns 5.16M +// single_thread_lifo_post 15.37ns 65.08M +// single_thread_lifo_wait 13.60ns 73.53M +// single_thread_lifo_postwait 29.43ns 33.98M +// single_thread_lifo_trywait 677.69ps 1.48G +// single_thread_posix_postwait 25.03ns 39.95M +// single_thread_posix_trywait 7.30ns 136.98M // ---------------------------------------------------------------------------- -// contendedUse(1_to_1) 143.60ns 6.96M -// contendedUse(1_to_32) 244.06ns 4.10M -// contendedUse(32_to_1) 131.99ns 7.58M -// contendedUse(16_to_16) 210.64ns 4.75M -// contendedUse(32_to_32) 222.91ns 4.49M -// contendedUse(32_to_1000) 453.39ns 2.21M +// contendedUse(1_to_1) 158.22ns 6.32M +// contendedUse(1_to_4) 574.73ns 1.74M +// contendedUse(1_to_32) 592.94ns 1.69M +// contendedUse(4_to_1) 118.28ns 8.45M +// contendedUse(4_to_24) 667.62ns 1.50M +// contendedUse(8_to_100) 701.46ns 1.43M +// contendedUse(32_to_1) 165.06ns 6.06M +// contendedUse(16_to_16) 238.57ns 4.19M +// contendedUse(32_to_32) 219.82ns 4.55M +// contendedUse(32_to_1000) 777.42ns 1.29M // ============================================================================ int main(int argc, char ** argv) {