Skip to content

Commit 56d94ab

Browse files
typelessclaude
andcommitted
Remove sequential execution path from scheduler
The parallel event loop with jobs=1 naturally handles sequential execution, making execute_sequential and execute_job redundant. - Remove Scheduler::Impl::execute_sequential and execute_job methods - Remove jobs==1 fast path that dispatched to sequential - Drop transitive runner.hpp include from scheduler.hpp (add chrono directly) - CommandRunner and runner.hpp/cpp retained for E2E fixture use Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 62d3f84 commit 56d94ab

2 files changed

Lines changed: 2 additions & 167 deletions

File tree

include/pup/exec/scheduler.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
#include "pup/core/string_id.hpp"
1010
#include "pup/core/types.hpp"
1111
#include "pup/core/vec.hpp"
12-
#include "runner.hpp"
1312

13+
#include <chrono>
1414
#include <optional>
1515

1616
namespace pup::graph {
@@ -140,7 +140,7 @@ class Scheduler final {
140140
private:
141141
std::unique_ptr<Impl> impl_;
142142

143-
/// Execute jobs in parallel (or dispatch to sequential for jobs == 1)
143+
/// Execute jobs in parallel using the async event loop
144144
auto execute_parallel(
145145
Vec<BuildJob> const& jobs,
146146
graph::BuildGraph const& graph

src/exec/scheduler.cpp

Lines changed: 0 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -473,21 +473,6 @@ struct Scheduler::Impl {
473473
JobStartCallback on_start;
474474
JobCompleteCallback on_complete;
475475
ProgressCallback on_progress;
476-
477-
auto execute_sequential(
478-
Vec<BuildJob> const& jobs,
479-
graph::BuildGraph const& graph,
480-
EnvCache const& env_cache
481-
) -> Result<void>;
482-
483-
auto execute_job(
484-
BuildJob const& job,
485-
CommandRunner& runner,
486-
EnvCache const& env_cache,
487-
std::string_view source_root_sv,
488-
std::string_view output_root_sv,
489-
std::string_view output_root_prefix
490-
) -> JobResult;
491476
};
492477

493478
Scheduler::Scheduler(SchedulerOptions options)
@@ -663,98 +648,13 @@ auto Scheduler::build_incremental(
663648
return impl_->stats;
664649
}
665650

666-
auto Scheduler::Impl::execute_sequential(
667-
Vec<BuildJob> const& jobs,
668-
graph::BuildGraph const& graph,
669-
EnvCache const& env_cache
670-
) -> Result<void>
671-
{
672-
auto& pool = global_pool();
673-
auto runner = CommandRunner {};
674-
if (!is_empty(options.source_root)) {
675-
runner.set_working_dir(options.source_root);
676-
}
677-
if (options.timeout) {
678-
runner.set_timeout(*options.timeout); // NOLINT(bugprone-unchecked-optional-access)
679-
}
680-
681-
auto source_root_sv = pool.get(options.source_root);
682-
auto output_root_sv = pool.get(options.output_root);
683-
auto output_root_prefix = pool.get(pup::path::relative(output_root_sv, source_root_sv));
684-
685-
auto [in_degree, dependents] = build_dependency_map(jobs, graph);
686-
687-
// Validate no active job depends on a skipped job
688-
if (auto result = validate_guard_dependencies(jobs, dependents); !result) {
689-
return pup::unexpected<Error>(result.error());
690-
}
691-
692-
// Count inactive jobs upfront (they never enter the queue)
693-
auto inactive_count = std::count_if(jobs.begin(), jobs.end(), [](auto const& j) { return !j.guard_active; });
694-
stats.skipped_jobs += static_cast<std::size_t>(inactive_count);
695-
696-
// Only queue active jobs with no dependencies
697-
auto ready_queue = std::queue<std::size_t> {};
698-
for (auto i = std::size_t { 0 }; i < jobs.size(); ++i) {
699-
if (in_degree[i] == 0 && jobs[i].guard_active) {
700-
ready_queue.push(i);
701-
}
702-
}
703-
704-
while (!ready_queue.empty()) {
705-
if (cancelled) {
706-
break;
707-
}
708-
709-
auto const job_idx = ready_queue.front();
710-
ready_queue.pop();
711-
auto const& job = jobs[job_idx];
712-
713-
if (on_start) {
714-
on_start(job);
715-
}
716-
717-
auto result = JobResult { execute_job(job, runner, env_cache, source_root_sv, output_root_sv, output_root_prefix) };
718-
719-
if (on_complete) {
720-
on_complete(job, result);
721-
}
722-
723-
stats.build_time += result.duration;
724-
725-
if (result.success) {
726-
++stats.completed_jobs;
727-
for (auto dep_idx : dependents[job_idx]) {
728-
if (--in_degree[dep_idx] == 0 && jobs[dep_idx].guard_active) {
729-
ready_queue.push(dep_idx);
730-
}
731-
}
732-
} else {
733-
++stats.failed_jobs;
734-
if (!options.keep_going) {
735-
return make_error<void>(ErrorCode::CommandFailed, "Command failed");
736-
}
737-
}
738-
739-
if (on_progress) {
740-
on_progress(stats.completed_jobs + stats.failed_jobs, stats.total_jobs);
741-
}
742-
}
743-
744-
return {};
745-
}
746-
747651
auto Scheduler::execute_parallel(
748652
Vec<BuildJob> const& jobs,
749653
graph::BuildGraph const& graph
750654
) -> Result<void>
751655
{
752656
auto const env_cache = build_env_cache(jobs);
753657

754-
if (impl_->options.jobs == 1 || jobs.size() == 1) {
755-
return impl_->execute_sequential(jobs, graph, env_cache);
756-
}
757-
758658
auto& pool = global_pool();
759659

760660
// Build dependency map
@@ -1082,71 +982,6 @@ auto Scheduler::execute_parallel(
1082982
return {};
1083983
}
1084984

1085-
auto Scheduler::Impl::execute_job(
1086-
BuildJob const& job,
1087-
CommandRunner& runner,
1088-
EnvCache const& env_cache,
1089-
std::string_view source_root_sv,
1090-
std::string_view output_root_sv,
1091-
std::string_view output_root_prefix
1092-
) -> JobResult
1093-
{
1094-
auto& pool = global_pool();
1095-
auto result = JobResult {
1096-
.id = job.id,
1097-
.success = false,
1098-
.exit_code = 0,
1099-
.output = {},
1100-
.duration = {},
1101-
};
1102-
1103-
auto prepared = prepare_job_launch(job, env_cache, options, source_root_sv, output_root_sv, output_root_prefix);
1104-
1105-
if (options.dry_run) {
1106-
result.success = true;
1107-
return result;
1108-
}
1109-
1110-
auto run_opts = RunOptions {};
1111-
if (!is_empty(prepared.working_dir)) {
1112-
run_opts.working_dir = prepared.working_dir;
1113-
}
1114-
run_opts.env = std::move(prepared.env_ids);
1115-
1116-
auto cmd_result = runner.run(pool.get(job.command), run_opts);
1117-
if (!cmd_result) {
1118-
result.output = pool.intern("Failed to execute command");
1119-
return result;
1120-
}
1121-
1122-
result.exit_code = cmd_result->exit_code;
1123-
result.success = (cmd_result->exit_code == 0);
1124-
result.duration = cmd_result->duration;
1125-
1126-
auto stdout_sv = pool.get(cmd_result->stdout_output);
1127-
auto stderr_sv = pool.get(cmd_result->stderr_output);
1128-
auto output_buf = Buf {};
1129-
if (!stdout_sv.empty()) {
1130-
output_buf += stdout_sv;
1131-
}
1132-
if (!stderr_sv.empty()) {
1133-
if (!output_buf.empty()) {
1134-
output_buf += '\n';
1135-
}
1136-
output_buf += stderr_sv;
1137-
}
1138-
if (!output_buf.empty()) {
1139-
result.output = output_buf.intern(pool);
1140-
}
1141-
1142-
if (result.success) {
1143-
parse_stdout_depfile(job, stdout_sv, result.discovered_deps, result.deps_for_command);
1144-
discover_d_file_deps(job, result, source_root_sv, output_root_sv, output_root_prefix);
1145-
}
1146-
1147-
return result;
1148-
}
1149-
1150985
auto Scheduler::build_job_list(
1151986
graph::BuildGraph const& graph
1152987
) -> Result<Vec<BuildJob>>

0 commit comments

Comments
 (0)