Merge pull request #886 from apple/akyrtzi/pr/detached-tasks

Introduce the concept of "detached" external commands for the C/Swift API

examples/c-api/buildsystem/main.c

@@ -96,6 +96,8 @@ fancy_tool_create_command(void *context, const llb_data_t* name) {
   delegate.provide_value = fancy_command_provide_value;
   delegate.execute_command = fancy_command_execute_command;
   delegate.execute_command_ex = NULL;
+  delegate.execute_command_detached = NULL;
+  delegate.cancel_detached_command = NULL;
   delegate.is_result_valid = NULL;
   return llb_buildsystem_external_command_create(name, delegate);
 }

include/llbuild/BuildSystem/BuildSystem.h

@@ -300,6 +300,14 @@ class BuildSystem {
   /// Cancel the current build.
   void cancel();
 
+  /// Add cancellation delegate. If the same delegate object was added before
+  /// then the call is a noop.
+  void addCancellationDelegate(core::CancellationDelegate* del);
+
+  /// Remove cancellation delegate. If the delegate was not added or was
+  /// previously removed the call is a noop.
+  void removeCancellationDelegate(core::CancellationDelegate* del);
+
   static uint32_t getSchemaVersion();
   /// @}
 

include/llbuild/BuildSystem/Command.h

@@ -154,6 +154,9 @@ class Command : public basic::JobDescriptor {
 
   virtual bool isExternalCommand() const { return false; }
 
+  /// The command should execute outside the execution lanes.
+  virtual bool isDetached() const { return false; }
+
   virtual void addOutput(BuildNode* node) final {
     outputs.push_back(node);
     node->getProducers().push_back(this);

include/llbuild/Core/BuildEngine.h

@@ -423,6 +423,14 @@ class BuildEngineDelegate {
 
 };
 
+/// Delegate interface for build cancellation notifications.
+class CancellationDelegate {
+public:
+  virtual ~CancellationDelegate();
+
+  virtual void buildCancelled() = 0;
+};
+
 /// A build engine supports fast, incremental, persistent, and parallel
 /// execution of computational graphs.
 ///
@@ -500,7 +508,10 @@ class BuildEngine {
 
   void resetForBuild();
   bool isCancelled();
-
+
+  void addCancellationDelegate(CancellationDelegate* del);
+  void removeCancellationDelegate(CancellationDelegate* del);
+
   /// Attach a database for persisting build state.
   ///
   /// A database should only be attached immediately after creating the engine,

lib/BuildSystem/BuildSystem.cpp

@@ -356,6 +356,14 @@ class BuildSystemImpl {
     return buildEngine.isCancelled();
   }
 
+  void addCancellationDelegate(CancellationDelegate* del) {
+    buildEngine.addCancellationDelegate(del);
+  }
+
+  void removeCancellationDelegate(CancellationDelegate* del) {
+    buildEngine.removeCancellationDelegate(del);
+  }
+
   /// @}
 };
 
@@ -1563,7 +1571,15 @@ class CommandTask : public Task {
         ti.complete(result.toData());
       });
     };
-    ti.spawn({ &command, std::move(fn) });
+    if (command.isDetached()) {
+      struct DetachedContext: public QueueJobContext {
+        unsigned laneID() const override { return -1; }
+      };
+      DetachedContext ctx;
+      fn(&ctx);
+    } else {
+      ti.spawn({ &command, std::move(fn) });
+    }
   }
 
 public:
@@ -4115,6 +4131,18 @@ void BuildSystem::cancel() {
   }
 }
 
+void BuildSystem::addCancellationDelegate(CancellationDelegate* del) {
+  if (impl) {
+    static_cast<BuildSystemImpl*>(impl)->addCancellationDelegate(del);
+  }
+}
+
+void BuildSystem::removeCancellationDelegate(CancellationDelegate* del) {
+  if (impl) {
+    static_cast<BuildSystemImpl*>(impl)->removeCancellationDelegate(del);
+  }
+}
+
 void BuildSystem::resetForBuild() {
   static_cast<BuildSystemImpl*>(impl)->resetForBuild();
 }

lib/Core/BuildEngine.cpp

@@ -18,6 +18,7 @@
 #include "llbuild/Core/BuildDB.h"
 #include "llbuild/Core/KeyID.h"
 
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 
@@ -55,6 +56,8 @@ bool BuildEngineDelegate::shouldResolveCycle(const std::vector<Rule*>& items,
   return false;
 }
 
+CancellationDelegate::~CancellationDelegate() = default;
+
 #pragma mark - BuildEngine implementation
 
 namespace {
@@ -103,6 +106,8 @@ class BuildEngineImpl : public BuildDBDelegate {
   std::atomic<bool> buildRunning{ false };
   std::mutex buildEngineMutex;
 
+  llvm::DenseSet<core::CancellationDelegate *> cancellationDelegates;
+
   /// The queue of input requests to process.
   struct TaskInputRequest {
     /// The task making the request.
@@ -1628,6 +1633,12 @@ class BuildEngineImpl : public BuildDBDelegate {
   void cancelBuild() {
     std::lock_guard<std::mutex> guard(executionQueueMutex);
 
+    if (!buildCancelled) {
+      for (const auto &del : cancellationDelegates) {
+        del->buildCancelled();
+      }
+    }
+
     // Set the build cancelled marker.
     //
     // We do not need to handle waking the engine up, if it is waiting, because
@@ -1646,6 +1657,20 @@ class BuildEngineImpl : public BuildDBDelegate {
     return buildCancelled;
   }
 
+  void addCancellationDelegate(CancellationDelegate* del) {
+    std::lock_guard<std::mutex> guard(executionQueueMutex);
+    if (buildCancelled) {
+      del->buildCancelled();
+      return;
+    }
+    cancellationDelegates.insert(del);
+  }
+
+  void removeCancellationDelegate(CancellationDelegate* del) {
+    std::lock_guard<std::mutex> guard(executionQueueMutex);
+    cancellationDelegates.erase(del);
+  }
+
   bool attachDB(std::unique_ptr<BuildDB> database, std::string* error_out) {
     assert(!db && "invalid attachDB() call");
     assert(currentEpoch == 0 && "invalid attachDB() call");
@@ -1921,6 +1946,14 @@ bool BuildEngine::isCancelled() {
   return static_cast<BuildEngineImpl*>(impl)->isCancelled();
 }
 
+void BuildEngine::addCancellationDelegate(CancellationDelegate* del) {
+  static_cast<BuildEngineImpl*>(impl)->addCancellationDelegate(std::move(del));
+}
+
+void BuildEngine::removeCancellationDelegate(CancellationDelegate* del) {
+  static_cast<BuildEngineImpl*>(impl)->removeCancellationDelegate(del);
+}
+
 void BuildEngine::dumpGraphToFile(const std::string& path) {
   static_cast<BuildEngineImpl*>(impl)->dumpGraphToFile(path);
 }

products/libllbuild/BuildSystem-C-API.cpp

@@ -958,6 +958,26 @@ class CAPIExternalCommand : public ExternalCommand {
   // executeExternalCommand().
   CAPIBuildValue* currentBuildValue = nullptr;
 
+  std::atomic<bool> detachedCommandFinished{false};
+  std::unique_ptr<core::CancellationDelegate> cancellationDelegate = nullptr;
+
+  bool isDetached() const override {
+    return cAPIDelegate.execute_command_detached != nullptr;
+  }
+
+  static ProcessStatus getProcessStatusFromLLBResult(llb_buildsystem_command_result_t result) {
+    switch (result) {
+      case llb_buildsystem_command_result_succeeded:
+        return ProcessStatus::Succeeded;
+      case llb_buildsystem_command_result_failed:
+        return ProcessStatus::Failed;
+      case llb_buildsystem_command_result_cancelled:
+        return ProcessStatus::Cancelled;
+      case llb_buildsystem_command_result_skipped:
+        return ProcessStatus::Skipped;
+    }
+  }
+
   virtual void executeExternalCommand(BuildSystem& system,
                                       core::TaskInterface ti,
                                       QueueJobContext* job_context,
@@ -1001,6 +1021,65 @@ class CAPIExternalCommand : public ExternalCommand {
         completionFn.getValue()(result);
     };
 
+    if (cAPIDelegate.execute_command_detached) {
+      struct ResultCallbackContext {
+        CAPIExternalCommand *thisCommand;
+        BuildSystem *buildSystem;
+        std::function<void(ProcessStatus result)> doneFn;
+
+        static void callback(void* result_ctx,
+                             llb_buildsystem_command_result_t result,
+                             llb_build_value* rvalue) {
+          ResultCallbackContext *ctx = static_cast<ResultCallbackContext*>(result_ctx);
+          auto thisCommand = ctx->thisCommand;
+          BuildSystem &system = *ctx->buildSystem;
+          auto doneFn = std::move(ctx->doneFn);
+          delete ctx;
+
+          thisCommand->detachedCommandFinished = true;
+          if (auto cancellationDelegate = thisCommand->cancellationDelegate.get()) {
+            system.removeCancellationDelegate(cancellationDelegate);
+            thisCommand->cancellationDelegate = nullptr;
+          }
+
+          thisCommand->currentBuildValue = reinterpret_cast<CAPIBuildValue*>(rvalue);
+          llbuild_defer {
+            delete thisCommand->currentBuildValue;
+            thisCommand->currentBuildValue = nullptr;
+          };
+          doneFn(getProcessStatusFromLLBResult(result));
+        }
+      };
+      auto *callbackCtx = new ResultCallbackContext{this, &system, std::move(doneFn)};
+      cAPIDelegate.execute_command_detached(
+        cAPIDelegate.context,
+        (llb_buildsystem_command_t*)this,
+        (llb_buildsystem_interface_t*)&system,
+        *reinterpret_cast<llb_task_interface_t*>(&ti),
+        (llb_buildsystem_queue_job_context_t*)job_context,
+        callbackCtx, ResultCallbackContext::callback);
+
+      if (cAPIDelegate.cancel_detached_command) {
+        class CAPICancellationDelegate: public core::CancellationDelegate {
+          CAPIExternalCommand *thisCommand;
+
+        public:
+          CAPICancellationDelegate(CAPIExternalCommand *thisCommand) : thisCommand(thisCommand) {}
+
+          void buildCancelled() override {
+            if (thisCommand->detachedCommandFinished)
+              return;
+            thisCommand->cAPIDelegate.cancel_detached_command(
+              thisCommand->cAPIDelegate.context,
+              (llb_buildsystem_command_t*)this);
+          }
+        };
+        this->cancellationDelegate = std::make_unique<CAPICancellationDelegate>(this);
+        system.addCancellationDelegate(this->cancellationDelegate.get());
+      }
+      return;
+    }
+
     if (cAPIDelegate.execute_command_ex) {
       llb_build_value* rvalue = cAPIDelegate.execute_command_ex(
         cAPIDelegate.context,
@@ -1031,22 +1110,7 @@ class CAPIExternalCommand : public ExternalCommand {
       (llb_buildsystem_interface_t*)&system,
       *reinterpret_cast<llb_task_interface_t*>(&ti),
       (llb_buildsystem_queue_job_context_t*)job_context);
-    ProcessStatus status;
-    switch (result) {
-      case llb_buildsystem_command_result_succeeded:
-        status = ProcessStatus::Succeeded;
-        break;
-      case llb_buildsystem_command_result_failed:
-        status = ProcessStatus::Failed;
-        break;
-      case llb_buildsystem_command_result_cancelled:
-        status = ProcessStatus::Cancelled;
-        break;
-      case llb_buildsystem_command_result_skipped:
-        status = ProcessStatus::Skipped;
-        break;
-    }
-    doneFn(status);
+    doneFn(getProcessStatusFromLLBResult(result));
   }
 
   BuildValue computeCommandResult(BuildSystem& system, core::TaskInterface ti) override {
@@ -1202,7 +1266,7 @@ llb_buildsystem_external_command_create(
   // Check that all required methods are provided.
   assert(delegate.start);
   assert(delegate.provide_value);
-  assert(delegate.execute_command);
+  assert(delegate.execute_command || delegate.execute_command_detached);
 
   return (llb_buildsystem_command_t*) new CAPIExternalCommand(
       StringRef((const char*)name->data, name->length), delegate);

products/libllbuild/include/llbuild/buildsystem.h

@@ -727,9 +727,10 @@ typedef struct llb_buildsystem_external_command_delegate_t_ {
   /// execution queue, so long as it arranges to only notify the system of
   /// completion once all that work is complete.
   ///
-  /// If defined, the build value returning `execute_command_ex` variant is
-  /// called first. If an 'invalid' buile value is returned, the bindings will
-  /// then try calling the legacy `execute_command` variant if it is defined.
+  /// If defined, the `execute_command_detached` variant is called first.
+  /// The build value returning `execute_command_ex` variant has priority next.
+  /// If an 'invalid' buile value is returned, the bindings will then try
+  /// calling the legacy `execute_command` variant if it is defined.
   ///
   /// The C API takes ownership of the value returned by `execute_command_ex`.
   llb_buildsystem_command_result_t (*execute_command)(void* context,
@@ -744,6 +745,21 @@ typedef struct llb_buildsystem_external_command_delegate_t_ {
                                          llb_task_interface_t ti,
                                          llb_buildsystem_queue_job_context_t* job_context);
 
+  /// Called for the external command to do its work without blocking an
+  /// execution lane. When done the external command should call `result_fn`
+  /// passing a result and optionally a `llb_build_value`.
+  void (*execute_command_detached)(void* context,
+                                llb_buildsystem_command_t* command,
+                                llb_buildsystem_interface_t* bi,
+                                llb_task_interface_t ti,
+                                llb_buildsystem_queue_job_context_t* job_context,
+                                void* result_ctx,
+                                void (*result_fn)(void* result_ctx, llb_buildsystem_command_result_t, llb_build_value*));
+
+  /// If non-NULL and command is 'detached', the build system will call it to
+  /// request the command to cancel when the build is cancelled.
+  void (*cancel_detached_command)(void* context, llb_buildsystem_command_t* command);
+
   /// Called by the build system to determine if the current build result
   /// remains valid.
   ///