Skip to content

Commit de5328d

Browse files
committed
mcpp.graph keeps each site's order: the module graph's stack order (link order, and so Mach-O initializer order) and the host-module depth-first orders; groups sharing one build directory build one after the other; status lines written whole
1 parent ba4b0c6 commit de5328d

9 files changed

Lines changed: 261 additions & 55 deletions

File tree

‎.agents/docs/2026-09-29-workspace-build-graph-design.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,7 @@ left open, the answer is recorded here.
522522
| The lock | `<workspace>/mcpp.lock`. A plan of all members writes the whole record; a plan of some keeps the other entries, and `--locked` then reports no entry of another member as drift. A selected member's git dependencies are locked as a root's. | prepare/records.cpp |
523523
| Tests | `mcpp test` keeps one plan per member (`-p X` each), in the shared directory; the members' dev-dependencies are their own. | cmd_build.cppm |
524524
| Concurrency | Groups are planned in turn and built on threads with a static share of the jobs; the `.build_cache` write is one locked step. | cmd_build.cppm |
525+
| The orders of `mcpp.graph` | Three: the stable order, the module graph's own (Kahn with the ready units on a stack, over the edges in their recorded order) and a depth-first post-order from roots in the given order. Each migrated site keeps the order it had: the module graph's unit order is the order of the objects on a link line, which Mach-O uses as initializer order, and the first migration to the stable order made openkal's `same-source` example crash at start on aarch64-macos from all three build hosts (CI, run 36486818199). With the orders restored, the link line of that example is byte-identical to 2026.9.28.3's. | modules/graph |
525526
| Module names across members | A graph has one module namespace (BMIs are found by name), so two members that each provide a module of one name are refused in one `--workspace` plan, as two `artifacts` programs are (#732, which tracks per-provider BMI names). Measured over the package index's 172 members: no module name is provided twice. | scanner |
526527

527528
Readings with the implementation (Linux, llvm 22.1.8):

‎modules/graph/src/graph.cppm‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,23 @@
2222
// a package's `[dependencies]`), so no call site has to invert its data to
2323
// call in.
2424
//
25+
// THREE ORDERS, AND WHY A CALLER KEEPS THE ONE IT HAD
26+
//
27+
// An order among nodes that do not constrain each other is still observable
28+
// when the order becomes a link order: Mach-O runs initializers in link order,
29+
// and the module graph's unit order is the order of the objects on the link
30+
// line. Changing it changed a program's initialization (measured 2026-09-29:
31+
// openkal's `same-source` example, linked for aarch64-macos, crashed at start
32+
// when its units moved to the stable order below). A caller whose order
33+
// reaches an artifact therefore keeps its order, and this module offers each
34+
// order the engine uses:
35+
//
36+
// topological_order Kahn, lowest ready index first (stable)
37+
// stack_topological_order Kahn, most recently readied first, over edges in
38+
// the order given (the module graph's unit order)
39+
// depth_first_order dependencies first, depth first, from roots in
40+
// the order given (the host-module orders)
41+
//
2542
// STABILITY
2643
//
2744
// `topological_order` and `levels` are Kahn's algorithm with the ready set
@@ -89,6 +106,27 @@ std::vector<std::size_t>
89106
closure(const AdjacencyList& deps, std::span<const std::size_t> roots,
90107
bool include_roots = false);
91108

109+
// Kahn's algorithm with the ready nodes on a stack: the node readied most
110+
// recently is emitted next. `edges` holds (dependent, dependency) pairs over
111+
// nodes 0..n-1, in the order that decides the ties. This is the order of the
112+
// module graph's units, and so of the objects on a link line (see the header).
113+
std::expected<std::vector<std::size_t>, CycleError>
114+
stack_topological_order(std::size_t n,
115+
std::span<const std::pair<std::size_t, std::size_t>> edges);
116+
117+
// What a dependency on a node still on the depth-first path means.
118+
enum class Cycles {
119+
Error, // a cycle: the order is refused with the ring
120+
Skip, // the edge is ignored; the node is emitted where it stands
121+
};
122+
123+
// Dependencies first, depth first: from each root in the order given, a node
124+
// is emitted after every node it depends on, its dependencies visited in the
125+
// order of `deps[u]`. Only nodes reachable from `roots` are emitted.
126+
std::expected<std::vector<std::size_t>, CycleError>
127+
depth_first_order(const AdjacencyList& deps, std::span<const std::size_t> roots,
128+
Cycles cycles = Cycles::Error);
129+
92130
} // namespace mcpp::graph
93131

94132
namespace mcpp::graph {
@@ -185,6 +223,74 @@ topological_order(const AdjacencyList& deps) {
185223
return kahn_order(deps);
186224
}
187225

226+
std::expected<std::vector<std::size_t>, CycleError>
227+
stack_topological_order(std::size_t n,
228+
std::span<const std::pair<std::size_t, std::size_t>> edges) {
229+
std::vector<std::size_t> remaining(n, 0);
230+
std::vector<std::vector<std::size_t>> dependents(n);
231+
for (auto [dependent, dependency] : edges) {
232+
remaining[dependent]++;
233+
dependents[dependency].push_back(dependent);
234+
}
235+
std::vector<std::size_t> order;
236+
order.reserve(n);
237+
std::vector<std::size_t> ready;
238+
for (std::size_t u = 0; u < n; ++u)
239+
if (remaining[u] == 0) ready.push_back(u);
240+
while (!ready.empty()) {
241+
const std::size_t u = ready.back();
242+
ready.pop_back();
243+
order.push_back(u);
244+
for (auto w : dependents[u])
245+
if (--remaining[w] == 0) ready.push_back(w);
246+
}
247+
if (order.size() == n) return order;
248+
// The ring, found the way `topological_order` finds one.
249+
AdjacencyList deps(n);
250+
for (auto [dependent, dependency] : edges) deps[dependent].push_back(dependency);
251+
auto ring = kahn_order(deps);
252+
if (!ring) return std::unexpected(ring.error());
253+
return std::unexpected(CycleError{});
254+
}
255+
256+
std::expected<std::vector<std::size_t>, CycleError>
257+
depth_first_order(const AdjacencyList& deps, std::span<const std::size_t> roots,
258+
Cycles cycles) {
259+
const std::size_t n = deps.size();
260+
std::vector<char> state(n, 0); // 0 unseen, 1 on the current path, 2 done
261+
std::vector<std::size_t> order;
262+
std::vector<std::size_t> path;
263+
for (auto root : roots) {
264+
if (root >= n || state[root] != 0) continue;
265+
std::vector<std::pair<std::size_t, std::size_t>> frame{{root, 0}};
266+
state[root] = 1;
267+
path.push_back(root);
268+
while (!frame.empty()) {
269+
auto& [u, k] = frame.back();
270+
if (k < deps[u].size()) {
271+
const auto v = deps[u][k++];
272+
if (v >= n || state[v] == 2) continue;
273+
if (state[v] == 1) {
274+
if (cycles == Cycles::Skip) continue;
275+
CycleError err;
276+
err.cycle.assign(std::ranges::find(path, v), path.end());
277+
err.cycle.push_back(v);
278+
return std::unexpected(std::move(err));
279+
}
280+
state[v] = 1;
281+
path.push_back(v);
282+
frame.push_back({v, 0});
283+
continue;
284+
}
285+
state[u] = 2;
286+
order.push_back(u);
287+
path.pop_back();
288+
frame.pop_back();
289+
}
290+
}
291+
return order;
292+
}
293+
188294
std::expected<std::vector<std::size_t>, CycleError>
189295
levels(const AdjacencyList& deps) {
190296
auto order = kahn_order(deps);

‎modules/graph/tests/test_graph.cpp‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,3 +243,90 @@ TEST(Closure, RootWithNoEdgesHasAnEmptyClosure) {
243243
auto c = g::closure(deps, std::array{std::size_t{0}});
244244
EXPECT_TRUE(c.empty());
245245
}
246+
247+
// ── stack_topological_order: the module graph's unit order ──────────────────
248+
//
249+
// The order the module graph has always had, which is the order of the
250+
// objects on a link line. The reference below is the hand-written loop it
251+
// replaced, run on the same edges.
252+
namespace {
253+
std::vector<std::size_t> reference_stack_order(
254+
std::size_t n, const std::vector<std::pair<std::size_t, std::size_t>>& edges) {
255+
std::vector<std::size_t> indeg(n, 0);
256+
std::vector<std::vector<std::size_t>> adj(n);
257+
for (auto [c, p] : edges) { indeg[c]++; adj[p].push_back(c); }
258+
std::vector<std::size_t> order, queue;
259+
for (std::size_t i = 0; i < n; ++i) if (indeg[i] == 0) queue.push_back(i);
260+
while (!queue.empty()) {
261+
auto u = queue.back(); queue.pop_back();
262+
order.push_back(u);
263+
for (auto v : adj[u]) if (--indeg[v] == 0) queue.push_back(v);
264+
}
265+
return order;
266+
}
267+
} // namespace
268+
269+
TEST(StackTopologicalOrder, IsTheModuleGraphsHistoricalOrder) {
270+
const std::vector<std::pair<std::size_t, std::size_t>> edges = {
271+
{3, 0}, {4, 0}, {4, 1}, {5, 3}, {5, 4}, {6, 2}, {7, 6}, {7, 5}, {1, 2},
272+
};
273+
auto order = g::stack_topological_order(8, edges);
274+
ASSERT_TRUE(order.has_value());
275+
EXPECT_EQ(*order, reference_stack_order(8, edges));
276+
// It is not the stable order: ties go to the most recently readied node.
277+
g::AdjacencyList deps(8);
278+
for (auto [c, p] : edges) deps[c].push_back(p);
279+
EXPECT_NE(*order, *g::topological_order(deps));
280+
}
281+
282+
TEST(StackTopologicalOrder, ACycleIsReportedAsItsPath) {
283+
const std::vector<std::pair<std::size_t, std::size_t>> edges = {{0, 1}, {1, 2}, {2, 0}};
284+
auto order = g::stack_topological_order(3, edges);
285+
ASSERT_FALSE(order.has_value());
286+
auto const& ring = order.error().cycle;
287+
ASSERT_GE(ring.size(), 2u);
288+
EXPECT_EQ(ring.front(), ring.back());
289+
}
290+
291+
// ── depth_first_order: the host-module orders ───────────────────────────────
292+
293+
TEST(DepthFirstOrder, DependenciesFirstFromEachRootInOrder) {
294+
// 0 -> {2, 1}, 1 -> {3}, 2 -> {3}: from root 0, dependency 2 is visited
295+
// before 1 because that is the order `deps[0]` lists them in.
296+
g::AdjacencyList deps(4);
297+
deps[0] = {2, 1};
298+
deps[1] = {3};
299+
deps[2] = {3};
300+
auto order = g::depth_first_order(deps, std::array{std::size_t{0}});
301+
ASSERT_TRUE(order.has_value());
302+
EXPECT_EQ(*order, (std::vector<std::size_t>{3, 2, 1, 0}));
303+
}
304+
305+
TEST(DepthFirstOrder, OnlyWhatTheRootsReachIsEmitted) {
306+
g::AdjacencyList deps(4);
307+
deps[1] = {0};
308+
deps[3] = {2};
309+
auto order = g::depth_first_order(deps, std::array{std::size_t{1}});
310+
ASSERT_TRUE(order.has_value());
311+
EXPECT_EQ(*order, (std::vector<std::size_t>{0, 1}));
312+
}
313+
314+
TEST(DepthFirstOrder, ACycleIsAnErrorNamingTheRing) {
315+
g::AdjacencyList deps(3);
316+
deps[0] = {1};
317+
deps[1] = {2};
318+
deps[2] = {1};
319+
auto order = g::depth_first_order(deps, std::array{std::size_t{0}});
320+
ASSERT_FALSE(order.has_value());
321+
EXPECT_EQ(order.error().cycle, (std::vector<std::size_t>{1, 2, 1}));
322+
}
323+
324+
TEST(DepthFirstOrder, ACycleCanBeSkipped) {
325+
g::AdjacencyList deps(2);
326+
deps[0] = {1};
327+
deps[1] = {0};
328+
auto order = g::depth_first_order(deps, std::array{std::size_t{0}, std::size_t{1}},
329+
g::Cycles::Skip);
330+
ASSERT_TRUE(order.has_value());
331+
EXPECT_EQ(*order, (std::vector<std::size_t>{1, 0}));
332+
}

‎src/build/prepare/features.cpp‎

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,28 +1019,25 @@ static std::vector<prov::HostModule> host_module_units(const PrepareState& st, s
10191019
byName.emplace(pending[i].name, i);
10201020

10211021
// deps[u] = the pending-list indices `u` imports BY NAME within this
1022-
// same package. mcpp.graph's stable tie-break (lowest index first among
1023-
// unconstrained nodes) is what preserves path order in the
1024-
// unconstrained case; the old hand-written DFS achieved the same thing
1025-
// by always trying the path-sorted list from the front.
1022+
// same package, in import order. A depth-first order from every unit in
1023+
// path order emits the first unit that can be emitted, which is what
1024+
// preserves path order in the unconstrained case, and is the order the
1025+
// build program's objects are linked in.
10261026
mcpp::graph::AdjacencyList deps(pending.size());
10271027
for (std::size_t u = 0; u < pending.size(); ++u)
10281028
for (auto const& want : pending[u].imports)
10291029
if (auto it = byName.find(want); it != byName.end())
10301030
deps[u].push_back(it->second);
1031+
std::vector<std::size_t> roots(pending.size());
1032+
std::iota(roots.begin(), roots.end(), std::size_t{0});
10311033

10321034
// A CYCLE IS LEFT TO THE COMPILER, ON PURPOSE. It is ill-formed C++ and
10331035
// the compiler says so with the two units named; refusing here would
10341036
// report the same fact in a worse place, and getting the ordering wrong
1035-
// is no longer possible either way -- so on a cycle this falls back to
1036-
// declaration order instead of surfacing mcpp.graph's CycleError.
1037+
// is no longer possible either way -- so the edge that closes a cycle is
1038+
// skipped.
10371039
std::vector<std::size_t> order;
1038-
if (auto ordered = mcpp::graph::topological_order(deps)) {
1039-
order = std::move(*ordered);
1040-
} else {
1041-
order.resize(pending.size());
1042-
std::iota(order.begin(), order.end(), std::size_t{0});
1043-
}
1040+
order = *mcpp::graph::depth_first_order(deps, roots, mcpp::graph::Cycles::Skip);
10441041

10451042
for (auto i : order) push(pending[i].path, std::move(pending[i].name));
10461043
return out;
@@ -1163,39 +1160,24 @@ step6_host_module_registration(PrepareState& state) {
11631160
}
11641161
}
11651162

1166-
// Dependency-first order, so a rule's own host modules are
1167-
// compiled BEFORE it. That ordering is the entire mechanism:
1168-
// the compile loop in build_program.cppm accumulates the
1169-
// module flags as it goes, so each entry sees the BMIs of
1170-
// everything ahead of it, and "a rule may import another
1171-
// rule" needs no second machinery — only this sort.
1172-
//
1173-
// Restricted to `c`'s own reachable closure (not the whole
1174-
// project) and reindexed to a dense local graph before
1175-
// calling mcpp.graph: a cycle elsewhere among packages `c`
1176-
// never reaches must not fail `c`'s build.
1177-
auto reach = mcpp::graph::closure(pkgHostDeps, direct,
1178-
/*include_roots=*/true);
1179-
std::map<std::size_t, std::size_t> local;
1180-
for (std::size_t i = 0; i < reach.size(); ++i)
1181-
local.emplace(reach[i], i);
1182-
mcpp::graph::AdjacencyList sub(reach.size());
1183-
for (std::size_t i = 0; i < reach.size(); ++i)
1184-
for (auto q : pkgHostDeps[reach[i]])
1185-
if (auto it = local.find(q); it != local.end())
1186-
sub[i].push_back(it->second);
1187-
1188-
auto topo = mcpp::graph::topological_order(sub);
1163+
// Post-order DFS, so a rule's own host modules are compiled
1164+
// BEFORE it. That ordering is the entire mechanism: the
1165+
// compile loop in build_program.cppm accumulates the module
1166+
// flags as it goes, so each entry sees the BMIs of everything
1167+
// ahead of it, and "a rule may import another rule" needs no
1168+
// second machinery — only this sort. It walks only what `c`
1169+
// reaches, so a cycle elsewhere never fails `c`'s build.
1170+
auto topo = mcpp::graph::depth_first_order(pkgHostDeps, direct);
11891171
if (!topo) {
11901172
// A cycle, reported AS a cycle and naming the packages
11911173
// on it. A depth limit would answer a different
11921174
// question and would answer it later.
11931175
std::string ring;
11941176
bool first = true;
1195-
for (auto li : topo.error().cycle) {
1177+
for (auto q : topo.error().cycle) {
11961178
if (!first) ring += " -> ";
11971179
first = false;
1198-
ring += identity(reach[li]);
1180+
ring += identity(q);
11991181
}
12001182
return std::unexpected(std::format(
12011183
"build rules form an import cycle: {}\n"
@@ -1205,18 +1187,18 @@ step6_host_module_registration(PrepareState& state) {
12051187
}
12061188

12071189
std::vector<prov::HostModule> ordered;
1208-
for (auto li : *topo) {
1209-
auto p = reach[li];
1190+
for (auto p : *topo) {
12101191
for (auto& hm : units(p)) {
12111192
hm.importable = isDirect.contains(p);
12121193
ordered.push_back(std::move(hm));
12131194
}
12141195
}
12151196

12161197
// Every provider on this consumer's rule closure, transitive
1217-
// ones included -- `reach` is exactly that set, and a rule
1218-
// imported by another rule declares payloads just as directly.
1219-
state.hostModuleProvidersByConsumer[c].assign(reach.begin(), reach.end());
1198+
// ones included, and a rule imported by another rule declares
1199+
// payloads just as directly.
1200+
std::set<std::size_t> reached(topo->begin(), topo->end());
1201+
state.hostModuleProvidersByConsumer[c].assign(reached.begin(), reached.end());
12201202

12211203
if (auto clash = prov::host_module_collision(ordered))
12221204
return std::unexpected(*clash);

‎src/build/stage.cppm‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,9 @@ bool links_on_placement(const std::filesystem::path& p) {
173173
// it and the link can be made, and a copy otherwise (another volume, FAT or
174174
// exFAT, a permission), so the fallback is today's behaviour.
175175
// Returns an empty error_code on success; otherwise the most informative error
176-
// (the in-place one — that's where 1224 / 32 shows up).
176+
// (the in-place one — that's where 1224 / 32 shows up), or the rename's when
177+
// the destination shares its bytes with another name and is not written in
178+
// place.
177179
std::error_code write_once(const std::filesystem::path& src,
178180
const std::filesystem::path& dst) {
179181
auto tmp = temp_sibling(dst);

‎src/cli/cmd_build.cppm‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -328,19 +328,28 @@ export int cmd_build(const mcpplibs::cmdline::ParsedArgs& parsed) {
328328
return r != 0 ? r : rc;
329329
}
330330
const std::size_t hw = std::max(1u, std::thread::hardware_concurrency());
331+
std::set<std::filesystem::path> directories;
332+
for (auto const& c : contexts) directories.insert(c.outputDir.lexically_normal());
331333
for (auto& c : contexts) {
332334
const std::size_t want = c.plan.scheduleNinjaJobs > 0
333335
? static_cast<std::size_t>(c.plan.scheduleNinjaJobs) : hw + 2;
334336
c.plan.scheduleNinjaJobs = static_cast<int>(
335-
std::max<std::size_t>(1, want / contexts.size()));
337+
std::max<std::size_t>(1, want / directories.size()));
336338
}
339+
// Concurrency is across build directories. Two groups whose values
340+
// resolve to one directory (one toolchain spelled two ways) are built
341+
// one after the other in it, since one ninja owns a directory.
342+
std::map<std::filesystem::path, std::vector<std::size_t>> byDirectory;
343+
for (std::size_t i = 0; i < contexts.size(); ++i)
344+
byDirectory[contexts[i].outputDir.lexically_normal()].push_back(i);
337345
std::vector<int> results(contexts.size(), 0);
338346
{
339347
std::vector<std::jthread> builds;
340-
for (std::size_t i = 0; i < contexts.size(); ++i)
341-
builds.emplace_back([&, i] {
342-
results[i] = run_build_with_hooks(contexts[i], verbose, no_cache,
343-
ov.target_triple);
348+
for (auto const& [dir, indices] : byDirectory)
349+
builds.emplace_back([&, indices] {
350+
for (auto i : indices)
351+
results[i] = run_build_with_hooks(contexts[i], verbose, no_cache,
352+
ov.target_triple);
344353
});
345354
}
346355
for (int r : results)

0 commit comments

Comments
 (0)