[compiler-rt] 111d8f7 - tsan: remove quadratic behavior in pthread_join
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 17 19:41:53 PST 2021
Author: Dmitry Vyukov
Date: 2021-11-17T19:41:49-08:00
New Revision: 111d8f785b3a2a163411cef00327248d35571612
URL: https://github.com/llvm/llvm-project/commit/111d8f785b3a2a163411cef00327248d35571612
DIFF: https://github.com/llvm/llvm-project/commit/111d8f785b3a2a163411cef00327248d35571612.diff
LOG: tsan: remove quadratic behavior in pthread_join
pthread_join needs to map pthread_t of the joined thread to our Tid.
Currently we do this with linear search over all threads.
This has quadratic complexity and becomes much worse with the new
tsan runtime, which memorizes all threads that ever existed.
To resolve this add a hash map of live threads only (that are still
associated with pthread_t) and use it for the mapping.
With the new tsan runtime some programs spent 1/3 of time in this mapping.
After this change the mapping disappears from profiles.
Depends on D113996.
Reviewed By: vitalybuka, melver
Differential Revision: https://reviews.llvm.org/D113997
Added:
Modified:
compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
index a34b8c15aa5b0..2e1cd0238812b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
@@ -13,6 +13,8 @@
#include "sanitizer_thread_registry.h"
+#include "sanitizer_placement_new.h"
+
namespace __sanitizer {
ThreadContextBase::ThreadContextBase(u32 tid)
@@ -162,6 +164,12 @@ u32 ThreadRegistry::CreateThread(uptr user_id, bool detached, u32 parent_tid,
max_alive_threads_++;
CHECK_EQ(alive_threads_, max_alive_threads_);
}
+ if (user_id) {
+ // Ensure that user_id is unique. If it's not the case we are screwed.
+ // Ignoring this situation may lead to very hard to debug false
+ // positives later (e.g. if we join a wrong thread).
+ CHECK(live_.try_emplace(user_id, tid).second);
+ }
tctx->SetCreated(user_id, total_threads_++, detached,
parent_tid, arg);
return tid;
@@ -221,14 +229,8 @@ void ThreadRegistry::SetThreadName(u32 tid, const char *name) {
void ThreadRegistry::SetThreadNameByUserId(uptr user_id, const char *name) {
ThreadRegistryLock l(this);
- for (u32 tid = 0; tid < threads_.size(); tid++) {
- ThreadContextBase *tctx = threads_[tid];
- if (tctx != 0 && tctx->user_id == user_id &&
- tctx->status != ThreadStatusInvalid) {
- tctx->SetName(name);
- return;
- }
- }
+ if (const auto *tid = live_.find(user_id))
+ threads_[tid->second]->SetName(name);
}
void ThreadRegistry::DetachThread(u32 tid, void *arg) {
@@ -241,6 +243,8 @@ void ThreadRegistry::DetachThread(u32 tid, void *arg) {
}
tctx->OnDetached(arg);
if (tctx->status == ThreadStatusFinished) {
+ if (tctx->user_id)
+ live_.erase(tctx->user_id);
tctx->SetDead();
QuarantinePush(tctx);
} else {
@@ -260,6 +264,8 @@ void ThreadRegistry::JoinThread(u32 tid, void *arg) {
return;
}
if ((destroyed = tctx->GetDestroyed())) {
+ if (tctx->user_id)
+ live_.erase(tctx->user_id);
tctx->SetJoined(arg);
QuarantinePush(tctx);
}
@@ -292,6 +298,8 @@ ThreadStatus ThreadRegistry::FinishThread(u32 tid) {
}
tctx->SetFinished();
if (dead) {
+ if (tctx->user_id)
+ live_.erase(tctx->user_id);
tctx->SetDead();
QuarantinePush(tctx);
}
@@ -333,6 +341,19 @@ ThreadContextBase *ThreadRegistry::QuarantinePop() {
return tctx;
}
+u32 ThreadRegistry::ConsumeThreadUserId(uptr user_id) {
+ ThreadRegistryLock l(this);
+ u32 tid;
+ auto *t = live_.find(user_id);
+ CHECK(t);
+ tid = t->second;
+ live_.erase(t);
+ auto *tctx = threads_[tid];
+ CHECK_EQ(tctx->user_id, user_id);
+ tctx->user_id = 0;
+ return tid;
+}
+
void ThreadRegistry::SetThreadUserId(u32 tid, uptr user_id) {
ThreadRegistryLock l(this);
ThreadContextBase *tctx = threads_[tid];
@@ -341,6 +362,7 @@ void ThreadRegistry::SetThreadUserId(u32 tid, uptr user_id) {
CHECK_NE(tctx->status, ThreadStatusDead);
CHECK_EQ(tctx->user_id, 0);
tctx->user_id = user_id;
+ CHECK(live_.try_emplace(user_id, tctx->tid).second);
}
} // namespace __sanitizer
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
index a8a4d4d86a03e..a259b324220f3 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
@@ -15,6 +15,7 @@
#define SANITIZER_THREAD_REGISTRY_H
#include "sanitizer_common.h"
+#include "sanitizer_dense_map.h"
#include "sanitizer_list.h"
#include "sanitizer_mutex.h"
@@ -127,6 +128,7 @@ class MUTEX ThreadRegistry {
// Finishes thread and returns previous status.
ThreadStatus FinishThread(u32 tid);
void StartThread(u32 tid, tid_t os_id, ThreadType thread_type, void *arg);
+ u32 ConsumeThreadUserId(uptr user_id);
void SetThreadUserId(u32 tid, uptr user_id);
private:
@@ -146,6 +148,7 @@ class MUTEX ThreadRegistry {
InternalMmapVector<ThreadContextBase *> threads_;
IntrusiveList<ThreadContextBase> dead_threads_;
IntrusiveList<ThreadContextBase> invalid_threads_;
+ DenseMap<uptr, Tid> live_;
void QuarantinePush(ThreadContextBase *tctx);
ThreadContextBase *QuarantinePop();
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
index dfead437f467b..c8f7124c009d6 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
@@ -270,29 +270,8 @@ struct ConsumeThreadContext {
ThreadContextBase *tctx;
};
-static bool ConsumeThreadByUid(ThreadContextBase *tctx, void *arg) {
- ConsumeThreadContext *findCtx = (ConsumeThreadContext *)arg;
- if (tctx->user_id == findCtx->uid && tctx->status != ThreadStatusInvalid) {
- if (findCtx->tctx) {
- // Ensure that user_id is unique. If it's not the case we are screwed.
- // Something went wrong before, but now there is no way to recover.
- // Returning a wrong thread is not an option, it may lead to very hard
- // to debug false positives (e.g. if we join a wrong thread).
- Report("ThreadSanitizer: dup thread with used id 0x%zx\n", findCtx->uid);
- Die();
- }
- findCtx->tctx = tctx;
- tctx->user_id = 0;
- }
- return false;
-}
-
Tid ThreadConsumeTid(ThreadState *thr, uptr pc, uptr uid) {
- ConsumeThreadContext findCtx = {uid, nullptr};
- ctx->thread_registry.FindThread(ConsumeThreadByUid, &findCtx);
- Tid tid = findCtx.tctx ? findCtx.tctx->tid : kInvalidTid;
- DPrintf("#%d: ThreadTid uid=%zu tid=%d\n", thr->tid, uid, tid);
- return tid;
+ return ctx->thread_registry.ConsumeThreadUserId(uid);
}
void ThreadJoin(ThreadState *thr, uptr pc, Tid tid) {
More information about the llvm-commits
mailing list