Skip to content

Commit 8a9b437

Browse files
yoneymeta-codesync[bot]
authored andcommitted
Guard deopt_stats_ with a mutex for free-threaded Python
Summary: Deopt stats could be accessed from any thread running JIT-compiled code. Add a mutex guarded by `Py_GIL_DISABLED`. Make the `deoptStat()` private and introduce `ifDeoptStat(.., func)` to use it for multi-threading. Reviewed By: alexmalyshev Differential Revision: D94702627 fbshipit-source-id: 957cc36a26d2fcdae5987b58e21b38f1ac473e1d
1 parent af94c02 commit 8a9b437

3 files changed

Lines changed: 42 additions & 11 deletions

File tree

cinderx/Jit/context.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@ void Context::recordDeopt(
211211
CodeRuntime* code_runtime,
212212
std::size_t idx,
213213
BorrowedRef<> guilty_value) {
214+
#ifdef Py_GIL_DISABLED
215+
std::lock_guard<std::mutex> lock(deopt_stats_mutex_);
216+
#endif
214217
DeoptStat& stat = deopt_stats_[code_runtime][idx];
215218
stat.count++;
216219
if (guilty_value != nullptr) {
@@ -233,6 +236,9 @@ const DeoptStat* Context::deoptStat(
233236
}
234237

235238
void Context::clearDeoptStats() {
239+
#ifdef Py_GIL_DISABLED
240+
std::lock_guard<std::mutex> lock(deopt_stats_mutex_);
241+
#endif
236242
deopt_stats_.clear();
237243
}
238244

cinderx/Jit/context.h

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include <functional>
2929
#include <memory>
30+
#include <mutex>
3031
#include <optional>
3132
#include <unordered_map>
3233
#include <unordered_set>
@@ -294,19 +295,35 @@ class Context : public IJitContext {
294295
// is typed to. Typed object references are explicitly excluded.
295296
_PyTypedArgsInfo* findFunctionPrimitiveArgInfo(PyFunctionObject* function);
296297

298+
// Invoke f with the DeoptStat for the given deopt index if it exists.
299+
// Returns true if the stat was found and f was called.
300+
//
301+
// Note: f runs while deopt_stats_mutex_ is held. If f triggers
302+
// recordDeopt() or clearDeoptStats(), the non-recursive mutex will
303+
// deadlock.
304+
template <typename F>
305+
bool ifDeoptStat(
306+
const CodeRuntime* code_runtime,
307+
std::size_t deopt_idx,
308+
F&& f) const {
309+
#ifdef Py_GIL_DISABLED
310+
std::lock_guard<std::mutex> lock(deopt_stats_mutex_);
311+
#endif
312+
const DeoptStat* stat = deoptStat(code_runtime, deopt_idx);
313+
if (stat == nullptr) {
314+
return false;
315+
}
316+
f(*stat);
317+
return true;
318+
}
319+
297320
// Record that a deopt of the given index happened at runtime, with an
298321
// optional guilty value.
299322
void recordDeopt(
300323
CodeRuntime* code_runtime,
301324
std::size_t idx,
302325
BorrowedRef<> guilty_value);
303326

304-
// Get the stat object for a given deopt. It will not exist if the deopt has
305-
// never been hit.
306-
const DeoptStat* deoptStat(
307-
const CodeRuntime* code_runtime,
308-
std::size_t deopt_idx) const;
309-
310327
// Clear all deopt stats.
311328
void clearDeoptStats();
312329

@@ -444,6 +461,16 @@ class Context : public IJitContext {
444461

445462
std::vector<DeoptMetadata> deopt_metadata_;
446463
DeoptStats deopt_stats_;
464+
#ifdef Py_GIL_DISABLED
465+
mutable std::mutex deopt_stats_mutex_;
466+
#endif
467+
468+
// Get the stat object for a given deopt. It will not exist if the deopt has
469+
// never been hit. Caller must hold deopt_stats_mutex_ when Py_GIL_DISABLED.
470+
const DeoptStat* deoptStat(
471+
const CodeRuntime* code_runtime,
472+
std::size_t deopt_idx) const;
473+
447474
GuardFailureCallback guard_failure_callback_;
448475

449476
// References to Python objects held by this Context

cinderx/Jit/pyjit.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,11 +2332,9 @@ Ref<> make_deopt_stats() {
23322332
++deopt_idx) {
23332333
const DeoptMetadata& meta = deopt_metadatas[deopt_idx];
23342334

2335-
auto stat_ptr = ctx->deoptStat(code_runtime, deopt_idx);
2336-
if (stat_ptr == nullptr) {
2337-
continue;
2338-
}
2339-
collect_deopt_stat(*stat_ptr, meta, stats);
2335+
ctx->ifDeoptStat(code_runtime, deopt_idx, [&](const auto& stat) {
2336+
collect_deopt_stat(stat, meta, stats);
2337+
});
23402338
}
23412339
}
23422340

0 commit comments

Comments
 (0)