Skip to content

Rdar 40526328 emit sigint for cancelled batch constituents #17167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/Driver/Compilation.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ class Compilation {
/// Provides a randomization seed to batch-mode partitioning, for debugging.
const unsigned BatchSeed;

/// Overrides parallelism level as count of batches, if in batch-mode.
const Optional<unsigned> BatchCount;

/// In order to test repartitioning, set to true if
/// -driver-force-one-batch-repartition is present.
const bool ForceOneBatchRepartition = false;
Expand Down Expand Up @@ -228,6 +231,7 @@ class Compilation {
bool EnableIncrementalBuild = false,
bool EnableBatchMode = false,
unsigned BatchSeed = 0,
Optional<unsigned> BatchCount = None,
bool ForceOneBatchRepartition = false,
bool SaveTemps = false,
bool ShowDriverTimeCompilation = false,
Expand Down
6 changes: 0 additions & 6 deletions include/swift/Driver/Driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,6 @@ class Driver {
/// Indicates whether the driver should check that the input files exist.
bool CheckInputFilesExist = true;

/// Provides a randomization seed to batch-mode partitioning, for debugging.
unsigned DriverBatchSeed = 0;

/// Forces a repartition for testing.
bool DriverForceOneBatchRepartition = false;

public:
Driver(StringRef DriverExecutable, StringRef Name,
ArrayRef<const char *> Args, DiagnosticEngine &Diags);
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Option/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ def driver_filelist_threshold_EQ : Joined<["-"], "driver-filelist-threshold=">,
def driver_batch_seed : Separate<["-"], "driver-batch-seed">,
InternalDebugOpt,
HelpText<"Use the given seed value to randomize batch-mode partitions">;
def driver_batch_count : Separate<["-"], "driver-batch-count">,
InternalDebugOpt,
HelpText<"Use the given number of batch-mode partitions, rather than default parallelism level">;
def driver_force_one_batch_repartition : Flag<["-"], "driver-force-one-batch-repartition">,
InternalDebugOpt,
HelpText<"Force one batch repartitioning for testing">;
Expand Down
66 changes: 59 additions & 7 deletions lib/Driver/Compilation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@

#include "CompilationRecord.h"

#include <signal.h>

#define DEBUG_TYPE "batch-mode"

// Batch-mode has a sub-mode for testing that randomizes batch partitions,
Expand Down Expand Up @@ -111,6 +113,7 @@ Compilation::Compilation(DiagnosticEngine &Diags,
bool EnableIncrementalBuild,
bool EnableBatchMode,
unsigned BatchSeed,
Optional<unsigned> BatchCount,
bool ForceOneBatchRepartition,
bool SaveTemps,
bool ShowDriverTimeCompilation,
Expand All @@ -130,6 +133,7 @@ Compilation::Compilation(DiagnosticEngine &Diags,
OutputCompilationRecordForModuleOnlyBuild),
EnableBatchMode(EnableBatchMode),
BatchSeed(BatchSeed),
BatchCount(BatchCount),
ForceOneBatchRepartition(ForceOneBatchRepartition),
SaveTemps(SaveTemps),
ShowDriverTimeCompilation(ShowDriverTimeCompilation),
Expand Down Expand Up @@ -456,6 +460,35 @@ namespace driver {
}
}

/// Check to see if a job produced a zero-length serialized diagnostics
/// file, which is used to indicate batch-constituents that were batched
/// together with a failing constituent but did not, themselves, produce any
/// errors.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if the function were named based on that purpose, rather than on how it's planning to check. jobWasBatchedWithFailingJobs or something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I wish I had caught that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

bool jobWasBatchedWithFailingJobs(const Job *J) const {
auto DiaPath =
J->getOutput().getAnyOutputForType(file_types::TY_SerializedDiagnostics);
if (DiaPath.empty())
return false;
if (!llvm::sys::fs::is_regular_file(DiaPath))
return false;
uint64_t Size;
auto EC = llvm::sys::fs::file_size(DiaPath, Size);
if (EC)
return false;
return Size == 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for rearranging. I like the way this came out, hope you do, too.


/// If a batch-constituent job happens to be batched together with a job
/// that exits with an error, the batch-constituent may be considered
/// "cancelled".
bool jobIsCancelledBatchConstituent(int ReturnCode,
const Job *ContainerJob,
const Job *ConstituentJob) {
return ReturnCode != 0 &&
isBatchJob(ContainerJob) &&
jobWasBatchedWithFailingJobs(ConstituentJob);
}

/// Unpack a \c BatchJob that has finished into its constituent \c Job
/// members, and call \c taskFinished on each, propagating any \c
/// TaskFinishedResponse other than \c
Expand All @@ -481,6 +514,27 @@ namespace driver {
return res;
}

void
emitParseableOutputForEachFinishedJob(ProcessId Pid, int ReturnCode,
StringRef Output,
const Job *FinishedCmd,
TaskProcessInformation ProcInfo) {
FinishedCmd->forEachContainedJobAndPID(Pid, [&](const Job *J,
Job::PID P) {
if (jobIsCancelledBatchConstituent(ReturnCode, FinishedCmd, J)) {
// Simulate SIGINT-interruption to parseable-output consumer for any
// constituent of a failing batch job that produced no errors of its
// own.
parseable_output::emitSignalledMessage(llvm::errs(), *J, P,
"cancelled batch constituent",
"", SIGINT, ProcInfo);
} else {
parseable_output::emitFinishedMessage(llvm::errs(), *J, P, ReturnCode,
Output, ProcInfo);
}
});
}

/// Callback which will be called immediately after a task has finished
/// execution. Determines if execution should continue, and also schedule
/// any additional Jobs which we now know we need to run.
Expand Down Expand Up @@ -508,12 +562,8 @@ namespace driver {
llvm::errs() << Output;
break;
case OutputLevel::Parseable:
// Parseable output was requested.
FinishedCmd->forEachContainedJobAndPID(Pid, [&](const Job *J,
Job::PID P) {
parseable_output::emitFinishedMessage(llvm::errs(), *J, P,
ReturnCode, Output, ProcInfo);
});
emitParseableOutputForEachFinishedJob(Pid, ReturnCode, Output,
FinishedCmd, ProcInfo);
break;
}
}
Expand Down Expand Up @@ -880,7 +930,9 @@ namespace driver {
return;
}

size_t NumPartitions = TQ->getNumberOfParallelTasks();
size_t NumPartitions = (Comp.BatchCount.hasValue() ?
Comp.BatchCount.getValue() :
TQ->getNumberOfParallelTasks());
CommandSetVector Batchable, NonBatchable;
std::vector<const Job *> Batches;
bool PretendTheCommandLineIsTooLongOnce =
Expand Down
40 changes: 33 additions & 7 deletions lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,35 @@ static bool getFilelistThreshold(DerivedArgList &Args, size_t &FilelistThreshold
return false;
}

static unsigned
getDriverBatchSeed(llvm::opt::InputArgList &ArgList,
DiagnosticEngine &Diags) {
unsigned DriverBatchSeed = 0;
if (const Arg *A = ArgList.getLastArg(options::OPT_driver_batch_seed)) {
if (StringRef(A->getValue()).getAsInteger(10, DriverBatchSeed)) {
Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
A->getAsString(ArgList), A->getValue());
}
}
return DriverBatchSeed;
}

static Optional<unsigned>
getDriverBatchCount(llvm::opt::InputArgList &ArgList,
DiagnosticEngine &Diags)
{
if (const Arg *A = ArgList.getLastArg(options::OPT_driver_batch_count)) {
unsigned Count = 0;
if (StringRef(A->getValue()).getAsInteger(10, Count)) {
Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
A->getAsString(ArgList), A->getValue());
} else {
return Count;
}
}
return None;
}

std::unique_ptr<Compilation>
Driver::buildCompilation(const ToolChain &TC,
std::unique_ptr<llvm::opt::InputArgList> ArgList) {
Expand All @@ -663,13 +692,9 @@ Driver::buildCompilation(const ToolChain &TC,
ArgList->hasArg(options::OPT_driver_show_incremental);
bool ShowJobLifecycle =
ArgList->hasArg(options::OPT_driver_show_job_lifecycle);
if (const Arg *A = ArgList->getLastArg(options::OPT_driver_batch_seed)) {
if (StringRef(A->getValue()).getAsInteger(10, DriverBatchSeed)) {
Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
A->getAsString(*ArgList), A->getValue());
}
}
DriverForceOneBatchRepartition =
unsigned DriverBatchSeed = getDriverBatchSeed(*ArgList, Diags);
Optional<unsigned> DriverBatchCount = getDriverBatchCount(*ArgList, Diags);
bool DriverForceOneBatchRepartition =
ArgList->hasArg(options::OPT_driver_force_one_batch_repartition);

bool Incremental = ArgList->hasArg(options::OPT_incremental);
Expand Down Expand Up @@ -837,6 +862,7 @@ Driver::buildCompilation(const ToolChain &TC,
Incremental,
BatchMode,
DriverBatchSeed,
DriverBatchCount,
DriverForceOneBatchRepartition,
SaveTemps,
ShowDriverTimeCompilation,
Expand Down