From 38a01d523a712410ea46ab0adf4cee9b1312cb0c Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Mon, 12 Oct 2026 18:02:10 +0800 Subject: [PATCH 01/16] feat: add runtime memory admission for detached compilers --- modules/platform/src/capacity.cppm | 84 ++++++++++++++++++++++++++ src/build/ninja_backend.cppm | 3 + src/build/plan.cppm | 5 ++ src/build/prepare.cppm | 1 + src/build/prepare/plan.cpp | 16 +++++ src/build/schedule/detach_codegen.cppm | 30 ++++++++- src/cli.cppm | 4 +- src/cli/cmd_build.cppm | 4 ++ src/config.cppm | 12 ++++ tests/unit/test_capacity_jobs.cpp | 20 ++++++ 10 files changed, 176 insertions(+), 3 deletions(-) diff --git a/modules/platform/src/capacity.cppm b/modules/platform/src/capacity.cppm index ba35b224d..4b95d25ac 100644 --- a/modules/platform/src/capacity.cppm +++ b/modules/platform/src/capacity.cppm @@ -58,8 +58,29 @@ struct HostCapacity { unsigned long long availableBytes = 0; // falls back to total when unknown }; +struct MemorySnapshot { + unsigned long long availableBytes = 0; + bool supported = false; +}; + HostCapacity host_capacity(); +// A fresh reading for admission control. Unlike host_capacity(), this is +// intentionally cheap and has no CPU topology work. `supported == false` means +// that callers must use their safe fallback rather than treating zero as free +// memory. +MemorySnapshot memory_snapshot(); + +// Whether a new compiler may be admitted. The baseline is the available +// physical memory observed when the build started. `thresholdPercent` is the +// proportion of that baseline which may be consumed. `hysteresisPercent` is +// the extra margin required before admitting work again after pressure. +bool memory_allows(const MemorySnapshot& now, + unsigned long long baselineBytes, + int thresholdPercent, + bool recovering = false, + int hysteresisPercent = 5); + // Job count for `auto`. See the module comment for why this is not `nproc`. // // cpu_budget = heterogeneous ? physicalCores : logicalCores @@ -128,6 +149,11 @@ HostCapacity host_capacity() { return cap; } +MemorySnapshot memory_snapshot() { + const auto cap = host_capacity(); + return {cap.availableBytes, cap.availableBytes != 0}; +} + #elif defined(__APPLE__) HostCapacity host_capacity() { @@ -163,6 +189,11 @@ HostCapacity host_capacity() { return cap; } +MemorySnapshot memory_snapshot() { + const auto cap = host_capacity(); + return {cap.availableBytes, cap.availableBytes != 0}; +} + #else // Linux and other POSIX static bool read_meminfo_kb(const char* key, unsigned long long& out) { @@ -181,6 +212,35 @@ static bool read_meminfo_kb(const char* key, unsigned long long& out) { return found; } +static bool read_file_u64(const char* path, unsigned long long& out) { + FILE* f = ::fopen(path, "r"); + if (!f) return false; + char value[128]{}; + const bool ok = ::fgets(value, sizeof(value), f) != nullptr; + ::fclose(f); + if (!ok) return false; + if (::strcmp(value, "max\n") == 0 || ::strcmp(value, "max") == 0) return false; + char* end = nullptr; + const auto n = ::strtoull(value, &end, 10); + if (end == value) return false; + out = n; + return true; +} + +static void apply_cgroup_limit(unsigned long long& available) { + // cgroup v2 exposes the container's memory ceiling separately from the + // host's /proc/meminfo. A build must not use memory which the container + // cannot actually obtain. Keep this deliberately conservative: if the + // hierarchy cannot be discovered, the host reading remains valid. + unsigned long long max = 0, current = 0; + if (!read_file_u64("/sys/fs/cgroup/memory.max", max) + || !read_file_u64("/sys/fs/cgroup/memory.current", current) + || max == 0 || current >= max) + return; + const auto limited = max - current; + if (limited < available) available = limited; +} + static int read_cpuinfo_cores() { FILE* f = ::fopen("/proc/cpuinfo", "r"); if (!f) return 0; @@ -239,9 +299,17 @@ HostCapacity host_capacity() { // page cache and would make every warm machine look starved. if (!read_meminfo_kb("MemAvailable:", cap.availableBytes)) cap.availableBytes = cap.totalBytes; + apply_cgroup_limit(cap.availableBytes); return cap; } +MemorySnapshot memory_snapshot() { + unsigned long long available = 0; + bool supported = read_meminfo_kb("MemAvailable:", available); + if (supported) apply_cgroup_limit(available); + return {available, supported && available != 0}; +} + #endif int recommended_jobs(const HostCapacity& cap, unsigned long long perJobBytes, @@ -269,4 +337,20 @@ int recommended_jobs(const HostCapacity& cap, unsigned long long perJobBytes, return jobs; } +bool memory_allows(const MemorySnapshot& now, unsigned long long baselineBytes, + int thresholdPercent, bool recovering, int hysteresisPercent) { + if (!now.supported || baselineBytes == 0) return false; + if (thresholdPercent < 1) thresholdPercent = 1; + if (thresholdPercent > 100) thresholdPercent = 100; + if (hysteresisPercent < 0) hysteresisPercent = 0; + const int margin = recovering + ? (thresholdPercent > hysteresisPercent + ? thresholdPercent - hysteresisPercent : 1) + : thresholdPercent; + // `thresholdPercent` is the consumed portion. At 80%, at least 20% of the + // startup available memory must remain free. + const auto required = baselineBytes * static_cast(100 - margin) / 100; + return now.availableBytes >= required; +} + } // namespace mcpp::platform::capacity diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index bafbe1325..ab27ba089 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -1907,6 +1907,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append("rule cxx_module_bmi\n"); append(" command = $mcpp bmi-compile --bmi $bmi_out --slot $slot" " --self $mcpp --sem $sched_sem --cap $sched_cap" + " --mem-baseline $sched_mem_baseline --mem-threshold $sched_mem_threshold" " --command-file $out.cmd --dep-from $scan_dep --dep-to $out.d\n"); append(std::format( " rspfile = $out.cmd\n" @@ -2931,6 +2932,8 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, e += " scan_dep = " + escape_ninja_path(ddi) + ".dep\n"; e += " sched_sem = .mcpp-sched\n"; e += std::format(" sched_cap = {}\n", plan.scheduleCompilerCap); + e += std::format(" sched_mem_baseline = {}\n", plan.memoryBaselineBytes); + e += std::format(" sched_mem_threshold = {}\n", plan.memoryThresholdPercent); if (auto inc = local_include_flags(cu, dial); !inc.empty()) e += " local_includes =" + inc + "\n"; if (auto fl = join_compile_flags(cu.packageCxxflags); !fl.empty()) diff --git a/src/build/plan.cppm b/src/build/plan.cppm index e4f1aa87c..74ec1c53e 100644 --- a/src/build/plan.cppm +++ b/src/build/plan.cppm @@ -450,6 +450,11 @@ struct BuildPlan { // frontier starves — see the hazard note in schedule/detach_codegen. int scheduleNinjaJobs = 0; int scheduleCompilerCap = 0; + // Runtime memory admission policy. These values are machine facts and are + // deliberately not part of the build fingerprint. + int memoryThresholdPercent = 80; + int memoryCompilerCap = 0; + unsigned long long memoryBaselineBytes = 0; // One immutable snapshot selected before workspace member substitution. // Build/run/test and cache fast paths consume this value; none may re-read // xlings active/current state. diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 822111635..f100f98c9 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -442,6 +442,7 @@ export struct BuildContext { // parser of one file, and because the two readers must not be able to // disagree about the machine. int globalDefaultJobs = 0; + int memoryThresholdPercent = 80; // --strict: degradations reported through mcpp::diag become errors. // Carried on the context because the build's degradations are discovered // during backend emission, i.e. after prepare_build has returned — the diff --git a/src/build/prepare/plan.cpp b/src/build/prepare/plan.cpp index 0253197f5..7de09c2db 100644 --- a/src/build/prepare/plan.cpp +++ b/src/build/prepare/plan.cpp @@ -864,6 +864,13 @@ static void step13_graph_and_schedule(PrepareState& state, BuildContext& ctx) { if (auto c = state.get_cfg(/*requireBootstrap=*/false)) globalDefaultJobs = static_cast((*c)->defaultJobs); ctx.globalDefaultJobs = globalDefaultJobs; + int memoryThresholdPercent = 80; + if (auto c = state.get_cfg(/*requireBootstrap=*/false)) { + memoryThresholdPercent = static_cast((*c)->memoryThresholdPercent); + if (memoryThresholdPercent < 1 || memoryThresholdPercent > 100) + memoryThresholdPercent = 80; + } + ctx.memoryThresholdPercent = memoryThresholdPercent; // Resolve the module-edge schedule ONCE, here, where both the toolchain and // the manifest are in hand. The backend writes the graph in this shape, the @@ -894,6 +901,15 @@ static void step13_graph_and_schedule(PrepareState& state, BuildContext& ctx) { ctx.plan.scheduleTag = std::string(mcpp::build::schedule::to_string(decision.strategy)); ctx.plan.scheduleNinjaJobs = decision.ninjaJobs; ctx.plan.scheduleCompilerCap = decision.compilerCap; + ctx.plan.memoryThresholdPercent = memoryThresholdPercent; + // For ordinary and two-phase edges Ninja's resolved job count is the + // hard ceiling. Detached GCC edges use compilerCap instead. + ctx.plan.memoryCompilerCap = decision.compilerCap > 0 + ? decision.compilerCap + : (decision.ninjaJobs > 0 ? decision.ninjaJobs + : mcpp::platform::capacity::recommended_jobs( + mcpp::platform::capacity::host_capacity())); + ctx.plan.memoryBaselineBytes = mcpp::platform::capacity::host_capacity().availableBytes; mcpp::log::verbose("build", std::format("schedule: {} — {}", ctx.plan.scheduleTag, decision.reason)); diff --git a/src/build/schedule/detach_codegen.cppm b/src/build/schedule/detach_codegen.cppm index 4cff05f59..a3ecc52b4 100644 --- a/src/build/schedule/detach_codegen.cppm +++ b/src/build/schedule/detach_codegen.cppm @@ -82,6 +82,7 @@ export module mcpp.build.schedule.detach_codegen; import std; import mcpp.platform.process; +import mcpp.platform.capacity; import mcpp.build.stage; export namespace mcpp::build::schedule::detach { @@ -98,6 +99,8 @@ struct CompileRequest { // Directory of concurrency tokens. Empty disables the cap (hazard 2). std::filesystem::path semaphore; int maxCompilers{0}; + unsigned long long memoryBaselineBytes{0}; + int memoryThresholdPercent{80}; // The compiler invocation, as ONE shell command line. // // Not a token list: the backend has already joined and quoted the flags for @@ -195,13 +198,34 @@ void write_rc(const std::filesystem::path& slot, int rc) { // The wait is bounded anyway. If a token is somehow still unreleasable, the // honest outcome is a failure that names the directory, not a build that stops // producing output and never returns. -std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap) { +std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap, + unsigned long long baselineBytes, + int thresholdPercent) { if (dir.empty() || cap <= 0) return {}; std::error_code ec; std::filesystem::create_directories(dir, ec); const auto started = std::chrono::steady_clock::now(); constexpr auto kLimit = std::chrono::hours(2); for (;;) { + const auto snapshot = mcpp::platform::capacity::memory_snapshot(); + // If metrics are unavailable, retain the existing jobs/token safety + // rather than turning a platform limitation into an unbounded build. + // A supported reading must pass the admission check before a token is + // created; the token remains held until the supervisor exits. + bool anyToken = false; + if (std::filesystem::exists(dir, ec)) { + for (auto it = std::filesystem::directory_iterator(dir, ec); + !ec && it != std::filesystem::directory_iterator{}; it.increment(ec)) { + anyToken = true; + break; + } + } + if (anyToken && snapshot.supported && baselineBytes != 0 + && !mcpp::platform::capacity::memory_allows( + snapshot, baselineBytes, thresholdPercent)) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + continue; + } for (int i = 0; i < cap; ++i) { const auto tok = dir / std::to_string(i); if (std::filesystem::create_directory(tok, ec) && !ec) return tok; @@ -502,7 +526,9 @@ int compile_release_at_bmi(const CompileRequest& req) { } } - const auto token = acquire_token(req.semaphore, req.maxCompilers); + const auto token = acquire_token(req.semaphore, req.maxCompilers, + req.memoryBaselineBytes, + req.memoryThresholdPercent); // The supervisor reads the SAME argv file rather than receiving the command // on its own command line: one representation, no re-quoting, and no limit diff --git a/src/cli.cppm b/src/cli.cppm index a7e3bbd7a..fc02a7e7c 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -1003,7 +1003,9 @@ int run(int argc, char** argv) { .option(cl::Option("slot").takes_value().value_name("PATH").help("where .log/.rc are kept")) .option(cl::Option("self").takes_value().value_name("PATH").help("path to mcpp, re-invoked as supervisor")) .option(cl::Option("sem").takes_value().value_name("DIR").help("concurrency token directory")) - .option(cl::Option("cap").takes_value().value_name("N").help("max concurrent compilers")) + .option(cl::Option("cap").takes_value().value_name("N").help("max concurrent compilers")) + .option(cl::Option("mem-baseline").takes_value().value_name("BYTES").help("memory baseline")) + .option(cl::Option("mem-threshold").takes_value().value_name("PERCENT").help("memory threshold")) .option(cl::Option("command-file").takes_value().value_name("PATH").help("file holding the compiler command line")) .option(cl::Option("dep-from").takes_value().value_name("PATH").help("scanner depfile to adopt")) .option(cl::Option("dep-to").takes_value().value_name("PATH").help("where ninja expects this edge's depfile")) diff --git a/src/cli/cmd_build.cppm b/src/cli/cmd_build.cppm index 83b2b77a9..2c1850c0c 100644 --- a/src/cli/cmd_build.cppm +++ b/src/cli/cmd_build.cppm @@ -1583,6 +1583,10 @@ export int cmd_bmi_compile(const mcpplibs::cmdline::ParsedArgs& parsed) { req.maxCompilers = 0; if (const auto cap = opt_value(parsed, "cap"); !cap.empty()) std::from_chars(cap.data(), cap.data() + cap.size(), req.maxCompilers); + if (const auto baseline = opt_value(parsed, "mem-baseline"); !baseline.empty()) + std::from_chars(baseline.data(), baseline.data() + baseline.size(), req.memoryBaselineBytes); + if (const auto threshold = opt_value(parsed, "mem-threshold"); !threshold.empty()) + std::from_chars(threshold.data(), threshold.data() + threshold.size(), req.memoryThresholdPercent); req.commandFile = file_path("command-file"); req.depFrom = file_path("dep-from"); req.depTo = file_path("dep-to"); diff --git a/src/config.cppm b/src/config.cppm index f3f6dc395..f840f75d1 100644 --- a/src/config.cppm +++ b/src/config.cppm @@ -118,6 +118,9 @@ struct GlobalConfig { // From config.toml [build] std::int64_t defaultJobs = 0; + // Percentage of the memory available when a build starts that the + // compiler pool may consume. This is machine policy, not project input. + std::int64_t memoryThresholdPercent = 80; // From config.toml [toolchain] (M5.5) // default = "@" e.g. "gcc@15.1.0" @@ -404,6 +407,7 @@ search_ttl_seconds = 3600 [build] default_jobs = 0 +memory_threshold_percent = 80 )"; write_file(path, tmpl); return std::filesystem::exists(path); @@ -773,6 +777,14 @@ std::expected load_or_init( std::chrono::seconds{cfg.indexRefreshTimeoutSeconds}); cfg.searchTtlSeconds = doc->get_int("cache.search_ttl_seconds").value_or(3600); cfg.defaultJobs = doc->get_int("build.default_jobs").value_or(0); + cfg.memoryThresholdPercent = doc->get_int("build.memory_threshold_percent").value_or(80); + if (const char* env = std::getenv("MCPP_MEMORY_THRESHOLD_PERCENT"); env && *env) { + char* end = nullptr; + const auto value = std::strtol(env, &end, 10); + if (end != env && *end == '\0') cfg.memoryThresholdPercent = value; + } + if (cfg.memoryThresholdPercent < 1 || cfg.memoryThresholdPercent > 100) + cfg.memoryThresholdPercent = 80; cfg.defaultToolchain = doc->get_string("toolchain.default").value_or(""); cfg.defaultTarget = doc->get_string("toolchain.default_target").value_or(""); // [xlings.overrides] (mcpp#755), read by the same function the tests read. diff --git a/tests/unit/test_capacity_jobs.cpp b/tests/unit/test_capacity_jobs.cpp index 34d01a9bc..47365b84f 100644 --- a/tests/unit/test_capacity_jobs.cpp +++ b/tests/unit/test_capacity_jobs.cpp @@ -10,6 +10,8 @@ import mcpp.platform.capacity; using mcpp::platform::capacity::HostCapacity; using mcpp::platform::capacity::recommended_jobs; +using mcpp::platform::capacity::MemorySnapshot; +using mcpp::platform::capacity::memory_allows; namespace { constexpr unsigned long long GiB = 1024ull * 1024 * 1024; @@ -81,3 +83,21 @@ TEST(RecommendedJobs, HostQueryIsPlausible) { EXPECT_LE(cap.physicalCores, cap.logicalCores); EXPECT_GE(recommended_jobs(cap), 1); } + +TEST(MemoryAdmission, StopsAtConfiguredConsumption) { + MemorySnapshot now{.availableBytes = 2 * GiB, .supported = true}; + EXPECT_TRUE(memory_allows(now, 10 * GiB, 80)); + now.availableBytes = 2 * GiB - 1; + EXPECT_FALSE(memory_allows(now, 10 * GiB, 80)); +} + +TEST(MemoryAdmission, HysteresisRequiresRecoveryMargin) { + MemorySnapshot now{.availableBytes = 1 * GiB, .supported = true}; + EXPECT_FALSE(memory_allows(now, 10 * GiB, 80, false)); + now.availableBytes = 3 * GiB; + EXPECT_TRUE(memory_allows(now, 10 * GiB, 80, true)); +} + +TEST(MemoryAdmission, UnsupportedReadIsNotPermission) { + EXPECT_FALSE(memory_allows(MemorySnapshot{}, 10 * GiB, 80)); +} From 4d98dff780f384ba6804d0ffdcf062ea934eab3d Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Mon, 12 Oct 2026 20:32:25 +0800 Subject: [PATCH 02/16] fix: stabilize memory admission baseline --- src/build/prepare/plan.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/build/prepare/plan.cpp b/src/build/prepare/plan.cpp index 7de09c2db..f2cd992af 100644 --- a/src/build/prepare/plan.cpp +++ b/src/build/prepare/plan.cpp @@ -909,7 +909,15 @@ static void step13_graph_and_schedule(PrepareState& state, BuildContext& ctx) { : (decision.ninjaJobs > 0 ? decision.ninjaJobs : mcpp::platform::capacity::recommended_jobs( mcpp::platform::capacity::host_capacity())); - ctx.plan.memoryBaselineBytes = mcpp::platform::capacity::host_capacity().availableBytes; + // Keep the value stable across incremental invocations. The raw + // available-memory reading is intentionally noisy; embedding it in + // build.ninja would change the command line on every build and make + // Ninja rebuild every detached module object. A GiB bucket still + // gives admission control a conservative build-start baseline while + // avoiding that false invalidation. + constexpr unsigned long long gib = 1024ULL * 1024ULL * 1024ULL; + const auto available = mcpp::platform::capacity::host_capacity().availableBytes; + ctx.plan.memoryBaselineBytes = (available / gib) * gib; mcpp::log::verbose("build", std::format("schedule: {} — {}", ctx.plan.scheduleTag, decision.reason)); From 6a868c4a10d58de3e2ff0109799b8886d3ffdbef Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Mon, 12 Oct 2026 21:41:52 +0800 Subject: [PATCH 03/16] feat: extend runtime memory admission to compile edges --- docs/04-mcpp-toml.md | 13 +++ docs/zh/04-mcpp-toml.md | 12 +++ src/build/ninja_backend.cppm | 108 ++++++++++++++++++------- src/build/schedule/detach_codegen.cppm | 43 ++++++++++ src/cli.cppm | 34 ++++++++ 5 files changed, 183 insertions(+), 27 deletions(-) diff --git a/docs/04-mcpp-toml.md b/docs/04-mcpp-toml.md index ed0edb8c8..a029c5414 100644 --- a/docs/04-mcpp-toml.md +++ b/docs/04-mcpp-toml.md @@ -937,6 +937,19 @@ with two behaviours has to be. `bmi_schedule` decides when importers are unblocked. +`memory_threshold_percent` is a machine setting in `~/.mcpp/config.toml`, not a +project-manifest field. It defaults to `80` and limits the compiler pool using +the available memory observed at build start. `MCPP_MEMORY_THRESHOLD_PERCENT` +overrides it for one invocation. Ordinary C/C++ edges and both detached GCC and +two-phase Clang module edges use the same admission check; unsupported memory +metrics leave the configured job/compiler cap in force. + +```toml +# ~/.mcpp/config.toml +[build] +memory_threshold_percent = 80 +``` + | value | | |---|---| | `"auto"` | **the default, and it currently means OFF** | diff --git a/docs/zh/04-mcpp-toml.md b/docs/zh/04-mcpp-toml.md index 659917b34..bc27fd739 100644 --- a/docs/zh/04-mcpp-toml.md +++ b/docs/zh/04-mcpp-toml.md @@ -878,6 +878,18 @@ bmi_schedule = "off" # auto (default, = off) | on | off `bmi_schedule` 决定 importer 何时被解除阻塞。 +`memory_threshold_percent` 是 `~/.mcpp/config.toml` 中的本机设置，不是 +工程 manifest 字段。默认值为 `80`，它依据构建开始时观测到的可用内存限制 +编译器池；单次调用可以用 `MCPP_MEMORY_THRESHOLD_PERCENT` 覆盖。普通 +C/C++ 编译边、GCC 分离式模块边和 Clang 两阶段模块边使用同一条准入检查； +不受支持的内存指标不会取消已经配置的 jobs/编译器上限。 + +```toml +# ~/.mcpp/config.toml +[build] +memory_threshold_percent = 80 +``` + | 取值 | | |---|---| | `"auto"` | **默认值，目前意味着关闭** | diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index ab27ba089..5a7d09fd3 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -1839,6 +1839,21 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append(std::format(" rspfile_content ={}{}\n", msvcDeps ? " " + std::string(kUtf8ByteOrderMark) : "", payload)); }; + // Keep ordinary compile edges inside the same admission controller as the + // detached GCC path. The compiler command remains argv-based; only the + // existing shell-free depfile filter is moved into the wrapper. + const bool memoryAdmission = plan.memoryCompilerCap > 0 + && plan.memoryBaselineBytes != 0; + auto admission_prefix = [&]() { + return std::format("$mcpp compile-admit --sem .mcpp-sched --cap {}" + " --mem-baseline {} --mem-threshold {}", + plan.memoryCompilerCap, plan.memoryBaselineBytes, + plan.memoryThresholdPercent); + }; + auto admission_dep = [&]() { + return needsGnuModuleFilter + ? std::string(" --dep-raw $out.d.raw --dep-out $out.d") : std::string{}; + }; // Tell the driver, every time, that this TU is a module interface. // @@ -1957,10 +1972,16 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append("rule cxx_precompile\n"); if constexpr (mcpp::platform::is_windows) { const std::string payload = " $local_includes"; - append(std::format( - " command = {}$cxx{} $cxxflags $unit_cxxflags{}{} {}$in {}$out\n", - win_depfile_filter, rsp_ref(payload), traits.bmiOnlyFlags, - module_src_flags, mmd_flag, dial.outputObjPrefix)); + if (memoryAdmission) + append(std::format( + " command = {} -- $cxx{} $cxxflags $unit_cxxflags{}{} {}$in {}$out\n", + admission_prefix(), rsp_ref(payload), traits.bmiOnlyFlags, + module_src_flags, mmd_flag, dial.outputObjPrefix)); + else + append(std::format( + " command = {}$cxx{} $cxxflags $unit_cxxflags{}{} {}$in {}$out\n", + win_depfile_filter, rsp_ref(payload), traits.bmiOnlyFlags, + module_src_flags, mmd_flag, dial.outputObjPrefix)); append_rspfile(payload); append_cxx_deps(); } else { @@ -1968,15 +1989,22 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, // same reason: ninja's `restat` compares the output's MTIME, and a // compiler that rewrites a byte-identical BMI still moves it. What // suppresses the cascade is putting the old file back. - append(std::format( - " command = " - "if [ -f \"$out\" ]; then cp -p \"$out\" \"$out.bak\"; fi && " - "$cxx $local_includes $cxxflags $unit_cxxflags{}{} {}$in {}$out && " - "if [ -f \"$out.bak\" ] && $mcpp bmi-equal \"$out\" \"$out.bak\"; then " - "mv \"$out.bak\" \"$out\"; " - "else rm -f \"$out.bak\"; fi\n", - traits.bmiOnlyFlags, module_src_flags, mmd_flag, - dial.outputObjPrefix)); + if (memoryAdmission) + append(std::format(" command = {}{} -- $cxx $local_includes" + " $cxxflags $unit_cxxflags{}{} {}$in {}$out\n", + admission_prefix(), admission_dep(), + traits.bmiOnlyFlags, module_src_flags, mmd_flag, + dial.outputObjPrefix)); + else + append(std::format( + " command = " + "if [ -f \"$out\" ]; then cp -p \"$out\" \"$out.bak\"; fi && " + "$cxx $local_includes $cxxflags $unit_cxxflags{}{} {}$in {}$out && " + "if [ -f \"$out.bak\" ] && $mcpp bmi-equal \"$out\" \"$out.bak\"; then " + "mv \"$out.bak\" \"$out\"; " + "else rm -f \"$out.bak\"; fi\n", + traits.bmiOnlyFlags, module_src_flags, mmd_flag, + dial.outputObjPrefix)); append_cxx_deps(); } append(" description = BMI $out\n"); @@ -1989,15 +2017,26 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append("rule cxx_module_object\n"); if constexpr (mcpp::platform::is_windows) { const std::string payload = " $local_includes"; - append(std::format(" command = {}$cxx{} $cxxflags $unit_cxxflags{} {}{}\n", - win_depfile_filter, rsp_ref(payload), module_src_flags, - mmd_flag, compile_tail)); + if (memoryAdmission) + append(std::format(" command = {}{} -- $cxx{} $cxxflags $unit_cxxflags{} {}{}\n", + admission_prefix(), admission_dep(), rsp_ref(payload), + module_src_flags, mmd_flag, compile_tail)); + else + append(std::format(" command = {}$cxx{} $cxxflags $unit_cxxflags{} {}{}\n", + win_depfile_filter, rsp_ref(payload), module_src_flags, + mmd_flag, compile_tail)); append_rspfile(payload); append_cxx_deps(); } else { - append(std::format( - " command = $cxx $local_includes $cxxflags $unit_cxxflags{} {}{}{}\n", - module_src_flags, mmd_flag, compile_tail, mmd_filter)); + if (memoryAdmission) + append(std::format(" command = {}{} -- $cxx $local_includes" + " $cxxflags $unit_cxxflags{} {}{}\n", + admission_prefix(), admission_dep(), + module_src_flags, mmd_flag, compile_tail)); + else + append(std::format( + " command = $cxx $local_includes $cxxflags $unit_cxxflags{} {}{}{}\n", + module_src_flags, mmd_flag, compile_tail, mmd_filter)); append_cxx_deps(); } append(" description = OBJ $out\n"); @@ -2007,14 +2046,24 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append("rule cxx_object\n"); if constexpr (mcpp::platform::is_windows) { const std::string payload = " $local_includes"; - append(std::format(" command = {}$cxx{} $cxxflags $unit_cxxflags {}{}\n", - win_depfile_filter, rsp_ref(payload), mmd_flag, - compile_tail)); + if (memoryAdmission) + append(std::format(" command = {}{} -- $cxx{} $cxxflags $unit_cxxflags {}{}\n", + admission_prefix(), admission_dep(), rsp_ref(payload), + mmd_flag, compile_tail)); + else + append(std::format(" command = {}$cxx{} $cxxflags $unit_cxxflags {}{}\n", + win_depfile_filter, rsp_ref(payload), mmd_flag, compile_tail)); append_rspfile(payload); } else { - append(std::format( - " command = $cxx $local_includes $cxxflags $unit_cxxflags {}{}{}\n", - mmd_flag, compile_tail, mmd_filter)); + if (memoryAdmission) + append(std::format(" command = {}{} -- $cxx $local_includes" + " $cxxflags $unit_cxxflags {}{}\n", + admission_prefix(), admission_dep(), + mmd_flag, compile_tail)); + else + append(std::format( + " command = $cxx $local_includes $cxxflags $unit_cxxflags {}{}{}\n", + mmd_flag, compile_tail, mmd_filter)); } append(" description = OBJ $out\n"); append_cxx_deps(); @@ -2025,8 +2074,13 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, if (need_c_rule) { append("rule c_object\n"); const std::string payload = " $local_includes"; - append(std::format(" command = $cc{} $cflags $unit_cflags {}{}\n", - rsp_ref(payload), c_mmd_flag, compile_tail)); + if (memoryAdmission) + append(std::format(" command = {} -- $cc{} $cflags $unit_cflags {}{}\n", + admission_prefix(), rsp_ref(payload), c_mmd_flag, + compile_tail)); + else + append(std::format(" command = $cc{} $cflags $unit_cflags {}{}\n", + rsp_ref(payload), c_mmd_flag, compile_tail)); append_rspfile(payload); append(" description = CC $out\n"); append_cxx_deps(); diff --git a/src/build/schedule/detach_codegen.cppm b/src/build/schedule/detach_codegen.cppm index a3ecc52b4..6835dee00 100644 --- a/src/build/schedule/detach_codegen.cppm +++ b/src/build/schedule/detach_codegen.cppm @@ -83,6 +83,7 @@ export module mcpp.build.schedule.detach_codegen; import std; import mcpp.platform.process; import mcpp.platform.capacity; +import mcpp.build.depfile; import mcpp.build.stage; export namespace mcpp::build::schedule::detach { @@ -151,6 +152,17 @@ int supervise(const std::filesystem::path& slot, // must not pass on. int await_unit(const std::filesystem::path& slot, const std::filesystem::path& object); +// Run an ordinary compiler edge while holding the same admission token used +// by detached compilation. The command is argv-based so compiler arguments do +// not acquire a second quoting language at the admission boundary. +int run_admitted(const std::vector& command, + const std::filesystem::path& semaphore, + int maxCompilers, + unsigned long long memoryBaselineBytes, + int memoryThresholdPercent, + const std::filesystem::path& depFrom = {}, + const std::filesystem::path& depTo = {}); + } // namespace mcpp::build::schedule::detach // --------------------------------------------------------------------------- @@ -690,4 +702,35 @@ int await_unit(const std::filesystem::path& slot, const std::filesystem::path& o return 0; } +int run_admitted(const std::vector& command, + const std::filesystem::path& semaphore, + int maxCompilers, + unsigned long long memoryBaselineBytes, + int memoryThresholdPercent, + const std::filesystem::path& depFrom, + const std::filesystem::path& depTo) { + if (command.empty()) return 2; + const auto token = acquire_token(semaphore, maxCompilers, + memoryBaselineBytes, memoryThresholdPercent); + bool timedOut = false; + const int rc = mcpp::platform::process::run_exec_deadline( + command, {}, std::chrono::milliseconds(0), &timedOut); + if (!token.empty()) { + std::error_code ec; + std::filesystem::remove(token, ec); + } + if (rc != 0 || depFrom.empty() || depTo.empty()) return rc; + + std::ifstream in(depFrom, std::ios::binary); + if (!in) return 1; + std::string raw{std::istreambuf_iterator{in}, {}}; + std::ofstream out(depTo, std::ios::binary | std::ios::trunc); + if (!out) return 1; + out << mcpp::build::depfile::first_record(raw); + if (!out) return 1; + std::error_code ec; + std::filesystem::remove(depFrom, ec); + return 0; +} + } // namespace mcpp::build::schedule::detach diff --git a/src/cli.cppm b/src/cli.cppm index fc02a7e7c..c4762961d 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -35,6 +35,7 @@ import mcpp.platform; // is_windows — the PATH separator of `__ import mcpp.platform.env; // --offline → MCPP_OFFLINE import mcpp.platform.process; // __action-stamp runs the checked command import mcpp.platform.fs; // __action-stamp writes its stamps +import mcpp.build.schedule.detach_codegen; import mcpp.build.progress; // __action reports its start import mcpp.platform.runtime_search; // linker-wrapper path-injection opt-out import mcpp.ui; @@ -1136,6 +1137,39 @@ int run(int argc, char** argv) { return 0; } + if (std::string_view(argv[1]) == "compile-admit") { + std::string sem, capText, baselineText, thresholdText, depRaw, depOut; + int i = 2; + for (; i < argc && std::string_view(argv[i]) != "--"; ++i) { + const std::string_view a = argv[i]; + if (i + 1 >= argc) return 2; + if (a == "--sem") sem = argv[++i]; + else if (a == "--cap") capText = argv[++i]; + else if (a == "--mem-baseline") baselineText = argv[++i]; + else if (a == "--mem-threshold") thresholdText = argv[++i]; + else if (a == "--dep-raw") depRaw = argv[++i]; + else if (a == "--dep-out") depOut = argv[++i]; + else { + std::println(stderr, "error: compile-admit: unknown option '{}'", a); + return 2; + } + } + if (i + 1 >= argc || sem.empty()) { + std::println(stderr, "error: compile-admit requires --sem and -- "); + return 2; + } + int cap = 0, threshold = 80; + unsigned long long baseline = 0; + std::from_chars(capText.data(), capText.data() + capText.size(), cap); + std::from_chars(baselineText.data(), baselineText.data() + baselineText.size(), baseline); + std::from_chars(thresholdText.data(), thresholdText.data() + thresholdText.size(), threshold); + std::vector command; + for (++i; i < argc; ++i) command.emplace_back(argv[i]); + return mcpp::build::schedule::detach::run_admitted( + command, std::filesystem::path{sem}, cap, baseline, threshold, + std::filesystem::path{depRaw}, std::filesystem::path{depOut}); + } + if (std::string_view(argv[1]) == "__action-stamp" || std::string_view(argv[1]) == "__action") { const bool named = std::string_view(argv[1]) == "__action"; From 4b1b5b1cceab4a988edf11dc57f9542a2edc3086 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Mon, 12 Oct 2026 23:14:56 +0800 Subject: [PATCH 04/16] fix: stabilize runtime memory admission commands --- src/build/ninja_backend.cppm | 6 +++--- src/build/prepare/plan.cpp | 24 ++++++++++++++---------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 5a7d09fd3..491d5ccb9 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -2018,7 +2018,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, if constexpr (mcpp::platform::is_windows) { const std::string payload = " $local_includes"; if (memoryAdmission) - append(std::format(" command = {}{} -- $cxx{} $cxxflags $unit_cxxflags{} {}{}\n", + append(std::format(" command = {}{} -- \"$cxx\"{} $cxxflags $unit_cxxflags{} {}{}\n", admission_prefix(), admission_dep(), rsp_ref(payload), module_src_flags, mmd_flag, compile_tail)); else @@ -2047,7 +2047,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, if constexpr (mcpp::platform::is_windows) { const std::string payload = " $local_includes"; if (memoryAdmission) - append(std::format(" command = {}{} -- $cxx{} $cxxflags $unit_cxxflags {}{}\n", + append(std::format(" command = {}{} -- \"$cxx\"{} $cxxflags $unit_cxxflags {}{}\n", admission_prefix(), admission_dep(), rsp_ref(payload), mmd_flag, compile_tail)); else @@ -2075,7 +2075,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append("rule c_object\n"); const std::string payload = " $local_includes"; if (memoryAdmission) - append(std::format(" command = {} -- $cc{} $cflags $unit_cflags {}{}\n", + append(std::format(" command = {} -- \"$cc\"{} $cflags $unit_cflags {}{}\n", admission_prefix(), rsp_ref(payload), c_mmd_flag, compile_tail)); else diff --git a/src/build/prepare/plan.cpp b/src/build/prepare/plan.cpp index f2cd992af..107643c5b 100644 --- a/src/build/prepare/plan.cpp +++ b/src/build/prepare/plan.cpp @@ -904,20 +904,24 @@ static void step13_graph_and_schedule(PrepareState& state, BuildContext& ctx) { ctx.plan.memoryThresholdPercent = memoryThresholdPercent; // For ordinary and two-phase edges Ninja's resolved job count is the // hard ceiling. Detached GCC edges use compilerCap instead. - ctx.plan.memoryCompilerCap = decision.compilerCap > 0 - ? decision.compilerCap - : (decision.ninjaJobs > 0 ? decision.ninjaJobs - : mcpp::platform::capacity::recommended_jobs( - mcpp::platform::capacity::host_capacity())); + // This cap belongs to the admission controller, not to Ninja's + // user-selected job count. Keeping it machine-derived means the + // generated command line remains identical for `-j 1` and `-j 4`, + // while Ninja still enforces the requested parallelism for ordinary + // edges. + const auto capacity = mcpp::platform::capacity::host_capacity(); + ctx.plan.memoryCompilerCap = std::max(1, capacity.logicalCores); // Keep the value stable across incremental invocations. The raw // available-memory reading is intentionally noisy; embedding it in // build.ninja would change the command line on every build and make - // Ninja rebuild every detached module object. A GiB bucket still - // gives admission control a conservative build-start baseline while - // avoiding that false invalidation. + // Ninja rebuild every detached module object. A coarse bucket gives + // admission control a conservative build-start baseline while + // avoiding that false invalidation when the scheduler changes jobs. constexpr unsigned long long gib = 1024ULL * 1024ULL * 1024ULL; - const auto available = mcpp::platform::capacity::host_capacity().availableBytes; - ctx.plan.memoryBaselineBytes = (available / gib) * gib; + constexpr unsigned long long bucket = 4 * gib; + const auto available = capacity.availableBytes; + ctx.plan.memoryBaselineBytes = std::max( + gib, (available / bucket) * bucket); mcpp::log::verbose("build", std::format("schedule: {} — {}", ctx.plan.scheduleTag, decision.reason)); From cac2ee500f4021660473e1430322036599d0d406 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 13:38:01 +0800 Subject: [PATCH 05/16] fix: read the compiler from behind the admission wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An ordinary compile edge runs through `mcpp compile-admit` now, and ninja echoes that wrapper as the command that failed. `failed_command_in` required a clang driver as the FIRST word of the line, found none, and the note for a compiler that does not support C++20 coroutines on its MSVC ABI stopped appearing — the failure was left unexplained. That is windows-e2e 892 case C: a C++20 program that uses coroutines, built for i686-windows-msvc, exits non-zero with clang's two errors and no note (run 38268962914, windows-e2e 2/3). Cases A and B kept passing because the std module precompile carries its command in mcpp's own message, which no wrapper touches. The driver is now searched for wherever it sits on the line; the target and the standard read from that line are unchanged by the wrapper. Two tests carry the CI output as it arrives through the wrapper, and fail without this. --- src/toolchain/msvc_coroutines.cppm | 21 +++++++++++++--- tests/unit/test_msvc_coroutines.cpp | 37 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/toolchain/msvc_coroutines.cppm b/src/toolchain/msvc_coroutines.cppm index 405171a99..6cd7d9f3b 100644 --- a/src/toolchain/msvc_coroutines.cppm +++ b/src/toolchain/msvc_coroutines.cppm @@ -22,7 +22,9 @@ // // * a compile ninja ran: the output carries the command after `FAILED:` and // the compiler's diagnostics, so the symptom is read from it (`advice`); -// the plan path and the fast path share that one function; +// the plan path and the fast path share that one function. An ordinary +// compile edge is wrapped in mcpp's own admission control, so the command +// on that line is mcpp's and the compiler is read from inside it; // * the std module precompile: its error carries the `command:` line but not // the compiler's diagnostics, which reach the terminal directly (measured // on #781's Windows CI). The answer there rests on the command alone @@ -66,6 +68,8 @@ Symptom symptom_in(std::string_view output); // The last command line in `output` that names a clang driver and a // `--target=`. Double-quoted words are honoured (Windows paths with spaces). +// The driver need not be the first word: an ordinary compile edge runs through +// mcpp's own admission wrapper, which ninja echoes in front of it. std::optional failed_command_in(std::string_view output); // The note, or empty when the output is not this failure. `probe` is called @@ -173,9 +177,20 @@ std::optional failed_command_in(std::string_view output) { for_each_line(output, [&](std::string_view line) { if (auto at = line.find("command: "); at == 0) line.remove_prefix(9); auto words = split_words(line); - if (words.empty() || !names_clang_driver(words.front())) return; + // The driver is not necessarily the FIRST word. An ordinary compile + // edge runs through mcpp's own admission wrapper, and ninja echoes + // that line verbatim on failure: + // + // compile-admit --sem .mcpp-sched --cap N \ + // --mem-baseline B --mem-threshold P -- "" --target=... + // + // The compiler is still on the line, one wrapper in front of it, so + // the driver is looked for wherever it sits. Everything read from the + // line (the target, the standard) is unchanged by the wrapper. + auto driver = std::find_if(words.begin(), words.end(), names_clang_driver); + if (driver == words.end()) return; FailedCommand cmd; - cmd.compiler = words.front(); + cmd.compiler = *driver; for (auto const& w : words) { if (w.starts_with("--target=")) cmd.target = w.substr(9); else if (w.starts_with("-std=")) cmd.standard = w.substr(5); diff --git a/tests/unit/test_msvc_coroutines.cpp b/tests/unit/test_msvc_coroutines.cpp index 9bec32451..b9c76f06f 100644 --- a/tests/unit/test_msvc_coroutines.cpp +++ b/tests/unit/test_msvc_coroutines.cpp @@ -35,6 +35,25 @@ constexpr std::string_view kCoroutineUseFailure = "3 errors generated.\n" "ninja: build stopped: subcommand failed.\n"; +// The same failure as it reaches a 32-bit MSVC build once mcpp's ordinary +// compile edges go through the runtime admission wrapper (PR #803, Windows e2e +// 892 case C, run 38268962914): the driver is one wrapper in from the start of +// the line, so it is read from where it sits. +constexpr std::string_view kCoroutineUseFailureUnderTheAdmissionWrapper = + "[1/1] D:/a/_temp/mcpp-built/mcpp.exe compile-admit --sem .mcpp-sched --cap 4 " + "--mem-baseline 12884901888 --mem-threshold 80 -- " + "\"C:/Users/runneradmin/.mcpp/registry/data/xpkgs/xim-x-llvm/23.1.3/bin/clang++.exe\" " + "@obj/main.o.rsp -std=c++20 -fms-runtime-lib=dll " + "\"-fprebuilt-module-path=D:\\a\\_temp\\coro\\pcm.cache\" -O0 -g " + "--target=i686-pc-windows-msvc " + "-Xmicrosoft-visualc-tools-root \"C:\\Program Files\\Microsoft Visual Studio\\18\\Enterprise\\VC\\Tools\\MSVC\\14.51.36231\" " + "-MMD -MF obj/main.o.d -c src/main.cpp -o obj/main.o\n" + "FAILED: obj/main.o \n" + "D:/a/_temp/coro/src/main.cpp:4:5: error: use of undeclared identifier 'std'\n" + "D:/a/_temp/coro/src/main.cpp:8:12: error: std::coroutine_traits type was not found; include before defining a coroutine\n" + "3 errors generated.\n" + "ninja: build stopped: subcommand failed.\n"; + co::ProbeFn answering(bool predefines, int* calls = nullptr) { return [=](const co::FailedCommand&) -> std::optional { if (calls) ++*calls; @@ -127,6 +146,24 @@ TEST(MsvcCoroutines, CoroutineUseNoteDoesNotOfferCxx20) { EXPECT_NE(note.find("llvm@22.1.8"), std::string::npos); } +TEST(MsvcCoroutines, CommandBehindTheAdmissionWrapperIsStillRead) { + auto cmd = co::failed_command_in(kCoroutineUseFailureUnderTheAdmissionWrapper); + ASSERT_TRUE(cmd.has_value()); + EXPECT_TRUE(cmd->compiler.ends_with("clang++.exe")); + EXPECT_EQ(cmd->target, "i686-pc-windows-msvc"); + EXPECT_EQ(cmd->standard, "c++20"); +} + +TEST(MsvcCoroutines, CoroutineUseBehindTheAdmissionWrapperGetsTheNote) { + EXPECT_EQ(co::symptom_in(kCoroutineUseFailureUnderTheAdmissionWrapper), + co::Symptom::CoroutineUse); + auto note = co::advice(kCoroutineUseFailureUnderTheAdmissionWrapper, answering(false)); + ASSERT_FALSE(note.empty()); + EXPECT_NE(note.find("Code that uses coroutines"), std::string::npos); + EXPECT_EQ(note.find("c++20\""), std::string::npos); + EXPECT_NE(note.find("llvm@22.1.8"), std::string::npos); +} + TEST(MsvcCoroutines, CompilerThatPredefinesTheMacroGetsNoNote) { EXPECT_TRUE(co::advice(kStdModuleFailure, answering(true)).empty()); } From 3a4f473215f472d1e8f9370d1114c0a6cef10b3a Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 17:00:12 +0800 Subject: [PATCH 06/16] fix: preserve BMI restat inside the admission wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review item §1 by backing up the BMI before compilation and restoring its original timestamp when the successful output is equivalent. Cover the wrapper and downstream Ninja restat behavior with regression tests. --- src/build/ninja_backend.cppm | 2 +- src/build/schedule/detach_codegen.cppm | 41 +++++++++++- src/cli.cppm | 6 +- tests/e2e/234_bmi_schedule_on.sh | 4 ++ tests/scripts/test_compile_admit.py | 90 ++++++++++++++++++++++++++ tests/unit/test_ninja_backend.cpp | 15 +++++ 6 files changed, 152 insertions(+), 6 deletions(-) create mode 100644 tests/scripts/test_compile_admit.py diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 491d5ccb9..58b66fb76 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -1990,7 +1990,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, // compiler that rewrites a byte-identical BMI still moves it. What // suppresses the cascade is putting the old file back. if (memoryAdmission) - append(std::format(" command = {}{} -- $cxx $local_includes" + append(std::format(" command = {}{} --bmi-restat \"$out\" -- $cxx $local_includes" " $cxxflags $unit_cxxflags{}{} {}$in {}$out\n", admission_prefix(), admission_dep(), traits.bmiOnlyFlags, module_src_flags, mmd_flag, diff --git a/src/build/schedule/detach_codegen.cppm b/src/build/schedule/detach_codegen.cppm index 6835dee00..371164868 100644 --- a/src/build/schedule/detach_codegen.cppm +++ b/src/build/schedule/detach_codegen.cppm @@ -161,7 +161,8 @@ int run_admitted(const std::vector& command, unsigned long long memoryBaselineBytes, int memoryThresholdPercent, const std::filesystem::path& depFrom = {}, - const std::filesystem::path& depTo = {}); + const std::filesystem::path& depTo = {}, + const std::filesystem::path& bmiRestat = {}); } // namespace mcpp::build::schedule::detach @@ -708,10 +709,33 @@ int run_admitted(const std::vector& command, unsigned long long memoryBaselineBytes, int memoryThresholdPercent, const std::filesystem::path& depFrom, - const std::filesystem::path& depTo) { + const std::filesystem::path& depTo, + const std::filesystem::path& bmiRestat) { if (command.empty()) return 2; const auto token = acquire_token(semaphore, maxCompilers, memoryBaselineBytes, memoryThresholdPercent); + std::error_code ec; + auto backup = bmiRestat; + backup += ".bak"; + bool backedUp = false; + if (!bmiRestat.empty() && file_exists(bmiRestat)) { + const auto oldTime = std::filesystem::last_write_time(bmiRestat, ec); + if (!ec) { + backedUp = std::filesystem::copy_file( + bmiRestat, backup, std::filesystem::copy_options::overwrite_existing, ec); + if (backedUp) { + std::filesystem::last_write_time(backup, oldTime, ec); + backedUp = !ec; + } + } + if (!backedUp) { + std::println(std::cerr, "mcpp: cannot back up BMI {}: {}", + bmiRestat.string(), ec.message()); + if (!token.empty()) std::filesystem::remove(token, ec); + std::filesystem::remove(backup, ec); + return 1; + } + } bool timedOut = false; const int rc = mcpp::platform::process::run_exec_deadline( command, {}, std::chrono::milliseconds(0), &timedOut); @@ -719,6 +743,18 @@ int run_admitted(const std::vector& command, std::error_code ec; std::filesystem::remove(token, ec); } + if (backedUp) { + if (rc == 0 && mcpp::build::stage::bmi_equivalent(bmiRestat, backup)) { + std::filesystem::rename(backup, bmiRestat, ec); + if (ec) { + std::println(std::cerr, "mcpp: cannot restore BMI {}: {}", + bmiRestat.string(), ec.message()); + return 1; + } + } else { + std::filesystem::remove(backup, ec); + } + } if (rc != 0 || depFrom.empty() || depTo.empty()) return rc; std::ifstream in(depFrom, std::ios::binary); @@ -728,7 +764,6 @@ int run_admitted(const std::vector& command, if (!out) return 1; out << mcpp::build::depfile::first_record(raw); if (!out) return 1; - std::error_code ec; std::filesystem::remove(depFrom, ec); return 0; } diff --git a/src/cli.cppm b/src/cli.cppm index c4762961d..050a61ec0 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -1138,7 +1138,7 @@ int run(int argc, char** argv) { } if (std::string_view(argv[1]) == "compile-admit") { - std::string sem, capText, baselineText, thresholdText, depRaw, depOut; + std::string sem, capText, baselineText, thresholdText, depRaw, depOut, bmiRestat; int i = 2; for (; i < argc && std::string_view(argv[i]) != "--"; ++i) { const std::string_view a = argv[i]; @@ -1149,6 +1149,7 @@ int run(int argc, char** argv) { else if (a == "--mem-threshold") thresholdText = argv[++i]; else if (a == "--dep-raw") depRaw = argv[++i]; else if (a == "--dep-out") depOut = argv[++i]; + else if (a == "--bmi-restat") bmiRestat = argv[++i]; else { std::println(stderr, "error: compile-admit: unknown option '{}'", a); return 2; @@ -1167,7 +1168,8 @@ int run(int argc, char** argv) { for (++i; i < argc; ++i) command.emplace_back(argv[i]); return mcpp::build::schedule::detach::run_admitted( command, std::filesystem::path{sem}, cap, baseline, threshold, - std::filesystem::path{depRaw}, std::filesystem::path{depOut}); + std::filesystem::path{depRaw}, std::filesystem::path{depOut}, + std::filesystem::path{bmiRestat}); } if (std::string_view(argv[1]) == "__action-stamp" diff --git a/tests/e2e/234_bmi_schedule_on.sh b/tests/e2e/234_bmi_schedule_on.sh index 5bf3dfc2b..5f25b76d7 100755 --- a/tests/e2e/234_bmi_schedule_on.sh +++ b/tests/e2e/234_bmi_schedule_on.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +# requires: python3 # `bmi_schedule = "on"` end to end, and the token leak that used to hang it. source "$(dirname "${BASH_SOURCE[0]}")/_toolchain_env.sh" # @@ -43,6 +44,9 @@ case "$MCPP" in /*) ;; *) MCPP="$_root/$MCPP" ;; esac [ -x "$MCPP" ] || { echo "FAIL: MCPP=$MCPP is not executable"; exit 1; } export MCPP +# Exercise the admission wrapper independently of the selected toolchain. +python3 "$_root/tests/scripts/test_compile_admit.py" + TMP=$(mktemp -d) trap "rm -rf $TMP" EXIT cd "$TMP" diff --git a/tests/scripts/test_compile_admit.py b/tests/scripts/test_compile_admit.py new file mode 100644 index 000000000..f16b043be --- /dev/null +++ b/tests/scripts/test_compile_admit.py @@ -0,0 +1,90 @@ +"""Exercise the real compile-admit wrapper with bounded subprocess waits.""" +import os +from pathlib import Path +import shutil +import subprocess +import sys +import tempfile +import time +import unittest + + +MCPP = os.environ.get("MCPP", "") + + +@unittest.skipUnless(MCPP, "set MCPP to the freshly built executable") +class CompileAdmit(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory(prefix="mcpp-admit-") + self.addCleanup(self.tmp.cleanup) + self.root = Path(self.tmp.name) + self.sem = self.root / "tokens" + + def argv(self, script, *args, baseline=0, bmi=None): + cmd = [MCPP, "compile-admit", "--sem", str(self.sem), + "--mem-baseline", str(baseline), "--mem-threshold", "1"] + if bmi is not None: + cmd += ["--bmi-restat", str(bmi)] + return cmd + ["--", sys.executable, "-c", script, *map(str, args)] + + def run_compile(self, script, *args, **kwargs): + return subprocess.run(self.argv(script, *args, **kwargs), + capture_output=True, text=True, timeout=10) + + @unittest.skipIf(os.name == "nt", "POSIX BMI restat contract") + def test_bmi_restore_and_failure_cleanup(self): + bmi = self.root / "interface.pcm" + old_time = 1_600_000_000_000_000_000 + writer = "import pathlib,sys; pathlib.Path(sys.argv[1]).write_bytes(sys.argv[2].encode())" + for content, status, expected in (("same", 0, b"same"), + ("changed", 0, b"changed"), + ("failed", 7, b"failed")): + with self.subTest(content=content): + bmi.write_bytes(b"same") + os.utime(bmi, ns=(old_time, old_time)) + result = self.run_compile(writer + f"; sys.exit({status})", + bmi, content, bmi=bmi) + self.assertEqual(result.returncode, status, result.stderr) + self.assertEqual(bmi.read_bytes(), expected) + if content == "same": + self.assertEqual(bmi.stat().st_mtime_ns, old_time) + else: + self.assertNotEqual(bmi.stat().st_mtime_ns, old_time) + self.assertFalse(Path(str(bmi) + ".bak").exists()) + bmi.unlink() + result = self.run_compile(writer, bmi, "new", bmi=bmi) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(bmi.read_bytes(), b"new") + + @unittest.skipIf(os.name == "nt", "POSIX BMI restat contract") + def test_equivalent_bmi_does_not_cascade_in_ninja(self): + ninja = shutil.which("ninja") + if not ninja: + self.skipTest("ninja unavailable") + source = self.root / "source" + source.write_text("same") + helper = self.root / "compile.py" + helper.write_text("from pathlib import Path\nPath('interface.pcm').write_bytes(b'same')\n") + consumer = self.root / "consumer.py" + consumer.write_text("from pathlib import Path\nPath('consumer').touch()\n") + (self.root / "build.ninja").write_text( + f'rule bmi\n command = "{MCPP}" compile-admit --sem tokens ' + f'--mem-baseline 0 --bmi-restat "$out" -- "{sys.executable}" "{helper}"\n' + ' restat = 1\n' + f'rule consume\n command = "{sys.executable}" "{consumer}"\n' + 'build interface.pcm: bmi source\n' + 'build consumer: consume interface.pcm\n') + def build(): + result = subprocess.run([ninja, "-f", "build.ninja"], cwd=self.root, + capture_output=True, text=True, timeout=10) + self.assertEqual(result.returncode, 0, result.stderr) + build() + before = (self.root / "consumer").stat().st_mtime_ns + time.sleep(.05) + source.touch() + build() + self.assertEqual((self.root / "consumer").stat().st_mtime_ns, before) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index a4a9434d5..72a856784 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -2971,3 +2971,18 @@ TEST(NinjaBackend, PeExportsPatternsAndAConsumerReachTheDefEdge) { std::error_code ec; std::filesystem::remove_all(plan.outputDir, ec); } + +TEST(NinjaBackend, AdmittedTwoPhaseBmiPreservesRestat) { + if constexpr (mcpp::platform::is_windows) + GTEST_SKIP() << "POSIX BMI restat contract"; + auto plan = minimal_plan(); + plan.toolchain.compiler = mcpp::toolchain::CompilerId::Clang; + plan.toolchain.version = "23.1.3"; + plan.scanDepsPath = "/selected/bin/clang-scan-deps"; + plan.scheduleTag = "two-phase"; + plan.memoryCompilerCap = 8; + plan.memoryBaselineBytes = 4ULL * 1024 * 1024 * 1024; + const auto ninja = emit_ninja_string(plan); + ASSERT_NE(ninja.find("rule cxx_precompile\n"), std::string::npos); + EXPECT_NE(ninja.find("--bmi-restat \"$out\" -- $cxx"), std::string::npos); +} From d6fc22e782b3d87b61245e561860d2988e82c3f4 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 17:00:36 +0800 Subject: [PATCH 07/16] fix: let Ninja bound ordinary compile parallelism MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review item §2 by removing counting slots from ordinary and two-phase compile admission. Retain memory gating with a shared single-slot fallback, include memory waits in timeout accounting, and reclaim stale fallback tokens for every build strategy. Update regression coverage and both language manuals. --- docs/04-mcpp-toml.md | 7 ++-- docs/zh/04-mcpp-toml.md | 4 ++- src/build/ninja_backend.cppm | 11 +++--- src/build/plan.cppm | 1 - src/build/prepare/plan.cpp | 6 ---- src/build/schedule/detach_codegen.cppm | 46 ++++++++++++++------------ src/build/schedule/policy.cppm | 4 +-- tests/e2e/234_bmi_schedule_on.sh | 12 ++----- tests/scripts/test_compile_admit.py | 40 ++++++++++++++++++++++ tests/unit/test_ninja_backend.cpp | 10 +++++- tests/unit/test_schedule_policy.cpp | 2 +- 11 files changed, 91 insertions(+), 52 deletions(-) diff --git a/docs/04-mcpp-toml.md b/docs/04-mcpp-toml.md index a029c5414..4020b9342 100644 --- a/docs/04-mcpp-toml.md +++ b/docs/04-mcpp-toml.md @@ -941,8 +941,11 @@ with two behaviours has to be. project-manifest field. It defaults to `80` and limits the compiler pool using the available memory observed at build start. `MCPP_MEMORY_THRESHOLD_PERCENT` overrides it for one invocation. Ordinary C/C++ edges and both detached GCC and -two-phase Clang module edges use the same admission check; unsupported memory -metrics leave the configured job/compiler cap in force. +two-phase Clang module edges use the same admission check. Ninja's jobs setting controls ordinary and +two-phase edge parallelism; detached GCC compilation retains its compiler count +cap. Under memory pressure, a shared single fallback slot still admits one +compiler. Unsupported memory metrics leave the configured job/compiler cap in +force. ```toml # ~/.mcpp/config.toml diff --git a/docs/zh/04-mcpp-toml.md b/docs/zh/04-mcpp-toml.md index bc27fd739..987f9f1d2 100644 --- a/docs/zh/04-mcpp-toml.md +++ b/docs/zh/04-mcpp-toml.md @@ -882,7 +882,9 @@ bmi_schedule = "off" # auto (default, = off) | on | off 工程 manifest 字段。默认值为 `80`，它依据构建开始时观测到的可用内存限制 编译器池；单次调用可以用 `MCPP_MEMORY_THRESHOLD_PERCENT` 覆盖。普通 C/C++ 编译边、GCC 分离式模块边和 Clang 两阶段模块边使用同一条准入检查； -不受支持的内存指标不会取消已经配置的 jobs/编译器上限。 +普通边和两阶段边的并行度由 Ninja 的 jobs 设置 +决定，GCC 分离式编译保留编译器数量上限。内存不足时，共享的单槽兜底仍可 +放行一个编译。不受支持的内存指标不会取消已经配置的 jobs/编译器上限。 ```toml # ~/.mcpp/config.toml diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 58b66fb76..3e0bb4c17 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -1842,12 +1842,11 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, // Keep ordinary compile edges inside the same admission controller as the // detached GCC path. The compiler command remains argv-based; only the // existing shell-free depfile filter is moved into the wrapper. - const bool memoryAdmission = plan.memoryCompilerCap > 0 - && plan.memoryBaselineBytes != 0; + const bool memoryAdmission = plan.memoryBaselineBytes != 0; auto admission_prefix = [&]() { - return std::format("$mcpp compile-admit --sem .mcpp-sched --cap {}" + return std::format("$mcpp compile-admit --sem .mcpp-sched" " --mem-baseline {} --mem-threshold {}", - plan.memoryCompilerCap, plan.memoryBaselineBytes, + plan.memoryBaselineBytes, plan.memoryThresholdPercent); }; auto admission_dep = [&]() { @@ -4203,7 +4202,7 @@ namespace { // RECLAIM THE STALE CONCURRENCY TOKENS, ONCE PER BUILD DIRECTORY PER COMMAND. // -// detach-codegen bounds real compiler concurrency with a semaphore of +// Detached compiler caps and the memory-pressure fallback use a semaphore of // directories under `/.mcpp-sched`, released by the supervisor // holding each token. A supervisor that never runs its cleanup (Ctrl-C on the // build, the OOM killer, a reboot) leaves its directory behind, and nothing @@ -4270,7 +4269,7 @@ std::expected NinjaBackend::build(const BuildPlan& plan return std::unexpected(BuildError{std::format("cannot create output dir '{}': {}", plan.outputDir.string(), ec.message()), plan.outputDir}); - if (!opts.dryRun && plan.scheduleTag == "detach-codegen") + if (!opts.dryRun) reclaim_stale_schedule_tokens(plan.outputDir); auto ninja_path = plan.outputDir / "build.ninja"; diff --git a/src/build/plan.cppm b/src/build/plan.cppm index 74ec1c53e..7d46b06cf 100644 --- a/src/build/plan.cppm +++ b/src/build/plan.cppm @@ -453,7 +453,6 @@ struct BuildPlan { // Runtime memory admission policy. These values are machine facts and are // deliberately not part of the build fingerprint. int memoryThresholdPercent = 80; - int memoryCompilerCap = 0; unsigned long long memoryBaselineBytes = 0; // One immutable snapshot selected before workspace member substitution. // Build/run/test and cache fast paths consume this value; none may re-read diff --git a/src/build/prepare/plan.cpp b/src/build/prepare/plan.cpp index 107643c5b..5040683dd 100644 --- a/src/build/prepare/plan.cpp +++ b/src/build/prepare/plan.cpp @@ -904,13 +904,7 @@ static void step13_graph_and_schedule(PrepareState& state, BuildContext& ctx) { ctx.plan.memoryThresholdPercent = memoryThresholdPercent; // For ordinary and two-phase edges Ninja's resolved job count is the // hard ceiling. Detached GCC edges use compilerCap instead. - // This cap belongs to the admission controller, not to Ninja's - // user-selected job count. Keeping it machine-derived means the - // generated command line remains identical for `-j 1` and `-j 4`, - // while Ninja still enforces the requested parallelism for ordinary - // edges. const auto capacity = mcpp::platform::capacity::host_capacity(); - ctx.plan.memoryCompilerCap = std::max(1, capacity.logicalCores); // Keep the value stable across incremental invocations. The raw // available-memory reading is intentionally noisy; embedding it in // build.ninja would change the command line on every build and make diff --git a/src/build/schedule/detach_codegen.cppm b/src/build/schedule/detach_codegen.cppm index 371164868..71476bf32 100644 --- a/src/build/schedule/detach_codegen.cppm +++ b/src/build/schedule/detach_codegen.cppm @@ -152,9 +152,9 @@ int supervise(const std::filesystem::path& slot, // must not pass on. int await_unit(const std::filesystem::path& slot, const std::filesystem::path& object); -// Run an ordinary compiler edge while holding the same admission token used -// by detached compilation. The command is argv-based so compiler arguments do -// not acquire a second quoting language at the admission boundary. +// Run an ordinary compiler edge through memory admission. A nonpositive cap +// skips counting slots; under pressure a single shared slot guarantees progress. +// The command is argv-based so compiler arguments do not acquire a second quoting language at the admission boundary. int run_admitted(const std::vector& command, const std::filesystem::path& semaphore, int maxCompilers, @@ -214,34 +214,36 @@ void write_rc(const std::filesystem::path& slot, int rc) { std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap, unsigned long long baselineBytes, int thresholdPercent) { - if (dir.empty() || cap <= 0) return {}; + if (dir.empty()) return {}; std::error_code ec; std::filesystem::create_directories(dir, ec); const auto started = std::chrono::steady_clock::now(); constexpr auto kLimit = std::chrono::hours(2); for (;;) { const auto snapshot = mcpp::platform::capacity::memory_snapshot(); - // If metrics are unavailable, retain the existing jobs/token safety - // rather than turning a platform limitation into an unbounded build. - // A supported reading must pass the admission check before a token is - // created; the token remains held until the supervisor exits. - bool anyToken = false; - if (std::filesystem::exists(dir, ec)) { + const bool denied = snapshot.supported && baselineBytes != 0 + && !mcpp::platform::capacity::memory_allows( + snapshot, baselineBytes, thresholdPercent); + if (cap <= 0) { + // Ninja alone bounds ordinary edges. Only memory denial uses a + // shared fallback slot, allowing one compiler to make progress even + // when external processes keep available memory below the reserve. + if (!denied) return {}; + const auto tok = dir / "0"; + if (std::filesystem::create_directory(tok, ec) && !ec) return tok; + } else { + bool anyToken = false; for (auto it = std::filesystem::directory_iterator(dir, ec); !ec && it != std::filesystem::directory_iterator{}; it.increment(ec)) { anyToken = true; break; } - } - if (anyToken && snapshot.supported && baselineBytes != 0 - && !mcpp::platform::capacity::memory_allows( - snapshot, baselineBytes, thresholdPercent)) { - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - continue; - } - for (int i = 0; i < cap; ++i) { - const auto tok = dir / std::to_string(i); - if (std::filesystem::create_directory(tok, ec) && !ec) return tok; + if (!denied || !anyToken) { + for (int i = 0; i < cap; ++i) { + const auto tok = dir / std::to_string(i); + if (std::filesystem::create_directory(tok, ec) && !ec) return tok; + } + } } if (std::chrono::steady_clock::now() - started > kLimit) { // Proceeds WITHOUT the cap rather than failing: the semaphore bounds @@ -251,10 +253,10 @@ std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap, "mcpp: no compiler slot became free in 2h — all {} tokens in {} " "are held by processes that are gone. Continuing without the " "concurrency cap; remove that directory to restore it.", - cap, dir.string()); + std::max(1, cap), dir.string()); return {}; } - std::this_thread::sleep_for(std::chrono::milliseconds(5)); + std::this_thread::sleep_for(std::chrono::milliseconds(denied ? 50 : 5)); } } diff --git a/src/build/schedule/policy.cppm b/src/build/schedule/policy.cppm index 428080c55..814248883 100644 --- a/src/build/schedule/policy.cppm +++ b/src/build/schedule/policy.cppm @@ -169,8 +169,8 @@ int resolve_jobs(const manifest::Manifest& m, // UNDER DetachCodegen, `hostJobs == 0` MEANS NO BOUND AT ALL. A detached // compiler stops holding a ninja slot the moment it publishes its BMI, so // ninja's -j is no longer a limit on how many compilers are running — the -// semaphore is. A cap of 0 disables the semaphore (`acquire_token` returns -// immediately), and the graph then goes out with `sched_cap = 0`: ninja keeps +// semaphore is. A cap of 0 disables counting slots, and the graph then goes +// out with `sched_cap = 0`: when memory allows, ninja keeps // starting compiles as fast as BMIs appear, with nothing counting them. That is // hazard 2 in detach_codegen, and it was live in the DEFAULT configuration — // nobody passing `--jobs` got any bound, on a workload whose single compile diff --git a/tests/e2e/234_bmi_schedule_on.sh b/tests/e2e/234_bmi_schedule_on.sh index 5f25b76d7..ad4826e14 100755 --- a/tests/e2e/234_bmi_schedule_on.sh +++ b/tests/e2e/234_bmi_schedule_on.sh @@ -89,9 +89,7 @@ import schedon.mid; int main() { std::println("{}", mid_value()); return mid_value() == 8 ? 0 : 1; } EOF -# detach-codegen is the GCC strategy. On a machine whose default toolchain -# resolves to clang the decision is TwoPhase and there is no semaphore at all, -# so the second contract has nothing to test — say so rather than pass hollowly. +# Both split strategies use the shared memory-pressure fallback directory. say_skip_reason() { echo "SKIP: $1"; exit 0; } echo "== 1. builds with bmi_schedule = \"on\" ==" @@ -130,12 +128,6 @@ grep -q '^# mcpp:graph=' "$BUILD_DIR/build.ninja" || { echo "FAIL: build.ninja carries no '# mcpp:graph=' line"; exit 1; } echo " $(grep -m1 '^# mcpp:graph=' "$BUILD_DIR/build.ninja")" -if [ "$strategy" != detach ]; then - echo "== 3. skipped: no semaphore under the $strategy strategy ==" - echo "234 bmi_schedule OK ($strategy)" - exit 0 -fi - echo "== 3. an incremental build still works ==" touch src/core.cppm "$MCPP" build --release > inc.log 2>&1 || { @@ -193,4 +185,4 @@ BIN=$(ls -t target/*/*/bin/schedon 2>/dev/null | head -1) out=$("$BIN") [ "$out" = "8" ] || { echo "FAIL: binary printed '$out' after the stale-token build"; exit 1; } -echo "234 bmi_schedule OK (detach-codegen, tokens reclaimed)" +echo "234 bmi_schedule OK ($strategy, tokens reclaimed)" diff --git a/tests/scripts/test_compile_admit.py b/tests/scripts/test_compile_admit.py index f16b043be..e14c5e430 100644 --- a/tests/scripts/test_compile_admit.py +++ b/tests/scripts/test_compile_admit.py @@ -31,6 +31,46 @@ def run_compile(self, script, *args, **kwargs): return subprocess.run(self.argv(script, *args, **kwargs), capture_output=True, text=True, timeout=10) + def test_allowed_memory_does_not_acquire_counting_slots(self): + (self.sem / "0").mkdir(parents=True) + result = self.run_compile("pass") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(list(self.sem.iterdir()), [self.sem / "0"]) + + def test_pressure_admits_one_and_releases_fallback(self): + ready = self.root / "ready" + release = self.root / "release" + done = self.root / "done" + script = ("import pathlib,sys,time; " + "pathlib.Path(sys.argv[1]).touch(); deadline=time.monotonic()+10; " + "\nwhile not pathlib.Path(sys.argv[2]).exists() and time.monotonic() Date: Tue, 13 Oct 2026 17:01:12 +0800 Subject: [PATCH 08/16] fix: round the memory admission baseline upward MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review item §3 by rounding observed available memory up to a 4 GiB bucket, retaining the unknown-memory fallback and saturating at the integer limit. Add boundary coverage and document upward quantization in both language manuals. --- docs/04-mcpp-toml.md | 3 ++- docs/zh/04-mcpp-toml.md | 2 +- modules/platform/src/capacity.cppm | 15 +++++++++++++++ src/build/prepare/plan.cpp | 15 ++++----------- tests/unit/test_capacity_jobs.cpp | 11 +++++++++++ 5 files changed, 33 insertions(+), 13 deletions(-) diff --git a/docs/04-mcpp-toml.md b/docs/04-mcpp-toml.md index 4020b9342..7c8a5a990 100644 --- a/docs/04-mcpp-toml.md +++ b/docs/04-mcpp-toml.md @@ -941,7 +941,8 @@ with two behaviours has to be. project-manifest field. It defaults to `80` and limits the compiler pool using the available memory observed at build start. `MCPP_MEMORY_THRESHOLD_PERCENT` overrides it for one invocation. Ordinary C/C++ edges and both detached GCC and -two-phase Clang module edges use the same admission check. Ninja's jobs setting controls ordinary and +two-phase Clang module edges use the same admission check. The admission baseline +is rounded up to a 4 GiB bucket. Ninja's jobs setting controls ordinary and two-phase edge parallelism; detached GCC compilation retains its compiler count cap. Under memory pressure, a shared single fallback slot still admits one compiler. Unsupported memory metrics leave the configured job/compiler cap in diff --git a/docs/zh/04-mcpp-toml.md b/docs/zh/04-mcpp-toml.md index 987f9f1d2..3a35f3ec0 100644 --- a/docs/zh/04-mcpp-toml.md +++ b/docs/zh/04-mcpp-toml.md @@ -882,7 +882,7 @@ bmi_schedule = "off" # auto (default, = off) | on | off 工程 manifest 字段。默认值为 `80`，它依据构建开始时观测到的可用内存限制 编译器池；单次调用可以用 `MCPP_MEMORY_THRESHOLD_PERCENT` 覆盖。普通 C/C++ 编译边、GCC 分离式模块边和 Clang 两阶段模块边使用同一条准入检查； -普通边和两阶段边的并行度由 Ninja 的 jobs 设置 +准入基线按 4 GiB 向上取整。普通边和两阶段边的并行度由 Ninja 的 jobs 设置 决定，GCC 分离式编译保留编译器数量上限。内存不足时，共享的单槽兜底仍可 放行一个编译。不受支持的内存指标不会取消已经配置的 jobs/编译器上限。 diff --git a/modules/platform/src/capacity.cppm b/modules/platform/src/capacity.cppm index 4b95d25ac..1900c2d0d 100644 --- a/modules/platform/src/capacity.cppm +++ b/modules/platform/src/capacity.cppm @@ -71,6 +71,9 @@ HostCapacity host_capacity(); // memory. MemorySnapshot memory_snapshot(); +// Round observed available memory up to a 4 GiB bucket (1 GiB if unknown). +unsigned long long memory_baseline(unsigned long long availableBytes); + // Whether a new compiler may be admitted. The baseline is the available // physical memory observed when the build started. `thresholdPercent` is the // proportion of that baseline which may be consumed. `hysteresisPercent` is @@ -337,6 +340,18 @@ int recommended_jobs(const HostCapacity& cap, unsigned long long perJobBytes, return jobs; } +unsigned long long memory_baseline(unsigned long long availableBytes) { + constexpr unsigned long long gib = 1024ULL * 1024ULL * 1024ULL; + constexpr unsigned long long bucket = 4 * gib; + if (availableBytes == 0) return gib; + const auto remainder = availableBytes % bucket; + if (remainder == 0) return availableBytes; + const auto increment = bucket - remainder; + // Saturate rather than wrap if an observation is near the integer limit. + constexpr auto maximum = ~0ULL; + return availableBytes > maximum - increment ? maximum : availableBytes + increment; +} + bool memory_allows(const MemorySnapshot& now, unsigned long long baselineBytes, int thresholdPercent, bool recovering, int hysteresisPercent) { if (!now.supported || baselineBytes == 0) return false; diff --git a/src/build/prepare/plan.cpp b/src/build/prepare/plan.cpp index 5040683dd..49fa097b3 100644 --- a/src/build/prepare/plan.cpp +++ b/src/build/prepare/plan.cpp @@ -905,17 +905,10 @@ static void step13_graph_and_schedule(PrepareState& state, BuildContext& ctx) { // For ordinary and two-phase edges Ninja's resolved job count is the // hard ceiling. Detached GCC edges use compilerCap instead. const auto capacity = mcpp::platform::capacity::host_capacity(); - // Keep the value stable across incremental invocations. The raw - // available-memory reading is intentionally noisy; embedding it in - // build.ninja would change the command line on every build and make - // Ninja rebuild every detached module object. A coarse bucket gives - // admission control a conservative build-start baseline while - // avoiding that false invalidation when the scheduler changes jobs. - constexpr unsigned long long gib = 1024ULL * 1024ULL * 1024ULL; - constexpr unsigned long long bucket = 4 * gib; - const auto available = capacity.availableBytes; - ctx.plan.memoryBaselineBytes = std::max( - gib, (available / bucket) * bucket); + // Round up to a stable bucket: the admission reserve must not be + // smaller than the configured fraction of observed available memory. + ctx.plan.memoryBaselineBytes = + mcpp::platform::capacity::memory_baseline(capacity.availableBytes); mcpp::log::verbose("build", std::format("schedule: {} — {}", ctx.plan.scheduleTag, decision.reason)); diff --git a/tests/unit/test_capacity_jobs.cpp b/tests/unit/test_capacity_jobs.cpp index 47365b84f..bfc251219 100644 --- a/tests/unit/test_capacity_jobs.cpp +++ b/tests/unit/test_capacity_jobs.cpp @@ -101,3 +101,14 @@ TEST(MemoryAdmission, HysteresisRequiresRecoveryMargin) { TEST(MemoryAdmission, UnsupportedReadIsNotPermission) { EXPECT_FALSE(memory_allows(MemorySnapshot{}, 10 * GiB, 80)); } + +TEST(MemoryAdmission, BaselineRoundsUpToTheNextBucket) { + using mcpp::platform::capacity::memory_baseline; + EXPECT_EQ(memory_baseline(0), GiB); + EXPECT_EQ(memory_baseline(1), 4 * GiB); + EXPECT_EQ(memory_baseline(3 * GiB), 4 * GiB); + EXPECT_EQ(memory_baseline(4 * GiB), 4 * GiB); + EXPECT_EQ(memory_baseline(4 * GiB + 1), 8 * GiB); + EXPECT_EQ(memory_baseline(7 * GiB), 8 * GiB); + EXPECT_EQ(memory_baseline(~0ULL), ~0ULL); +} From 9df269c83732a3c4ee6c57df51789df2a4e049f7 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 18:10:59 +0800 Subject: [PATCH 09/16] fix: repair cross-platform admission CI --- src/build/schedule/detach_codegen.cppm | 2 +- tests/e2e/234_bmi_schedule_on.sh | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/build/schedule/detach_codegen.cppm b/src/build/schedule/detach_codegen.cppm index 71476bf32..9b70b81b1 100644 --- a/src/build/schedule/detach_codegen.cppm +++ b/src/build/schedule/detach_codegen.cppm @@ -253,7 +253,7 @@ std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap, "mcpp: no compiler slot became free in 2h — all {} tokens in {} " "are held by processes that are gone. Continuing without the " "concurrency cap; remove that directory to restore it.", - std::max(1, cap), dir.string()); + (std::max)(1, cap), dir.string()); return {}; } std::this_thread::sleep_for(std::chrono::milliseconds(denied ? 50 : 5)); diff --git a/tests/e2e/234_bmi_schedule_on.sh b/tests/e2e/234_bmi_schedule_on.sh index ad4826e14..2f05322ef 100755 --- a/tests/e2e/234_bmi_schedule_on.sh +++ b/tests/e2e/234_bmi_schedule_on.sh @@ -111,7 +111,7 @@ case "$sched_line" in this test covers the split strategies" ;; esac -BIN=$(ls -t target/*/*/bin/schedon 2>/dev/null | head -1) +BIN=$(bash "$_root/.github/tools/newest_artifact.sh" target schedon) [ -n "$BIN" ] || { echo "FAIL: no binary produced"; exit 1; } out=$("$BIN") [ "$out" = "8" ] || { echo "FAIL: binary printed '$out', expected 8"; exit 1; } @@ -158,8 +158,14 @@ printf '\nexport int core_extra() { return 1; }\n' >> src/core.cppm # `set -e` has to come off for exactly this command: with errexit on, a failing # build aborts the script before `rc=$?` runs, which is what pushed the previous # version into the `if !` form that then could not read the status at all. +# Use the suite's process-tree timeout fallback on macOS, where neither GNU +# timeout nor gtimeout is installed. python3 is a declared capability above. +TIMEOUT_CMD=(python3 "$_root/tests/e2e/_timeout.py") +if command -v timeout >/dev/null 2>&1; then TIMEOUT_CMD=(timeout) +elif command -v gtimeout >/dev/null 2>&1; then TIMEOUT_CMD=(gtimeout) +fi set +e -timeout 600 "$MCPP" build --release > stale.log 2>&1 +"${TIMEOUT_CMD[@]}" 600 "$MCPP" build --release > stale.log 2>&1 rc=$? set -e if [ "$rc" != 0 ]; then @@ -181,7 +187,7 @@ left=$(ls "$SEM" 2>/dev/null | wc -l) exit 1; } echo " reclaimed: $planted -> $left" -BIN=$(ls -t target/*/*/bin/schedon 2>/dev/null | head -1) +BIN=$(bash "$_root/.github/tools/newest_artifact.sh" target schedon) out=$("$BIN") [ "$out" = "8" ] || { echo "FAIL: binary printed '$out' after the stale-token build"; exit 1; } From 92daf0c61d05fa0e5dee5a3213f05c61e6b41254 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 19:03:56 +0800 Subject: [PATCH 10/16] fix: make admission regression portable on Windows --- tests/scripts/test_compile_admit.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/scripts/test_compile_admit.py b/tests/scripts/test_compile_admit.py index e14c5e430..ff7740509 100644 --- a/tests/scripts/test_compile_admit.py +++ b/tests/scripts/test_compile_admit.py @@ -41,10 +41,20 @@ def test_pressure_admits_one_and_releases_fallback(self): ready = self.root / "ready" release = self.root / "release" done = self.root / "done" - script = ("import pathlib,sys,time; " - "pathlib.Path(sys.argv[1]).touch(); deadline=time.monotonic()+10; " - "\nwhile not pathlib.Path(sys.argv[2]).exists() and time.monotonic() Date: Tue, 13 Oct 2026 20:14:53 +0800 Subject: [PATCH 11/16] fix: carry memory denial state into admission hysteresis --- src/build/schedule/detach_codegen.cppm | 4 +++- tests/unit/test_capacity_jobs.cpp | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/build/schedule/detach_codegen.cppm b/src/build/schedule/detach_codegen.cppm index 9b70b81b1..5ccb1559c 100644 --- a/src/build/schedule/detach_codegen.cppm +++ b/src/build/schedule/detach_codegen.cppm @@ -219,11 +219,13 @@ std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap, std::filesystem::create_directories(dir, ec); const auto started = std::chrono::steady_clock::now(); constexpr auto kLimit = std::chrono::hours(2); + bool wasDenied = false; for (;;) { const auto snapshot = mcpp::platform::capacity::memory_snapshot(); const bool denied = snapshot.supported && baselineBytes != 0 && !mcpp::platform::capacity::memory_allows( - snapshot, baselineBytes, thresholdPercent); + snapshot, baselineBytes, thresholdPercent, wasDenied); + if (denied) wasDenied = true; if (cap <= 0) { // Ninja alone bounds ordinary edges. Only memory denial uses a // shared fallback slot, allowing one compiler to make progress even diff --git a/tests/unit/test_capacity_jobs.cpp b/tests/unit/test_capacity_jobs.cpp index bfc251219..1cac8b294 100644 --- a/tests/unit/test_capacity_jobs.cpp +++ b/tests/unit/test_capacity_jobs.cpp @@ -92,10 +92,9 @@ TEST(MemoryAdmission, StopsAtConfiguredConsumption) { } TEST(MemoryAdmission, HysteresisRequiresRecoveryMargin) { - MemorySnapshot now{.availableBytes = 1 * GiB, .supported = true}; - EXPECT_FALSE(memory_allows(now, 10 * GiB, 80, false)); - now.availableBytes = 3 * GiB; - EXPECT_TRUE(memory_allows(now, 10 * GiB, 80, true)); + MemorySnapshot now{.availableBytes = 2 * GiB + 1, .supported = true}; + EXPECT_TRUE(memory_allows(now, 10 * GiB, 80, false)); + EXPECT_FALSE(memory_allows(now, 10 * GiB, 80, true)); } TEST(MemoryAdmission, UnsupportedReadIsNotPermission) { From b601e71b89848e926c4355e51dc7d03c53ceff4b Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 20:21:13 +0800 Subject: [PATCH 12/16] fix: pass memory baseline through Ninja environment --- src/build/execute.cppm | 18 ++++++++++++++++++ src/build/ninja_backend.cppm | 12 +++++++----- src/cli.cppm | 7 ++++++- src/cli/cmd_build.cppm | 4 ++++ src/toolchain/msvc_coroutines.cppm | 2 +- tests/unit/test_msvc_coroutines.cpp | 2 +- tests/unit/test_ninja_backend.cpp | 4 +++- 7 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/build/execute.cppm b/src/build/execute.cppm index 91b61c73e..f1503fccf 100644 --- a/src/build/execute.cppm +++ b/src/build/execute.cppm @@ -1367,6 +1367,22 @@ export bool dep_sources_newer_than(const std::vector& depSourceRo // should abandon the fast path and fall back to a full prepare_build — or // an exit code otherwise (0 success; 1 hard failure, diagnostics already // printed to stderr). +std::optional read_ninja_memory_baseline( + const std::filesystem::path& ninjaPath) { + std::ifstream in(ninjaPath); + std::string line; + constexpr std::string_view prefix = "mcpp_mem_baseline = "; + while (std::getline(in, line)) { + if (!line.starts_with(prefix)) continue; + unsigned long long value = 0; + const auto text = std::string_view(line).substr(prefix.size()); + const auto [end, ec] = std::from_chars(text.data(), text.data() + text.size(), value); + if (ec == std::errc{} && end == text.data() + text.size()) return value; + return std::nullopt; + } + return std::nullopt; +} + std::optional run_ninja_fast(const std::string& ninjaProgram, const std::filesystem::path& outputDir, const std::filesystem::path& ninjaPath, @@ -1417,6 +1433,8 @@ std::optional run_ninja_fast(const std::string& ninjaProgram, } else if (runtimeEnvKey != "-" && !runtimeEnvValue.empty()) { childEnv.emplace_back(runtimeEnvKey, runtimeEnvValue); } + if (auto baseline = read_ninja_memory_baseline(ninjaPath)) + childEnv.emplace_back("MCPP_MEM_BASELINE", std::to_string(*baseline)); auto t0 = std::chrono::steady_clock::now(); // capture_exec merges stderr into the captured output (replacing `2>&1`), diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 3e0bb4c17..a13136e78 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -1406,6 +1406,10 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, plan.accelOverridden, plan.packFormat, plan.requestTag) + "\n"); append("ninja_required_version = 1.11\n\n"); + // Runtime memory admission is deliberately an environment input rather + // than part of any edge command. Ninja's command hash must not rebuild the + // graph when the startup observation moves between baseline buckets. + append(std::format("mcpp_mem_baseline = {}\n\n", plan.memoryBaselineBytes)); // All compile/link flags are computed once via flags.cppm. auto flags = compute_flags(plan); @@ -1845,9 +1849,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, const bool memoryAdmission = plan.memoryBaselineBytes != 0; auto admission_prefix = [&]() { return std::format("$mcpp compile-admit --sem .mcpp-sched" - " --mem-baseline {} --mem-threshold {}", - plan.memoryBaselineBytes, - plan.memoryThresholdPercent); + " --mem-threshold {}", plan.memoryThresholdPercent); }; auto admission_dep = [&]() { return needsGnuModuleFilter @@ -1921,7 +1923,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append("rule cxx_module_bmi\n"); append(" command = $mcpp bmi-compile --bmi $bmi_out --slot $slot" " --self $mcpp --sem $sched_sem --cap $sched_cap" - " --mem-baseline $sched_mem_baseline --mem-threshold $sched_mem_threshold" + " --mem-threshold $sched_mem_threshold" " --command-file $out.cmd --dep-from $scan_dep --dep-to $out.d\n"); append(std::format( " rspfile = $out.cmd\n" @@ -2985,7 +2987,6 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, e += " scan_dep = " + escape_ninja_path(ddi) + ".dep\n"; e += " sched_sem = .mcpp-sched\n"; e += std::format(" sched_cap = {}\n", plan.scheduleCompilerCap); - e += std::format(" sched_mem_baseline = {}\n", plan.memoryBaselineBytes); e += std::format(" sched_mem_threshold = {}\n", plan.memoryThresholdPercent); if (auto inc = local_include_flags(cu, dial); !inc.empty()) e += " local_includes =" + inc + "\n"; @@ -4602,6 +4603,7 @@ std::expected NinjaBackend::build(const BuildPlan& plan nenv.emplace_back(runtimeEnv->first, runtimeEnv->second); for (auto& ev : plan.toolchain.envOverrides) nenv.emplace_back(ev.key, ev.value); + nenv.emplace_back("MCPP_MEM_BASELINE", std::to_string(plan.memoryBaselineBytes)); // THE STAGED CACHE IS PLACED BY A PASS OF ITS OWN, before the graph that // reads it. ninja 1.12.1 loads a dyndep file whose producer is up to date diff --git a/src/cli.cppm b/src/cli.cppm index 050a61ec0..6eb761795 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -1162,7 +1162,12 @@ int run(int argc, char** argv) { int cap = 0, threshold = 80; unsigned long long baseline = 0; std::from_chars(capText.data(), capText.data() + capText.size(), cap); - std::from_chars(baselineText.data(), baselineText.data() + baselineText.size(), baseline); + if (!baselineText.empty()) { + std::from_chars(baselineText.data(), baselineText.data() + baselineText.size(), baseline); + } else if (const char* env = std::getenv("MCPP_MEM_BASELINE"); env && *env) { + const auto text = std::string_view(env); + std::from_chars(text.data(), text.data() + text.size(), baseline); + } std::from_chars(thresholdText.data(), thresholdText.data() + thresholdText.size(), threshold); std::vector command; for (++i; i < argc; ++i) command.emplace_back(argv[i]); diff --git a/src/cli/cmd_build.cppm b/src/cli/cmd_build.cppm index 2c1850c0c..a9f5fb006 100644 --- a/src/cli/cmd_build.cppm +++ b/src/cli/cmd_build.cppm @@ -1585,6 +1585,10 @@ export int cmd_bmi_compile(const mcpplibs::cmdline::ParsedArgs& parsed) { std::from_chars(cap.data(), cap.data() + cap.size(), req.maxCompilers); if (const auto baseline = opt_value(parsed, "mem-baseline"); !baseline.empty()) std::from_chars(baseline.data(), baseline.data() + baseline.size(), req.memoryBaselineBytes); + else if (const char* env = std::getenv("MCPP_MEM_BASELINE"); env && *env) { + const auto text = std::string_view(env); + std::from_chars(text.data(), text.data() + text.size(), req.memoryBaselineBytes); + } if (const auto threshold = opt_value(parsed, "mem-threshold"); !threshold.empty()) std::from_chars(threshold.data(), threshold.data() + threshold.size(), req.memoryThresholdPercent); req.commandFile = file_path("command-file"); diff --git a/src/toolchain/msvc_coroutines.cppm b/src/toolchain/msvc_coroutines.cppm index 6cd7d9f3b..697d43640 100644 --- a/src/toolchain/msvc_coroutines.cppm +++ b/src/toolchain/msvc_coroutines.cppm @@ -182,7 +182,7 @@ std::optional failed_command_in(std::string_view output) { // that line verbatim on failure: // // compile-admit --sem .mcpp-sched --cap N \ - // --mem-baseline B --mem-threshold P -- "" --target=... + // --mem-threshold P -- "" --target=... // // The compiler is still on the line, one wrapper in front of it, so // the driver is looked for wherever it sits. Everything read from the diff --git a/tests/unit/test_msvc_coroutines.cpp b/tests/unit/test_msvc_coroutines.cpp index b9c76f06f..a57a4a9ad 100644 --- a/tests/unit/test_msvc_coroutines.cpp +++ b/tests/unit/test_msvc_coroutines.cpp @@ -41,7 +41,7 @@ constexpr std::string_view kCoroutineUseFailure = // the line, so it is read from where it sits. constexpr std::string_view kCoroutineUseFailureUnderTheAdmissionWrapper = "[1/1] D:/a/_temp/mcpp-built/mcpp.exe compile-admit --sem .mcpp-sched --cap 4 " - "--mem-baseline 12884901888 --mem-threshold 80 -- " + "--mem-threshold 80 -- " "\"C:/Users/runneradmin/.mcpp/registry/data/xpkgs/xim-x-llvm/23.1.3/bin/clang++.exe\" " "@obj/main.o.rsp -std=c++20 -fms-runtime-lib=dll " "\"-fprebuilt-module-path=D:\\a\\_temp\\coro\\pcm.cache\" -O0 -g " diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index 01d184cc1..0c76b1216 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -2976,7 +2976,9 @@ TEST(NinjaBackend, OrdinaryMemoryAdmissionHasNoCountingCap) { auto plan = minimal_plan(); plan.memoryBaselineBytes = 4ULL * 1024 * 1024 * 1024; const auto ninja = emit_ninja_string(plan); - EXPECT_NE(ninja.find("compile-admit --sem .mcpp-sched --mem-baseline"), std::string::npos); + EXPECT_NE(ninja.find("compile-admit --sem .mcpp-sched --mem-threshold"), std::string::npos); + EXPECT_EQ(ninja.find("compile-admit --sem .mcpp-sched --mem-baseline"), std::string::npos); + EXPECT_NE(ninja.find("mcpp_mem_baseline = 4294967296"), std::string::npos); EXPECT_EQ(ninja.find("compile-admit --sem .mcpp-sched --cap"), std::string::npos); } From 77b5a9b4ca9d8b5a1ddd8a979bf12ca533faafa4 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 20:30:42 +0800 Subject: [PATCH 13/16] fix: bound memory admission waits --- src/build/schedule/detach_codegen.cppm | 29 ++++++++++++++++---------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/build/schedule/detach_codegen.cppm b/src/build/schedule/detach_codegen.cppm index 5ccb1559c..7c9fc835d 100644 --- a/src/build/schedule/detach_codegen.cppm +++ b/src/build/schedule/detach_codegen.cppm @@ -219,6 +219,7 @@ std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap, std::filesystem::create_directories(dir, ec); const auto started = std::chrono::steady_clock::now(); constexpr auto kLimit = std::chrono::hours(2); + const auto deadline = started + kLimit; bool wasDenied = false; for (;;) { const auto snapshot = mcpp::platform::capacity::memory_snapshot(); @@ -226,6 +227,23 @@ std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap, && !mcpp::platform::capacity::memory_allows( snapshot, baselineBytes, thresholdPercent, wasDenied); if (denied) wasDenied = true; + // Check before every admission attempt. Memory denial is a wait just + // like a full semaphore: it must not bypass the bounded-wait escape. + if (std::chrono::steady_clock::now() >= deadline) { + if (denied) { + std::println(std::cerr, + "mcpp: timed out waiting for memory admission in {} after 2h; " + "continuing without the concurrency cap", + dir.string()); + } else { + std::println(std::cerr, + "mcpp: no compiler slot became free in 2h — all {} tokens in {} " + "are held by processes that are gone. Continuing without the " + "concurrency cap; remove that directory to restore it.", + (std::max)(1, cap), dir.string()); + } + return {}; + } if (cap <= 0) { // Ninja alone bounds ordinary edges. Only memory denial uses a // shared fallback slot, allowing one compiler to make progress even @@ -247,17 +265,6 @@ std::filesystem::path acquire_token(const std::filesystem::path& dir, int cap, } } } - if (std::chrono::steady_clock::now() - started > kLimit) { - // Proceeds WITHOUT the cap rather than failing: the semaphore bounds - // memory pressure, it is not a correctness property, so an unbounded - // compile is a worse build and a failed one is no build at all. - std::println(std::cerr, - "mcpp: no compiler slot became free in 2h — all {} tokens in {} " - "are held by processes that are gone. Continuing without the " - "concurrency cap; remove that directory to restore it.", - (std::max)(1, cap), dir.string()); - return {}; - } std::this_thread::sleep_for(std::chrono::milliseconds(denied ? 50 : 5)); } } From a591f3424241dc165a233135f042cb200c4329d5 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 22:11:44 +0800 Subject: [PATCH 14/16] fix: name silent admission and depfile failures Reject a malformed --cap, --mem-baseline or --mem-threshold instead of letting it fall back to the default silently: from_chars was unchecked, so a garbage value was indistinguishable from an omitted one. The check is shared by compile-admit and bmi-compile, and MCPP_MEM_BASELINE is validated the same way. Name the file when the scanner depfile cannot be read or written. The wrapper exited 1 with no output after a successful compile, which reads as an anonymous compiler failure. Cover both with tests/scripts/test_compile_admit.py. --- src/build/schedule/detach_codegen.cppm | 26 +++++++++++++++-- src/cli.cppm | 37 ++++++++++++++++++------ src/cli/cmd_build.cppm | 39 ++++++++++++++++++++------ tests/scripts/test_compile_admit.py | 31 ++++++++++++++++++++ 4 files changed, 113 insertions(+), 20 deletions(-) diff --git a/src/build/schedule/detach_codegen.cppm b/src/build/schedule/detach_codegen.cppm index 7c9fc835d..f644d3fa2 100644 --- a/src/build/schedule/detach_codegen.cppm +++ b/src/build/schedule/detach_codegen.cppm @@ -77,6 +77,7 @@ module; #include extern char** environ; #endif +#include export module mcpp.build.schedule.detach_codegen; @@ -768,13 +769,32 @@ int run_admitted(const std::vector& command, } if (rc != 0 || depFrom.empty() || depTo.empty()) return rc; + // The compiler succeeded, so a failure from here on is this wrapper's, not + // a compiler failure. Name the file: an exit 1 with no message is read as + // a compiler failure and points at no culprit. + const auto reason = [] { return std::error_code{errno, std::generic_category()}.message(); }; std::ifstream in(depFrom, std::ios::binary); - if (!in) return 1; + if (!in) { + std::println(std::cerr, "mcpp: cannot read scanner depfile {}: {}", + depFrom.string(), reason()); + return 1; + } std::string raw{std::istreambuf_iterator{in}, {}}; + if (in.bad()) { + std::println(std::cerr, "mcpp: cannot read scanner depfile {}: {}", + depFrom.string(), reason()); + return 1; + } std::ofstream out(depTo, std::ios::binary | std::ios::trunc); - if (!out) return 1; + if (!out) { + std::println(std::cerr, "mcpp: cannot write depfile {}: {}", depTo.string(), reason()); + return 1; + } out << mcpp::build::depfile::first_record(raw); - if (!out) return 1; + if (!out) { + std::println(std::cerr, "mcpp: cannot write depfile {}: {}", depTo.string(), reason()); + return 1; + } std::filesystem::remove(depFrom, ec); return 0; } diff --git a/src/cli.cppm b/src/cli.cppm index 6eb761795..27862426c 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -1004,9 +1004,9 @@ int run(int argc, char** argv) { .option(cl::Option("slot").takes_value().value_name("PATH").help("where .log/.rc are kept")) .option(cl::Option("self").takes_value().value_name("PATH").help("path to mcpp, re-invoked as supervisor")) .option(cl::Option("sem").takes_value().value_name("DIR").help("concurrency token directory")) - .option(cl::Option("cap").takes_value().value_name("N").help("max concurrent compilers")) - .option(cl::Option("mem-baseline").takes_value().value_name("BYTES").help("memory baseline")) - .option(cl::Option("mem-threshold").takes_value().value_name("PERCENT").help("memory threshold")) + .option(cl::Option("cap").takes_value().value_name("N").help("max concurrent compilers")) + .option(cl::Option("mem-baseline").takes_value().value_name("BYTES").help("memory baseline")) + .option(cl::Option("mem-threshold").takes_value().value_name("PERCENT").help("memory threshold")) .option(cl::Option("command-file").takes_value().value_name("PATH").help("file holding the compiler command line")) .option(cl::Option("dep-from").takes_value().value_name("PATH").help("scanner depfile to adopt")) .option(cl::Option("dep-to").takes_value().value_name("PATH").help("where ninja expects this edge's depfile")) @@ -1159,16 +1159,37 @@ int run(int argc, char** argv) { std::println(stderr, "error: compile-admit requires --sem and -- "); return 2; } + // An omitted value keeps its default; a MALFORMED one must not be + // silently indistinguishable from an omitted one. from_chars leaves + // the destination untouched on failure (and reports an empty range), + // so every rejection has to be checked and named here. + auto parse_number = [](std::string_view text, auto& out) -> bool { + if (text.empty()) return true; + const auto [end, ec] = std::from_chars(text.data(), text.data() + text.size(), out); + return ec == std::errc{} && end == text.data() + text.size(); + }; int cap = 0, threshold = 80; unsigned long long baseline = 0; - std::from_chars(capText.data(), capText.data() + capText.size(), cap); + if (!parse_number(capText, cap)) { + std::println(stderr, "error: compile-admit: invalid --cap '{}'", capText); + return 2; + } if (!baselineText.empty()) { - std::from_chars(baselineText.data(), baselineText.data() + baselineText.size(), baseline); + if (!parse_number(baselineText, baseline)) { + std::println(stderr, "error: compile-admit: invalid --mem-baseline '{}'", + baselineText); + return 2; + } } else if (const char* env = std::getenv("MCPP_MEM_BASELINE"); env && *env) { - const auto text = std::string_view(env); - std::from_chars(text.data(), text.data() + text.size(), baseline); + if (!parse_number(std::string_view{env}, baseline)) { + std::println(stderr, "error: compile-admit: invalid MCPP_MEM_BASELINE '{}'", env); + return 2; + } + } + if (!parse_number(thresholdText, threshold)) { + std::println(stderr, "error: compile-admit: invalid --mem-threshold '{}'", thresholdText); + return 2; } - std::from_chars(thresholdText.data(), thresholdText.data() + thresholdText.size(), threshold); std::vector command; for (++i; i < argc; ++i) command.emplace_back(argv[i]); return mcpp::build::schedule::detach::run_admitted( diff --git a/src/cli/cmd_build.cppm b/src/cli/cmd_build.cppm index a9f5fb006..22a4d0569 100644 --- a/src/cli/cmd_build.cppm +++ b/src/cli/cmd_build.cppm @@ -1581,16 +1581,37 @@ export int cmd_bmi_compile(const mcpplibs::cmdline::ParsedArgs& parsed) { req.self = std::filesystem::path{opt_value(parsed, "self")}; req.semaphore = file_path("sem"); req.maxCompilers = 0; - if (const auto cap = opt_value(parsed, "cap"); !cap.empty()) - std::from_chars(cap.data(), cap.data() + cap.size(), req.maxCompilers); - if (const auto baseline = opt_value(parsed, "mem-baseline"); !baseline.empty()) - std::from_chars(baseline.data(), baseline.data() + baseline.size(), req.memoryBaselineBytes); - else if (const char* env = std::getenv("MCPP_MEM_BASELINE"); env && *env) { - const auto text = std::string_view(env); - std::from_chars(text.data(), text.data() + text.size(), req.memoryBaselineBytes); + // Omitted values keep their defaults; malformed ones are named rather than + // silently collapsed into the same fallback. The emitters are in-repo, so a + // malformed value means a bug worth surfacing rather than hiding. + auto parse_number = [](std::string_view text, auto& out) -> bool { + if (text.empty()) return true; + const auto [end, ec] = std::from_chars(text.data(), text.data() + text.size(), out); + return ec == std::errc{} && end == text.data() + text.size(); + }; + if (const auto cap = opt_value(parsed, "cap"); !cap.empty()) { + if (!parse_number(cap, req.maxCompilers)) { + std::println(stderr, "error: bmi-compile: invalid --cap '{}'", cap); + return 2; + } + } + if (const auto baseline = opt_value(parsed, "mem-baseline"); !baseline.empty()) { + if (!parse_number(baseline, req.memoryBaselineBytes)) { + std::println(stderr, "error: bmi-compile: invalid --mem-baseline '{}'", baseline); + return 2; + } + } else if (const char* env = std::getenv("MCPP_MEM_BASELINE"); env && *env) { + if (!parse_number(std::string_view{env}, req.memoryBaselineBytes)) { + std::println(stderr, "error: bmi-compile: invalid MCPP_MEM_BASELINE '{}'", env); + return 2; + } + } + if (const auto threshold = opt_value(parsed, "mem-threshold"); !threshold.empty()) { + if (!parse_number(threshold, req.memoryThresholdPercent)) { + std::println(stderr, "error: bmi-compile: invalid --mem-threshold '{}'", threshold); + return 2; + } } - if (const auto threshold = opt_value(parsed, "mem-threshold"); !threshold.empty()) - std::from_chars(threshold.data(), threshold.data() + threshold.size(), req.memoryThresholdPercent); req.commandFile = file_path("command-file"); req.depFrom = file_path("dep-from"); req.depTo = file_path("dep-to"); diff --git a/tests/scripts/test_compile_admit.py b/tests/scripts/test_compile_admit.py index ff7740509..da5c2afa1 100644 --- a/tests/scripts/test_compile_admit.py +++ b/tests/scripts/test_compile_admit.py @@ -81,6 +81,37 @@ def test_pressure_admits_one_and_releases_fallback(self): proc.kill() proc.wait(timeout=10) + def test_malformed_numeric_option_is_named(self): + # An omitted value keeps its default; a MALFORMED one must not be + # silently indistinguishable from an omitted one. + cases = ( + (["compile-admit", "--sem", str(self.sem), "--cap", "x", + "--", sys.executable, "-c", "pass"], "--cap"), + (["compile-admit", "--sem", str(self.sem), "--mem-baseline", "x", + "--", sys.executable, "-c", "pass"], "--mem-baseline"), + (["compile-admit", "--sem", str(self.sem), "--mem-threshold", "x", + "--", sys.executable, "-c", "pass"], "--mem-threshold"), + (["bmi-compile", "--cap", "x"], "--cap"), + ) + for argv, flag in cases: + with self.subTest(argv=argv): + result = subprocess.run([MCPP, *argv], capture_output=True, + text=True, timeout=10) + self.assertEqual(result.returncode, 2, result.stderr) + self.assertIn(flag, result.stderr) + + def test_missing_scanner_depfile_is_named(self): + # The compile succeeded, so the exit 1 is the wrapper's; it must name + # the file it could not read rather than look like a compiler failure. + missing = self.root / "no-such.d" + result = subprocess.run( + [MCPP, "compile-admit", "--sem", str(self.sem), "--mem-baseline", "0", + "--dep-raw", str(missing), "--dep-out", str(self.root / "out.d"), + "--", sys.executable, "-c", "pass"], + capture_output=True, text=True, timeout=10) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertIn(str(missing), result.stderr) + @unittest.skipIf(os.name == "nt", "POSIX BMI restat contract") def test_bmi_restore_and_failure_cleanup(self): bmi = self.root / "interface.pcm" From 497ea405f3a336ec0a68f8aa3cd54cfe5da50c74 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 22:11:44 +0800 Subject: [PATCH 15/16] refactor: share one spelling of the schedule semaphore directory The generated rules spelled the token directory as a literal in the admission prefix, as the $sched_sem edge variable and in the stale-token reclaim. Derive all three from one file-scope constant so they cannot drift apart. --- src/build/ninja_backend.cppm | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index a13136e78..43cf42fe2 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -319,6 +319,12 @@ namespace { // U+FEFF in UTF-8. The response files of the MSVC tools begin with it (#693). constexpr std::string_view kUtf8ByteOrderMark = "\xEF\xBB\xBF"; +// The token directory every admission wrapper shares: the detached-compiler +// edges spell it as the `$sched_sem` edge variable, the ordinary compile rules +// interpolate it straight into the command, and the stale-token reclaim below +// removes it. One constant, so the spellings cannot drift apart. +constexpr std::string_view kSchedSemaphoreDir = ".mcpp-sched"; + // Whether the plan's compiler names an imported module's BMI as MSVC does // (`/reference =`), and so reads an argument file by MSVC's rules. bool msvc_module_spelling(const BuildPlan& plan) { @@ -1848,8 +1854,8 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, // existing shell-free depfile filter is moved into the wrapper. const bool memoryAdmission = plan.memoryBaselineBytes != 0; auto admission_prefix = [&]() { - return std::format("$mcpp compile-admit --sem .mcpp-sched" - " --mem-threshold {}", plan.memoryThresholdPercent); + return std::format("$mcpp compile-admit --sem {} --mem-threshold {}", + kSchedSemaphoreDir, plan.memoryThresholdPercent); }; auto admission_dep = [&]() { return needsGnuModuleFilter @@ -2985,7 +2991,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, e += " obj_out = " + obj + "\n"; e += " slot = " + slot + "\n"; e += " scan_dep = " + escape_ninja_path(ddi) + ".dep\n"; - e += " sched_sem = .mcpp-sched\n"; + e += std::format(" sched_sem = {}\n", kSchedSemaphoreDir); e += std::format(" sched_cap = {}\n", plan.scheduleCompilerCap); e += std::format(" sched_mem_threshold = {}\n", plan.memoryThresholdPercent); if (auto inc = local_include_flags(cu, dial); !inc.empty()) @@ -4224,7 +4230,7 @@ void reclaim_stale_schedule_tokens(const std::filesystem::path& outputDir) { std::lock_guard lock(m); if (!reclaimed.insert(outputDir.lexically_normal().generic_string()).second) return; std::error_code ec; - std::filesystem::remove_all(outputDir / ".mcpp-sched", ec); + std::filesystem::remove_all(outputDir / kSchedSemaphoreDir, ec); } } // namespace From 4c28dd7e648e06173582fb0877c5e1af4d1e7119 Mon Sep 17 00:00:00 2001 From: Zhe Feng Date: Tue, 13 Oct 2026 23:48:22 +0800 Subject: [PATCH 16/16] test: hold the admission gate across the Windows shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compile-admit starts a child through cmd.exe on Windows (run_exec's documented shell path), where a literal newline ends the command. The pressure test's holding child embedded a newline, so only its first line ran: the child touched `ready` and exited at once, releasing the token the test needs it to hold. Whether that showed as a failure or as the "memory metrics unavailable" skip was then a ~10ms race — and the test never actually exercised admission on Windows. Make the program single-line (the wait loop moves into an exec'd string, whose newline is an escape rather than a separator) and require the holding edge to still be running before the gate is read, so an early exit is a named failure instead of a silent skip. --- tests/scripts/test_compile_admit.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/scripts/test_compile_admit.py b/tests/scripts/test_compile_admit.py index da5c2afa1..1f92211c7 100644 --- a/tests/scripts/test_compile_admit.py +++ b/tests/scripts/test_compile_admit.py @@ -50,10 +50,19 @@ def test_pressure_admits_one_and_releases_fallback(self): ready_literal = repr(str(ready)) release_literal = repr(str(release)) done_literal = repr(str(done)) + # The program is ONE LINE on purpose. `compile-admit` starts the child + # through cmd.exe on Windows (`run_exec`'s documented shell path), and a + # literal newline ends the command there: only the first line would run, + # the child would touch `ready` and exit immediately, releasing the very + # token this test needs it to hold. Whether that surfaced as a failure + # or as the "metrics unavailable" skip was then a ~10ms race. The wait + # loop therefore lives in an exec'd string, whose `\n` is an escape and + # not a command separator. script = (f"import pathlib,time; ready=pathlib.Path({ready_literal}); " f"release=pathlib.Path({release_literal}); ready.touch(); " "deadline=time.monotonic()+10; " - "\nwhile not release.exists() and time.monotonic()