Skip to content

Commit 6efee0e

Browse files
ricklavoiemeta-codesync[bot]
authored andcommitted
Fix HHBBC start-up to allow for const folding in distributed mode
Summary: HHVM startup is a bit of a mess. You have to call a bunch of functions in exactly the right order or things will break. This dance was not done properly in process_init(), meaning the runtime was not properly initialized. As a result, all attempts to const-fold (by calling builtins) silently failed (the stack was not set up properly so everything failed with a stack overflow). This required calling hphp_request_init(). However, this caused a different problem. It turns out, due to quirks of how RDS is initialized, that you can never call into the interpreter/JIT from the process initial thread, no matter how you initialize the runtime. This isn't a problem in general since we do all the work on other threads. For distributed HHBBC though, the thread doing the analysis is the initial thread. Untangling this provided difficult. I fixed it for distributed HHBBC by pre-initializing RDS before anything else, then providing a flag to hphp_process_init() to tell it *not* to re-initialize RDS. Reviewed By: mdko Differential Revision: D89994296 fbshipit-source-id: 94da25d06ac217c2883554f1e58254056569e0ff
1 parent 7a46fc1 commit 6efee0e

6 files changed

Lines changed: 30 additions & 20 deletions

File tree

hphp/hhbbc/eval-cell.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ namespace HPHP::HHBBC {
3939
* will return non-static objects, or throw exceptions (e.g. tvAdd()
4040
* with an array and an int).
4141
*
42-
* This routine converts these things back to types. In the case of
43-
* an exception it returns TInitCell.
42+
* This routine converts these things back to types. In the case of
43+
* an exception it returns std::nullopt.
4444
*/
4545
template<class Pred>
4646
Optional<Type> eval_cell(Pred p) {

hphp/hhbbc/interp-builtin.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -729,10 +729,10 @@ bool optimize_builtin(ISS& env, const php::Func* func, const FCallArgs& fca) {
729729
}
730730

731731
Optional<Type> const_fold(ISS& env,
732-
uint32_t nArgs,
733-
uint32_t numExtraInputs,
734-
const php::Func& phpFunc,
735-
bool variadicsPacked) {
732+
uint32_t nArgs,
733+
uint32_t numExtraInputs,
734+
const php::Func& phpFunc,
735+
bool variadicsPacked) {
736736
assertx(phpFunc.attrs & AttrIsFoldable);
737737

738738
std::vector<TypedValue> args(nArgs);
@@ -771,7 +771,7 @@ Optional<Type> const_fold(ISS& env,
771771
);
772772
}
773773

774-
FTRACE(1, "invoking: {}\n", func->fullName()->data());
774+
ITRACE(2, "invoking: {}\n", func->fullName());
775775

776776
assertx(!Cfg::Jit::Enabled);
777777
// NB: Coeffects are already checked prior to here by `shouldAttemptToFold`

hphp/hhbbc/main.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ struct LoadRepoJob {
249249
process_init(config.o, config.gd, false);
250250
}
251251
static std::vector<W::Key> fini() {
252-
process_exit();
252+
process_exit(false);
253253
return std::move(s_keys);
254254
}
255255
static Variadic<W::Value> run(UnitEmitterSerdeWrapper wrapper) {
@@ -537,7 +537,6 @@ void compile_repo() {
537537
};
538538
};
539539

540-
HphpSession session{Treadmill::SessionKind::HHBBC};
541540
whole_program(
542541
std::move(inputs),
543542
std::move(config),
@@ -572,6 +571,7 @@ void process_init(const Options& o,
572571
setrlimit(RLIMIT_CORE, &rl);
573572
}
574573

574+
rds::threadInit();
575575
rds::local::init();
576576
SCOPE_FAIL { rds::local::fini(); };
577577

@@ -608,7 +608,7 @@ void process_init(const Options& o,
608608
gd.load(false);
609609

610610
register_process_init();
611-
hphp_process_init(!fullInit);
611+
hphp_process_init(!fullInit, true);
612612
SCOPE_FAIL { hphp_process_exit(); };
613613

614614
options = o;
@@ -624,9 +624,15 @@ void process_init(const Options& o,
624624
Cfg::LoadFromGlobalDataOnlyHHBBC(gd);
625625

626626
options.SourceRootForFileBC = gd.SourceRootForFileBC;
627+
628+
if (fullInit) hphp_session_init(Treadmill::SessionKind::HHBBC);
627629
}
628630

629-
void process_exit() {
631+
void process_exit(bool full) {
632+
if (full) {
633+
hphp_context_exit();
634+
hphp_session_exit();
635+
}
630636
hphp_process_exit();
631637
rds::local::fini();
632638
}
@@ -653,7 +659,7 @@ int main(int argc, char** argv) try {
653659
gd.load(false);
654660

655661
process_init(options, gd, true);
656-
SCOPE_EXIT { process_exit(); };
662+
SCOPE_EXIT { process_exit(true); };
657663

658664
Logger::LogLevel = logging ? Logger::LogInfo : Logger::LogError;
659665
Logger::Escape = false;

hphp/hhbbc/misc.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ using UniquePtrRefVec = std::vector<UniquePtrRef<T>>;
325325
// structures will be parsed and initialized.
326326
void process_init(const Options&, const RepoGlobalData&, bool full);
327327
// Undo process_init().
328-
void process_exit();
328+
void process_exit(bool full);
329329

330330
}}
331331

hphp/runtime/base/program-functions.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2547,12 +2547,13 @@ static void update_constants_and_options() {
25472547
}
25482548
}
25492549

2550-
void hphp_thread_init(bool skipExtensions /* = false */) {
2550+
void hphp_thread_init(bool skipExtensions /* = false */,
2551+
bool skipRDSInit /* = false */) {
25512552
init_current_pthread_stack_limits();
25522553
#if USE_JEMALLOC
25532554
arenas_thread_init();
25542555
#endif
2555-
rds::threadInit();
2556+
if (!skipRDSInit) rds::threadInit();
25562557
ServerStats::GetLogger();
25572558
zend_get_bigint_data();
25582559
zend_rand_init();
@@ -2646,7 +2647,8 @@ void init_current_pthread_stack_limits() {
26462647
}
26472648
}
26482649

2649-
void hphp_process_init(bool initForWorkerProcess /* = false */) {
2650+
void hphp_process_init(bool initForWorkerProcess /* = false */,
2651+
bool skipRDSInit /* = false */) {
26502652
init_current_pthread_stack_limits();
26512653
BootStats::mark("pthread_init");
26522654

@@ -2659,9 +2661,9 @@ void hphp_process_init(bool initForWorkerProcess /* = false */) {
26592661
timezone_init();
26602662
BootStats::mark("timezone_init");
26612663

2662-
rds::processInit();
2664+
if (!skipRDSInit) rds::processInit();
26632665

2664-
hphp_thread_init(/* skipExtensions= */ initForWorkerProcess);
2666+
hphp_thread_init(/* skipExtensions= */ initForWorkerProcess, skipRDSInit);
26652667

26662668
struct sigaction action = {};
26672669
action.sa_sigaction = on_timeout;

hphp/runtime/base/program-functions.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ struct ExecutionContext;
8787
* If `initAsWorker` is set, JIT data structure and extensions will not be
8888
* initialized as they're expensive to set up and unused by workers.
8989
*/
90-
void hphp_process_init(bool initForWorkerProcess = false);
90+
void hphp_process_init(bool initForWorkerProcess = false,
91+
bool skipRDSInit = false);
9192
void cli_client_init();
9293
void cli_client_thread_init();
9394
void cli_client_thread_exit();
@@ -117,7 +118,8 @@ bool hphp_invoke(ExecutionContext *context,
117118
bool allowDynCallNoPointer = false);
118119
void hphp_context_exit();
119120

120-
void hphp_thread_init(bool skipExtensions = false);
121+
void hphp_thread_init(bool skipExtensions = false,
122+
bool skipRDSInit = false);
121123
void hphp_thread_exit(bool skipExtensions = false);
122124

123125
void init_current_pthread_stack_limits();

0 commit comments

Comments
 (0)