Skip to content

Commit 01c7c81

Browse files
Andy Limeta-codesync[bot]
authored andcommitted
fix bugs
Summary: We saw some crashes in Jumpstart from a couple different reasons: 1. Race condition in accessing the autoload map from different threads. More details from patzar in D95252496. The fix for this is stolen from that diff just to roll both fixes at once 2. Hitting the max translation limit in HHVM from jumpstart translations. We didn't dedupe the jumpstart translations, so there was a greater chance it could hit the limit. Reviewed By: jaewie Differential Revision: D95271543 fbshipit-source-id: 623e17b7e8acdcaefbc2efa863706eaafd55769c
1 parent c22f939 commit 01c7c81

2 files changed

Lines changed: 17 additions & 3 deletions

File tree

hphp/runtime/vm/jit/mcgen-async.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "hphp/runtime/vm/jit/tc-region.h"
2929
#include "hphp/runtime/vm/jit/tc-internal.h"
3030
#include "hphp/runtime/vm/type-profile.h"
31+
#include "hphp/runtime/vm/jit/mcgen-async.h"
3132

3233
#include "hphp/util/alloc.h"
3334
#include "hphp/util/build-info.h"
@@ -414,9 +415,9 @@ struct AsyncTranslationWorker
414415
SCOPE_EXIT {
415416
if (!Cfg::Repo::Authoritative) {
416417
assertx(Cfg::Eval::EnableAsyncJIT);
417-
if (rctx.currNumTranslations != kIgnoreNumTrans) {
418-
enqueuedSKs().dequeue(ctx.sk);
419-
}
418+
// Dequeue for both normal and jumpstart requests so the SrcKey can be
419+
// re-enqueued if a future retranslation is needed.
420+
enqueuedSKs().dequeue(ctx.sk);
420421
} else {
421422
assertx(Cfg::Eval::EnableAsyncJITLive ||
422423
Cfg::Eval::EnableAsyncJITProfile);
@@ -633,10 +634,18 @@ void joinAsyncTranslationWorkerThreads() {
633634
}
634635

635636
void enqueueAsyncTranslateRequestForJumpstart(RegionContext&& ctx) {
637+
// Use the SrcKeySet to deduplicate jumpstart requests for the same SrcKey.
638+
// The serialized SBProf data may contain multiple entries for the same
639+
// SrcKey (e.g. with different live types). Without dedup, multiple workers
640+
// can concurrently translate and publish for the same SrcKey, pushing
641+
// numTrans past MaxTranslations and violating the translation limit
642+
// invariant.
643+
if (detail::mayEnqueueAsyncTranslateRequest(ctx.sk)) {
636644
dispatcher().enqueue(AsyncRegionTranslationContext {
637645
TransKind::Live, std::move(ctx), kIgnoreNumTrans
638646
});
639647
FTRACE(2, "Enqueued sk {} for jitting in jumpstart\n", show(ctx.sk));
648+
}
640649
}
641650

642651
namespace {

hphp/runtime/vm/jit/prof-data-serialize.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,6 +1163,11 @@ void merge_and_enqueue_for_jit(const std::string& root, int numWorkers) {
11631163

11641164
auto const& pds = getSBDeserProfData();
11651165

1166+
auto const opts = RepoOptions::forFile(root + '/');
1167+
g_context->onLoadWithOptions("", opts);
1168+
auto const map = AutoloadHandler::s_instance->getAutoloadMap();
1169+
always_assert(map);
1170+
11661171
std::vector<VMWorker> workers;
11671172
// numBatches is taken from merge_loaded_units. See the comment there.
11681173
// TODO: tune numBatches.

0 commit comments

Comments
 (0)