Skip to content

Commit

Permalink
Reset TLS on Profiling Entrance (#999)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #999

D63924780 broke some tests because of pthread_atfork having strange properties with subsequent calls. To fix this, lets deviate from this method and just reset the TLS whenever we enter the a profiling context. This will ensure that we will start "fresh" for all the PID/TID related content upon every profile. The drawbacks are:

1. 1 Extra Cache Miss per Profile - This is negligible because the cache miss is during the prepare stage for auto-trace and schedule for on-demand. To add to this, the cache miss penalty is very small

2. No Reset if Fork during Profile - If someone were to fork in the middle of a profile the TLS won't get reset. However, there are many other issues that could happen due to a fork midway through a profile such as undefined behavior with cupti, distorted profiling window etc. We shouldn't worry about this case as of today.

Reviewed By: aaronenyeshi, briancoutinho

Differential Revision: D64120658

fbshipit-source-id: d7ed8462f76dfe6042f4a9a97979fa5010cccd2e
  • Loading branch information
sraikund16 authored and facebook-github-bot committed Oct 10, 2024
1 parent a6febf5 commit 00a00e0
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 1 deletion.
4 changes: 4 additions & 0 deletions libkineto/include/ThreadUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,8 @@ std::string processName(int32_t pid);
// and its parents.
std::vector<std::pair<int32_t, std::string>> pidCommandPairsOfAncestors();

// Resets all cached Thread local state, this must be done on
// forks to prevent stale values from being retained.
void resetTLS();

} // namespace libkineto
4 changes: 4 additions & 0 deletions libkineto/include/libkineto.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ class LibkinetoApi {
suppressLibkinetoLogMessages();
}

void resetKinetoTLS() {
resetTLS();
}

// Provides access to profier configuration manaegement
ConfigLoader& configLoader() {
return configLoader_;
Expand Down
2 changes: 2 additions & 0 deletions libkineto/src/ActivityProfilerProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ActivityProfilerController.h"
#include "Config.h"
#include "Logger.h"
#include "ThreadUtil.h"

namespace KINETO_NAMESPACE {

Expand All @@ -31,6 +32,7 @@ void ActivityProfilerProxy::init() {
}

void ActivityProfilerProxy::scheduleTrace(const std::string& configStr) {
resetTLS();
Config config;
config.parse(configStr);
controller_->scheduleTrace(config);
Expand Down
4 changes: 3 additions & 1 deletion libkineto/src/ConfigLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,9 @@ void ConfigLoader::stopThread() {
std::lock_guard<std::mutex> lock(updateThreadMutex_);
updateThreadCondVar_.notify_one();
}
updateThread_->join();
if (updateThread_->joinable()) {
updateThread_->join();
}
updateThread_ = nullptr;
}
}
Expand Down
8 changes: 8 additions & 0 deletions libkineto/src/ThreadUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ int32_t threadId() {
return _tid;
}

// Resets all cached Thread local state, this must be done on
// forks to prevent stale values from being retained.
void resetTLS() {
_pid = 0;
_tid = 0;
_sysTid = 0;
}

namespace {
static constexpr size_t kMaxThreadNameLength = 16;

Expand Down
1 change: 1 addition & 0 deletions libkineto/src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "ConfigLoader.h"
#include "DaemonConfigLoader.h"
#include "DeviceUtil.h"
#include "ThreadUtil.h"
#ifdef HAS_CUPTI
#include "CuptiActivityApi.h"
#include "CuptiCallbackApi.h"
Expand Down

0 comments on commit 00a00e0

Please sign in to comment.