Skip to content

Commit 1368add

Browse files
Hunter Goldsteinfacebook-github-bot
Hunter Goldstein
authored andcommitted
RFC: Expose systemlib decls in full repo builds
Summary: The previous diffs in this stack exposed systemlib decls to: * Incremental file compilation * Compiling systemlib itself at HHVM startup time This diff indexes all of the decls in `s_builtin_symbols` akin to how files in the repo itself are indexed. They're indexed with an empty file name, which I did specifically to avoid attempts to parse / emit bytecode for said files as part of `Package::resolveOnDemand`: we'll unconditionally parse / emit the bytecode for systemlib. Reviewed By: edwinsmith Differential Revision: D39754321 fbshipit-source-id: 3a4fa9d30823293ae9b5570040bc03b0ffec6ecf
1 parent 3956046 commit 1368add

File tree

6 files changed

+104
-16
lines changed

6 files changed

+104
-16
lines changed

hphp/compiler/compiler.cpp

+53-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
#include "hphp/compiler/option.h"
2020
#include "hphp/compiler/package.h"
2121

22+
#include "hphp/hack/src/hackc/ffi_bridge/compiler_ffi.rs.h"
23+
2224
#include "hphp/hhbbc/hhbbc.h"
2325
#include "hphp/hhbbc/misc.h"
2426
#include "hphp/hhbbc/options.h"
@@ -31,6 +33,7 @@
3133
#include "hphp/runtime/base/variable-serializer.h"
3234
#include "hphp/runtime/version.h"
3335

36+
#include "hphp/runtime/vm/builtin-symbol-map.h"
3437
#include "hphp/runtime/vm/disas.h"
3538
#include "hphp/runtime/vm/preclass-emitter.h"
3639
#include "hphp/runtime/vm/repo-autoload-map-builder.h"
@@ -816,6 +819,38 @@ void logPhaseStats(const std::string& phase, const Package& package,
816819
sample.setStr(phase + "_fellback", client.fellback() ? "true" : "false");
817820
}
818821

822+
namespace {
823+
// Upload all builtin decls, and pass their IndexMeta summary and
824+
// Ref<UnitDecls> to callback() to include in the overall UnitIndex. This
825+
// makes systemlib decls visible to files being compiled as part of the
826+
// full repo build, but does not make repo decls available to systemlib.
827+
coro::Task<bool> indexBuiltinSymbolDecls(
828+
const Package::IndexCallback& callback,
829+
coro::TicketExecutor& executor,
830+
extern_worker::Client& client
831+
) {
832+
std::vector<coro::TaskWithExecutor<void>> tasks;
833+
auto const declCallback = [&](auto const* d) -> coro::Task<void> {
834+
auto const facts = hackc::decls_to_facts_cpp_ffi(*d, "");
835+
auto summary = summary_of_facts(facts);
836+
callback(
837+
"",
838+
summary,
839+
HPHP_CORO_AWAIT(client.store(Package::UnitDecls{
840+
summary,
841+
std::string{d->serialized.begin(), d->serialized.end()}
842+
}))
843+
);
844+
HPHP_CORO_RETURN_VOID;
845+
};
846+
for (auto const& d: Native::getAllBuiltinDecls()) {
847+
tasks.emplace_back(declCallback(d).scheduleOn(executor.sticky()));
848+
}
849+
HPHP_CORO_AWAIT(coro::collectRange(std::move(tasks)));
850+
HPHP_CORO_RETURN(true);
851+
}
852+
}
853+
819854
// Compute a UnitIndex by parsing decls for all autoload-eligible files.
820855
// If no Autoload.Query is specified by RepoOptions, this just indexes
821856
// the input files.
@@ -862,8 +897,25 @@ std::unique_ptr<UnitIndex> computeIndex(
862897
// index just the input files
863898
addInputsToPackage(indexPackage, po);
864899
}
900+
// Here, we are doing the following in parallel:
901+
// * Indexing the build package
902+
// * Indexing builtin decls to be used by decl driven bytecode compilation
903+
// If DDB is not enabled, we will return early from the second task.
904+
auto const [indexingRepoOK, indexingSystemlibDeclsOK] = coro::wait(
905+
coro::collect(
906+
indexPackage.index(indexUnit),
907+
coro::invoke([&]() -> coro::Task<bool> {
908+
if (RO::EvalEnableDecl) {
909+
HPHP_CORO_RETURN(HPHP_CORO_AWAIT(
910+
indexBuiltinSymbolDecls(indexUnit, executor, client)
911+
));
912+
}
913+
HPHP_CORO_RETURN(true);
914+
})
915+
)
916+
);
865917

866-
if (!coro::wait(indexPackage.index(indexUnit))) return nullptr;
918+
if (!indexingRepoOK || !indexingSystemlibDeclsOK) return nullptr;
867919

868920
logPhaseStats("index", indexPackage, client, sample,
869921
indexTimer.getMicroSeconds());

hphp/compiler/package.cpp

+18-13
Original file line numberDiff line numberDiff line change
@@ -1115,19 +1115,7 @@ Package::UnitDecls IndexJob::run(
11151115

11161116
// Get Facts from Decls, then populate IndexMeta.
11171117
auto facts = hackc::decls_to_facts_cpp_ffi(decls, "");
1118-
Package::IndexMeta summary;
1119-
for (auto& e : facts.facts.types) {
1120-
summary.types.emplace_back(makeStaticString(std::string(e.name)));
1121-
}
1122-
for (auto& e : facts.facts.functions) {
1123-
summary.funcs.emplace_back(makeStaticString(std::string(e)));
1124-
}
1125-
for (auto& e : facts.facts.constants) {
1126-
summary.constants.emplace_back(makeStaticString(std::string(e)));
1127-
}
1128-
for (auto& e : facts.facts.modules) {
1129-
summary.modules.emplace_back(makeStaticString(std::string(e.name)));
1130-
}
1118+
auto summary = summary_of_facts(facts);
11311119
s_indexMetas.emplace_back(summary);
11321120
if (!RO::EvalEnableDecl) {
11331121
// If decl-directed bytecode is disabled, parseRun() will not need
@@ -1518,3 +1506,20 @@ coro::Task<Package::FileAndSizeVec> Package::emitGroup(
15181506

15191507
HPHP_CORO_RETURN(FileAndSizeVec{});
15201508
}
1509+
1510+
Package::IndexMeta HPHP::summary_of_facts(const hackc::FactsResult& facts) {
1511+
Package::IndexMeta summary;
1512+
for (auto& e : facts.facts.types) {
1513+
summary.types.emplace_back(makeStaticString(std::string(e.name)));
1514+
}
1515+
for (auto& e : facts.facts.functions) {
1516+
summary.funcs.emplace_back(makeStaticString(std::string(e)));
1517+
}
1518+
for (auto& e : facts.facts.constants) {
1519+
summary.constants.emplace_back(makeStaticString(std::string(e)));
1520+
}
1521+
for (auto& e : facts.facts.modules) {
1522+
summary.modules.emplace_back(makeStaticString(std::string(e.name)));
1523+
}
1524+
return summary;
1525+
}

hphp/compiler/package.h

+8
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@
3838
namespace HPHP {
3939
///////////////////////////////////////////////////////////////////////////////
4040

41+
namespace hackc {
42+
struct FactsResult;
43+
}
44+
4145
struct UnitIndex;
4246

4347
struct Package {
@@ -371,5 +375,9 @@ struct UnitIndex final {
371375
Map modules;
372376
};
373377

378+
// Given the result of running `hackc::decls_to_facts_cpp_ffi`, create a
379+
// `Package::IndexMeta` containing the names of all decls in `facts`.
380+
Package::IndexMeta summary_of_facts(const hackc::FactsResult& facts);
381+
374382
///////////////////////////////////////////////////////////////////////////////
375383
}

hphp/runtime/vm/builtin-symbol-map.cpp

+18
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,22 @@ Optional<hackc::ExternalDeclProviderResult> getBuiltinDecls(
123123
not_reached();
124124
}
125125

126+
hphp_fast_set<const hackc::DeclResult*> getAllBuiltinDecls() {
127+
assertx(RuntimeOption::EvalEnableDecl);
128+
hphp_fast_set<const hackc::DeclResult*> res;
129+
for (const auto& it: s_types) {
130+
res.emplace(it.second.get());
131+
}
132+
for (const auto& it: s_functions) {
133+
res.emplace(it.second.get());
134+
}
135+
for (const auto& it: s_constants) {
136+
res.emplace(it.second.get());
137+
}
138+
for (const auto& it: s_modules) {
139+
res.emplace(it.second.get());
140+
}
141+
return res;
142+
}
143+
126144
} // namespace HPHP::Native

hphp/runtime/vm/builtin-symbol-map.h

+7-2
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,17 @@
1616

1717
#pragma once
1818

19-
#include "hphp/hack/src/hackc/ffi_bridge/decl_provider.h"
20-
2119
#include "hphp/runtime/base/autoload-map.h"
2220

2321
#include "hphp/util/optional.h"
2422

2523
#include <string>
2624

25+
namespace HPHP::hackc {
26+
struct ExternalDeclProviderResult;
27+
struct DeclResult;
28+
}
29+
2730
namespace HPHP::Native {
2831

2932
/**
@@ -48,4 +51,6 @@ Optional<hackc::ExternalDeclProviderResult> getBuiltinDecls(
4851
AutoloadMap::KindOf kind
4952
);
5053

54+
hphp_fast_set<const hackc::DeclResult*> getAllBuiltinDecls();
55+
5156
} // namespace HPHP::Native

hphp/test/slow/decl-provider/embed_type_decl.php.norepo

Whitespace-only changes.

0 commit comments

Comments
 (0)