Skip to content

Commit

Permalink
fix accidentally-quadratic complexity of ThreadEntrySet ops in debug
Browse files Browse the repository at this point in the history
Summary:
`ThreadEntrySet::basicSanity()` is linear in the size of the entry-set, and is invoked on every mutation to (and test of!) the entry-set in debug. A sequence of `N` ops then has quadratic complexity `O(N^2)`.

As one possible resolution, we scan the entry-set once once every `log(N) / N` times, so that `N` ops has complexity `O(N log(N))` rather than `O(N^2)`. And we randomize the scan so that deterministic bugs may be surfaced that would evade a deterministic scan algorithm.

Differential Revision: D66733958

fbshipit-source-id: fb91bf834e6a9221ef012fc8f8cad064b2a8c26c
  • Loading branch information
yfeldblum authored and facebook-github-bot committed Dec 6, 2024
1 parent be79ab6 commit 36e5c5b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
2 changes: 2 additions & 0 deletions folly/detail/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ cpp_library(
headers = ["ThreadLocalDetail.h"],
deps = [
":thread_local_globals",
"//folly:constexpr_math",
"//folly:utility",
"//folly/lang:hint",
"//folly/memory:sanitize_leak",
"//folly/synchronization:call_once",
Expand Down
26 changes: 25 additions & 1 deletion folly/detail/ThreadLocalDetail.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
#include <algorithm>
#include <list>
#include <mutex>
#include <random>

#include <folly/ConstexprMath.h>
#include <folly/Utility.h>
#include <folly/detail/thread_local_globals.h>
#include <folly/lang/Hint.h>
#include <folly/memory/SanitizeLeak.h>
Expand All @@ -31,6 +34,13 @@ constexpr auto kBigGrowthFactor = 1.7;
namespace folly {
namespace threadlocal_detail {

struct rand_engine {
using result_type = unsigned int;
result_type operator()() { return to_unsigned(std::rand()); }
static constexpr result_type min() { return 0; }
static constexpr result_type max() { return RAND_MAX; }
};

SharedPtrDeleter::SharedPtrDeleter(std::shared_ptr<void> const& ts) noexcept
: ts_{ts} {}
SharedPtrDeleter::SharedPtrDeleter(SharedPtrDeleter const& that) noexcept
Expand All @@ -48,8 +58,22 @@ uintptr_t ElementWrapper::castForgetAlign(DeleterFunType* f) noexcept {
}

bool ThreadEntrySet::basicSanity() const {
if constexpr (!kIsDebug) {
return true;
}
if (threadEntries.empty() && entryToVectorSlot.empty()) {
return true;
}
if (threadEntries.size() != entryToVectorSlot.size()) {
return false;
}
auto const size = threadEntries.size();
rand_engine rng;
std::uniform_int_distribution<size_t> dist{0, size - 1};
if (dist(rng) < constexpr_log2(size)) {
return true;
}
return //
threadEntries.size() == entryToVectorSlot.size() &&
std::all_of(
entryToVectorSlot.begin(),
entryToVectorSlot.end(),
Expand Down

0 comments on commit 36e5c5b

Please sign in to comment.