Skip to content

Commit

Permalink
never destroy LifoSem's wait node pool
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
Nathan Bronson authored and facebook-github-bot-9 committed Oct 13, 2015
1 parent 5049c2b commit 1dee433
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 26 deletions.
28 changes: 17 additions & 11 deletions folly/LifoSem.h
View file
Expand Up @@ -131,15 +131,21 @@ struct LifoSemRawNode {
typedef folly::IndexedMemPool<LifoSemRawNode<Atom>,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<Atom>::Pool \
folly::detail::LifoSemRawNode<Atom>::pool((capacity));
#define LIFOSEM_DECLARE_POOL(Atom, capacity) \
namespace folly { \
namespace detail { \
template <> \
LifoSemRawNode<Atom>::Pool& LifoSemRawNode<Atom>::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.
Expand Down Expand Up @@ -180,8 +186,8 @@ template <typename Handoff, template<typename> class Atom>
struct LifoSemNodeRecycler {
void operator()(LifoSemNode<Handoff,Atom>* elem) const {
elem->destroy();
auto idx = LifoSemRawNode<Atom>::pool.locateElem(elem);
LifoSemRawNode<Atom>::pool.recycleIndex(idx);
auto idx = LifoSemRawNode<Atom>::pool().locateElem(elem);
LifoSemRawNode<Atom>::pool().recycleIndex(idx);
}
};

Expand Down Expand Up @@ -478,14 +484,14 @@ struct LifoSemBase {
/// Returns a node that can be passed to decrOrLink
template <typename... Args>
UniquePtr allocateNode(Args&&... args) {
auto idx = LifoSemRawNode<Atom>::pool.allocIndex();
auto idx = LifoSemRawNode<Atom>::pool().allocIndex();
if (idx != 0) {
auto& node = idxToNode(idx);
node.clearShutdownNotice();
try {
node.init(std::forward<Args>(args)...);
} catch (...) {
LifoSemRawNode<Atom>::pool.recycleIndex(idx);
LifoSemRawNode<Atom>::pool().recycleIndex(idx);
throw;
}
return UniquePtr(&node);
Expand Down Expand Up @@ -515,12 +521,12 @@ struct LifoSemBase {


static LifoSemNode<Handoff, Atom>& idxToNode(uint32_t idx) {
auto raw = &LifoSemRawNode<Atom>::pool[idx];
auto raw = &LifoSemRawNode<Atom>::pool()[idx];
return *static_cast<LifoSemNode<Handoff, Atom>*>(raw);
}

static uint32_t nodeToIdx(const LifoSemNode<Handoff, Atom>& node) {
return LifoSemRawNode<Atom>::pool.locateElem(&node);
return LifoSemRawNode<Atom>::pool().locateElem(&node);
}

/// Either increments by n and returns 0, or pops a node and returns it.
Expand Down
39 changes: 24 additions & 15 deletions folly/test/LifoSemTests.cpp
View file
Expand Up @@ -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) {
Expand Down

0 comments on commit 1dee433

Please sign in to comment.