[BatchMode] Fix rdar://38123690 a little more robustly.

Author: graydon

include/swift/Driver/ToolChain.h

@@ -29,9 +29,6 @@ namespace driver {
   class Job;
   class OutputInfo;
 
-/// The limit for passing a list of files on the command line.
-const size_t TOO_MANY_FILES = 128;
-
 /// A ToolChain is responsible for turning abstract Actions into concrete,
 /// runnable Jobs.
 ///

lib/Driver/Compilation.cpp

@@ -688,7 +688,6 @@ namespace driver {
           NonBatchable.insert(Cmd);
         }
       }
-      PendingExecution.clear();
     }
 
     /// If \p Batch is nonempty, construct a new \c BatchJob from its
@@ -770,6 +769,30 @@ namespace driver {
       }
     }
 
+    // FIXME: at the moment we're not passing OutputFileMaps to frontends, so
+    // due to the multiplication of the number of additional files and the
+    // number of files in a batch, it's pretty easy to construct too-long
+    // command lines here, which will then fail to exec. We address this crudely
+    // by re-forming batches with a finer partition when we overflow.
+    bool shouldRetryWithMorePartitions(std::vector<const Job *> const &Batches,
+                                       size_t &NumPartitions) {
+
+      // Stop rebatching if we can't subdivide batches any further.
+      if (NumPartitions > PendingExecution.size())
+        return false;
+
+      for (auto const *B : Batches) {
+        if (!llvm::sys::commandLineFitsWithinSystemLimits(B->getExecutable(),
+                                                          B->getArguments())) {
+          // To avoid redoing the batch loop too many times, repartition pretty
+          // aggressively by doubling partition count / halving size.
+          NumPartitions *= 2;
+          return true;
+        }
+      }
+      return false;
+    }
+
     /// Select jobs that are batch-combinable from \c PendingExecution, combine
     /// them together into \p BatchJob instances (also inserted into \p
     /// BatchJobs), and enqueue all \c PendingExecution jobs (whether batched or
@@ -783,23 +806,29 @@ namespace driver {
         return;
       }
 
-      // Split the batchable from non-batchable pending jobs.
+      size_t NumPartitions = Comp.NumberOfParallelCommands;
       CommandSetVector Batchable, NonBatchable;
-      getPendingBatchableJobs(Batchable, NonBatchable);
+      std::vector<const Job *> Batches;
+      do {
+        // We might be restarting loop; clear these before proceeding.
+        Batchable.clear();
+        NonBatchable.clear();
+        Batches.clear();
 
-      // Partition the batchable jobs into sets.
-      // FIXME: at present we set a maximum batch size of TOO_MANY_FILES
-      // so that we don't overflow command-line lengths. This can go away
-      // when we're passing OFMs to the frontend processes.
-      BatchPartition Partition(std::max(size_t(Comp.NumberOfParallelCommands),
-                                        Batchable.size() / TOO_MANY_FILES));
-      partitionIntoBatches(Batchable.takeVector(), Partition);
+        // Split the batchable from non-batchable pending jobs.
+        getPendingBatchableJobs(Batchable, NonBatchable);
 
-      // Construct a BatchJob from each batch in the partition.
-      std::vector<const Job *> Batches;
-      for (auto const &Batch : Partition) {
-        formBatchJobFromPartitionBatch(Batches, Batch);
-      }
+        // Partition the batchable jobs into sets.
+        BatchPartition Partition(NumPartitions);
+        partitionIntoBatches(Batchable.takeVector(), Partition);
+
+        // Construct a BatchJob from each batch in the partition.
+        for (auto const &Batch : Partition) {
+          formBatchJobFromPartitionBatch(Batches, Batch);
+        }
+
+      } while (shouldRetryWithMorePartitions(Batches, NumPartitions));
+      PendingExecution.clear();
 
       // Save batches so we can locate and decompose them on task-exit.
       for (const Job *Cmd : Batches)

lib/Driver/ToolChains.cpp

@@ -37,6 +37,9 @@ using namespace swift;
 using namespace swift::driver;
 using namespace llvm::opt;
 
+/// The limit for passing a list of files on the command line.
+static const size_t TOO_MANY_FILES = 128;
+
 static void addInputsOfType(ArgStringList &Arguments,
                             ArrayRef<const Action *> Inputs,
                             types::ID InputType,

test/Driver/batch_mode_overlong_argv.swift

@@ -1,3 +1,4 @@
+// REQUIRES: OS=macosx
 // RUN: %empty-directory(%t)
 // RUN: touch  %t/f_1_1.swift %t/f_1_2.swift %t/f_1_3.swift %t/f_1_4.swift %t/f_1_5.swift %t/f_1_6.swift %t/f_1_7.swift %t/f_1_8.swift %t/f_1_9.swift %t/f_1_10.swift
 // RUN: touch  %t/f_2_1.swift %t/f_2_2.swift %t/f_2_3.swift %t/f_2_4.swift %t/f_2_5.swift %t/f_2_6.swift %t/f_2_7.swift %t/f_2_8.swift %t/f_2_9.swift %t/f_2_10.swift
@@ -99,11 +100,15 @@
 // RUN: touch  %t/f_98_1.swift %t/f_98_2.swift %t/f_98_3.swift %t/f_98_4.swift %t/f_98_5.swift %t/f_98_6.swift %t/f_98_7.swift %t/f_98_8.swift %t/f_98_9.swift %t/f_98_10.swift
 // RUN: touch  %t/f_99_1.swift %t/f_99_2.swift %t/f_99_3.swift %t/f_99_4.swift %t/f_99_5.swift %t/f_99_6.swift %t/f_99_7.swift %t/f_99_8.swift %t/f_99_9.swift %t/f_99_10.swift
 // RUN: touch  %t/f_100_1.swift %t/f_100_2.swift %t/f_100_3.swift %t/f_100_4.swift %t/f_100_5.swift %t/f_100_6.swift %t/f_100_7.swift %t/f_100_8.swift %t/f_100_9.swift %t/f_100_10.swift
-// RUN: %swiftc_driver -driver-show-job-lifecycle -c -module-name foo -emit-module -serialize-diagnostics -emit-dependencies -j 1 -enable-batch-mode %t/f_*.swift >%t/out.txt 2>&1
+// RUN: mkdir -p %t/additional/path/elements/often/make/filenames/longer/than/one/might/expect/especially/given/output/directories/deep/within/a/derived/data/folder/of/a/CI/machine/
+// RUN: %swiftc_driver -driver-show-job-lifecycle -v -c -module-name foo -o %t/additional/path/elements/often/make/filenames/longer/than/one/might/expect/especially/given/output/directories/deep/within/a/derived/data/folder/of/a/CI/machine/foo.o -emit-module -serialize-diagnostics -emit-dependencies -j 1 -enable-batch-mode %t/f_*.swift >%t/out.txt 2>&1
 // RUN: %FileCheck %s <%t/out.txt
 // CHECK-NOT: unable to execute command
-// CHECK: Found 1000 batchable jobs
-// CHECK: Forming into 7 batches
+// CHECK: Forming into 1 batches
+// CHECK: Forming batch job from 1000 constituents
+// CHECK: Forming into 2 batches
+// CHECK: Forming batch job from 500 constituents
+// CHECK: Forming batch job from 500 constituents
 func thing() {
     print(1)
 }