Skip to content

Commit 14d7af5

Browse files
ricklavoiemeta-codesync[bot]
authored andcommitted
Add closure use-var tracking to distributed HHBBC
Summary: Add support for tracking closure use-var types for HHBBC in distributed mode. This is similar to local mode, except that it never really handled dependencies properly. Distributed mode does this correctly. Reviewed By: mdko Differential Revision: D89898538 fbshipit-source-id: 9c263b01dc7c27d376ce32342501c518ff372409
1 parent fbf6c09 commit 14d7af5

11 files changed

Lines changed: 459 additions & 158 deletions

hphp/hhbbc/analyze.cpp

Lines changed: 122 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ const StaticString s_Closure("Closure");
5353
const StaticString s_AsyncGenerator("HH\\AsyncGenerator");
5454
const StaticString s_Generator("Generator");
5555

56+
const StaticString s_invoke("__invoke");
57+
5658
//////////////////////////////////////////////////////////////////////
5759

5860
/*
@@ -143,9 +145,10 @@ Optional<State> entry_state(const IIndex& index, CollectedInfo& collect,
143145
}
144146

145147
// Closures have use vars, we need to look up their types from the index.
146-
auto const useVars = ctx.func->isClosureBody
147-
? index.lookup_closure_use_vars(*ctx.func)
148-
: CompactVector<Type>{};
148+
auto const useVars = [&] {
149+
if (!ctx.func->isClosureBody) return CompactVector<Type>{};
150+
return collect.closureUseVars.initial(*ctx.func);
151+
}();
149152

150153
/*
151154
* Reified functions have a hidden local that's always the first
@@ -499,7 +502,7 @@ FuncAnalysis do_analyze_collect(const IIndex& index,
499502
}
500503
} while (!incompleteQ.empty());
501504

502-
ai.closureUseTypes = std::move(collect.closureUseTypes);
505+
ai.closureUseTypes = collect.closureUseVars.merged();
503506
ai.effectFree = collect.effectFree;
504507
ai.reanalyzeOnUpdate = collect.reanalyzeOnUpdate;
505508
ai.hasInvariantIterBase = collect.hasInvariantIterBase;
@@ -801,6 +804,12 @@ void ClassAnalysisWorklist::scheduleForReturnType(const php::Func& callee) {
801804
for (auto const f : it->second) schedule(*f);
802805
}
803806

807+
void ClassAnalysisWorklist::scheduleForUseVars(const php::Func& invoke) {
808+
auto const it = useVarDeps.find(&invoke);
809+
if (it == useVarDeps.end()) return;
810+
for (auto const f : it->second) schedule(*f);
811+
}
812+
804813
//////////////////////////////////////////////////////////////////////
805814

806815
ClsConstantWork::ClsConstantWork(const IIndex& index,
@@ -1214,6 +1223,7 @@ ClassAnalysis analyze_class(const IIndex& index, const Context& ctx) {
12141223
assertx(inserted);
12151224
auto [type, refinements] = index.lookup_return_type_raw(f);
12161225
work.returnTypes.emplace(f, std::move(type));
1226+
work.useVars.emplace(f, index.lookup_closure_use_vars_raw(*f));
12171227
funcMeta.emplace(
12181228
f, FuncMeta{ctx.unit, c, &clsAnalysis.closures, refinements}
12191229
);
@@ -1227,8 +1237,29 @@ ClassAnalysis analyze_class(const IIndex& index, const Context& ctx) {
12271237
);
12281238
}
12291239

1230-
// Keep analyzing until we have more functions scheduled (the fixed
1231-
// point).
1240+
// Suppose we first analyze a function which has a CreateCl and
1241+
// record use-var types, then later analysis (within analyze_class)
1242+
// determines the CreateCl was dead. In that case, we'll overwrite
1243+
// the original FuncAnalysisResult (containing the use-var types)
1244+
// with the new one (containing no use-var types because we didn't
1245+
// see a CreateCl). The lack of info in the FuncAnalysisResult will
1246+
// prevent the index from being updated, even though our analysis
1247+
// might depend on some of those initial use-var types. This can
1248+
// cause monotonicity violations. To prevent this, when overwriting
1249+
// a FuncAnalysisResult entry, always make sure to copy over any
1250+
// use-var type into the new one. This ensures that if ever see a
1251+
// CreateCl, the FuncAnalysisResult will always have a record of
1252+
// that.
1253+
auto const persistUseTypes = [&] (const FuncAnalysisResult& from,
1254+
FuncAnalysisResult& to) {
1255+
for (auto const& [clo, types] : from.closureUseTypes) {
1256+
// Will only add closure entries which don't already exist.
1257+
to.closureUseTypes.emplace(clo, types);
1258+
}
1259+
};
1260+
1261+
// Keep analyzing until we have more no functions scheduled (the
1262+
// fixed point).
12321263
while (!work.worklist.empty()) {
12331264
hphp_fast_set<const php::Func*> changed;
12341265

@@ -1247,7 +1278,9 @@ ClassAnalysis analyze_class(const IIndex& index, const Context& ctx) {
12471278
meta.outputIdx = meta.output->size();
12481279
meta.output->emplace_back(std::move(results));
12491280
} else {
1250-
(*meta.output)[meta.outputIdx] = std::move(results);
1281+
auto& output = (*meta.output)[meta.outputIdx];
1282+
persistUseTypes(output, results);
1283+
output = std::move(results);
12511284
}
12521285
}
12531286

@@ -1330,6 +1363,40 @@ ClassAnalysis analyze_class(const IIndex& index, const Context& ctx) {
13301363
);
13311364
}
13321365

1366+
for (auto const& [clo, useVars] : results.closureUseTypes) {
1367+
assertx(is_closure(*clo));
1368+
auto invoke = find_method(clo, s_invoke.get());
1369+
always_assert(invoke);
1370+
1371+
auto const old = folly::get_ptr(work.useVars, invoke);
1372+
if (!old) continue;
1373+
1374+
auto anyChanged = false;
1375+
for (size_t i = 0, size = useVars.size(); i < size; ++i) {
1376+
auto u = useVars.get_default(i, TCell);
1377+
auto const o = old->get_default(i, TCell);
1378+
1379+
if (u.strictlyMoreRefined(o)) {
1380+
old->ensure(i, TCell) = std::move(u);
1381+
anyChanged = true;
1382+
} else {
1383+
always_assert_flog(
1384+
u.moreRefined(o),
1385+
"Index closure use-var invariant violated for {}.\n"
1386+
" {} is not at least as refined as {}\n",
1387+
clo->name,
1388+
show(u),
1389+
show(o)
1390+
);
1391+
}
1392+
}
1393+
1394+
if (anyChanged) {
1395+
work.worklist.scheduleForUseVars(*invoke);
1396+
changed.emplace(invoke);
1397+
}
1398+
}
1399+
13331400
results.localReturnRefinements = meta.localReturnRefinements;
13341401
if (results.localReturnRefinements > 0) --results.localReturnRefinements;
13351402
}
@@ -1396,11 +1463,47 @@ ClassAnalysis analyze_class(const IIndex& index, const Context& ctx) {
13961463
);
13971464
}
13981465

1466+
for (auto const& [clo, useVars] : results.closureUseTypes) {
1467+
assertx(is_closure(*clo));
1468+
auto invoke = find_method(clo, s_invoke.get());
1469+
always_assert(invoke);
1470+
1471+
auto const old = folly::get_ptr(work.useVars, invoke);
1472+
if (!old) continue;
1473+
1474+
auto anyChanged = false;
1475+
for (size_t i = 0, size = useVars.size(); i < size; ++i) {
1476+
auto u = useVars.get_default(i, TCell);
1477+
auto const o = old->get_default(i, TCell);
1478+
1479+
if (u.strictlyMoreRefined(o)) {
1480+
old->ensure(i, TCell) = std::move(u);
1481+
anyChanged = true;
1482+
} else {
1483+
always_assert_flog(
1484+
u.moreRefined(o),
1485+
"Index closure use-var invariant violated for {}.\n"
1486+
" {} is not at least as refined as {}\n",
1487+
clo->name,
1488+
show(u),
1489+
show(o)
1490+
);
1491+
}
1492+
}
1493+
1494+
if (anyChanged) {
1495+
work.worklist.scheduleForUseVars(*invoke);
1496+
changed.emplace(invoke);
1497+
}
1498+
}
1499+
13991500
results.localReturnRefinements = meta.localReturnRefinements;
14001501
if (results.localReturnRefinements > 0) --results.localReturnRefinements;
14011502

14021503
assertx(meta.outputIdx >= 0);
1403-
(*meta.output)[meta.outputIdx] = std::move(results);
1504+
auto& output = (*meta.output)[meta.outputIdx];
1505+
persistUseTypes(output, results);
1506+
output = std::move(results);
14041507
}
14051508

14061509
// Return types have reached a fixed point. However, this means
@@ -1430,14 +1533,21 @@ ClassAnalysis analyze_class(const IIndex& index, const Context& ctx) {
14301533
};
14311534

14321535
hphp_fast_set<SString> retryProps;
1433-
for (auto const f : changed) {
1434-
auto const deps = work.worklist.depsForReturnType(*f);
1435-
if (!deps) continue;
1436-
for (auto const dep : *deps) {
1536+
auto const addProps = [&] (const hphp_fast_set<const php::Func*>& deps) {
1537+
for (auto const dep : deps) {
14371538
auto const propsIt = work.propMutators.find(dep);
14381539
if (propsIt == work.propMutators.end()) continue;
14391540
for (auto const prop : propsIt->second) retryProps.emplace(prop);
14401541
}
1542+
};
1543+
1544+
for (auto const f : changed) {
1545+
if (auto const deps = work.worklist.depsForReturnType(*f)) {
1546+
addProps(*deps);
1547+
}
1548+
if (auto const deps = work.worklist.depsForUseVars(*f)) {
1549+
addProps(*deps);
1550+
}
14411551
}
14421552

14431553
// Schedule the funcs which mutate the props before the ones

hphp/hhbbc/analyze.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ struct ClassAnalysisWorklist {
170170
void scheduleForProp(SString name);
171171
void scheduleForPropMutate(SString name);
172172
void scheduleForReturnType(const php::Func& callee);
173+
void scheduleForUseVars(const php::Func& invoke);
173174

174175
void addPropDep(SString name, const php::Func& f) {
175176
propDeps[name].emplace(&f);
@@ -180,6 +181,9 @@ struct ClassAnalysisWorklist {
180181
void addReturnTypeDep(const php::Func& callee, const php::Func& f) {
181182
returnTypeDeps[&callee].emplace(&f);
182183
}
184+
void addUseVarsDep(const php::Func& invoke, const php::Func& caller) {
185+
useVarDeps[&invoke].emplace(&caller);
186+
}
183187

184188
bool empty() const { return worklist.empty(); }
185189

@@ -194,6 +198,13 @@ struct ClassAnalysisWorklist {
194198
return &it->second;
195199
}
196200

201+
const hphp_fast_set<const php::Func*>*
202+
depsForUseVars(const php::Func& f) const {
203+
auto const it = useVarDeps.find(&f);
204+
if (it == useVarDeps.end()) return nullptr;
205+
return &it->second;
206+
}
207+
197208
private:
198209
hphp_fast_set<const php::Func*> inWorklist;
199210
std::deque<const php::Func*> worklist;
@@ -203,12 +214,14 @@ struct ClassAnalysisWorklist {
203214
Deps<SString> propDeps;
204215
Deps<SString> propMutateDeps;
205216
Deps<const php::Func*> returnTypeDeps;
217+
Deps<const php::Func*> useVarDeps;
206218
};
207219

208220
struct ClassAnalysisWork {
209221
ClassAnalysisWorklist worklist;
210222
hphp_fast_map<const php::Func*, Index::ReturnType> returnTypes;
211223
hphp_fast_map<const php::Func*, hphp_fast_set<SString>> propMutators;
224+
hphp_fast_map<const php::Func*, CompactVector<Type>> useVars;
212225
// List of properties whose initial values have been determined to
213226
// satisfy their type-constraint and which haven't been reflected in
214227
// the index yet.

hphp/hhbbc/emit.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1137,7 +1137,7 @@ void emit_class(EmitUnitState& state, UnitEmitter& ue, PreClassEmitter* pce,
11371137
if (is_closure(cls)) {
11381138
auto f = find_method(&cls, s_invoke.get());
11391139
always_assert(f);
1140-
useVars = state.index.lookup_closure_use_vars(*f, true);
1140+
useVars = state.index.lookup_closure_use_vars_raw(*f);
11411141
}
11421142
auto uvIt = useVars.begin();
11431143

0 commit comments

Comments
 (0)