better build DepNodes in parallel

src/DepNode.cc

@@ -1,7 +1,6 @@
 #include "DepNode.h"
 
 #include <mutex>
-#include <condition_variable>
 #include <unordered_set>
 
 #include "parallel.h"
@@ -24,108 +23,65 @@ void DepNode::addChild(std::shared_ptr<DepNode> node) {
 	deps_.push_back(std::move(node));
 }
 
-bool DepNode::haveDepsChanged(const std::string &outDir) {
-	auto it = deps_.begin();
-	bool changed = false;
-	int workers = 0;
-	std::mutex mut;
-	std::condition_variable cond;
-
-	std::unique_lock<std::mutex> lock(mut, std::defer_lock);
-	while (it != deps_.end()) {
-		lock.lock();
-		if (changed) {
-			cond.wait(lock, [&] { return workers == 0; });
-			return true;
-		}
-
-		workers += 1;
-		lock.unlock();
-		parallel::run([&, it] {
-			bool ch = (*it)->hasChanged(outDir);
-			std::unique_lock<std::mutex> lock(mut);
-			workers -= 1;
-			if (ch) {
-				changed = true;
-			}
-
-			lock.unlock();
-			cond.notify_one();
-		});
-
-		it = ++it;
-	}
-
-	lock.lock();
-	cond.wait(lock, [&] { return workers == 0; });
-	return changed;
-}
-
 bool DepNode::hasChanged(const std::string &outDir) {
-	std::unique_lock<std::mutex> glock(mut_);
-
-	if (was_rebuilt_) {
-		return true;
-	}
-
 	if (has_changed_ == TriState::TRUE) {
 		return true;
 	} else if (has_changed_ == TriState::FALSE) {
 		return false;
 	} else {
-		bool changed = checkHasChanged(outDir);
-
-		if (!changed) {
-			// Check children without blocking this node
-			glock.unlock();
-			changed = haveDepsChanged(outDir);
-			glock.lock();
+		for (auto &dep: deps_) {
+			if (dep->hasChanged(outDir)) {
+				has_changed_ = TriState::TRUE;
+				return true;
+			}
 		}
 
-		if (changed) {
+		if (checkHasChanged(outDir)) {
 			has_changed_ = TriState::TRUE;
 			return true;
-		} else {
-			has_changed_ = TriState::FALSE;
-			return false;
 		}
+
+		has_changed_ = TriState::FALSE;
+		return false;
 	}
 }
 
-void DepNode::build(const std::string &outDir) {
-	bool changed = hasChanged(outDir);
-	std::unique_lock<std::mutex> glock(mut_);\
+void DepNode::startBuild(const std::string &outDir) {
+	std::unique_lock<std::mutex> lock(mut_);
+	if (build_complete_|| build_started_) {
+		return;
+	}
 
-	if (!was_rebuilt_ && changed) {
-		int workers = 0;
-		std::mutex mut;
-		std::condition_variable cond;
+	if (!hasChanged(outDir)) {
+		return;
+	}
 
-		// Build children without blocking this node
-		glock.unlock();
-		std::unique_lock<std::mutex> lock(mut, std::defer_lock);
-		for (auto &dep: deps_) {
-			if (dep->hasChanged(outDir)) {
-				lock.lock();
-				workers += 1;
-				lock.unlock();
-				parallel::run([&] {
-					dep->build(outDir);
-					std::unique_lock<std::mutex> lock(mut);
-					workers -= 1;
-
-					lock.unlock();
-					cond.notify_one();
-				});
-			}
-		}
+	build_started_ = true;
+	lock.unlock();
 
-		lock.lock();
-		cond.wait(lock, [&] { return workers == 0; });
-		glock.lock();
+	for (auto &dep: deps_) {
+		dep->startBuild(outDir);
+	}
+
+	for (auto &dep: deps_) {
+		dep->joinBuild();
+	}
+
+	parallel::run([&] {
 		doBuild(outDir);
-		was_rebuilt_ = true;
+		std::unique_lock<std::mutex> lock(mut_);
+		build_complete_ = true;
+		build_cond_.notify_all();
+	});
+}
+
+void DepNode::joinBuild() {
+	std::unique_lock<std::mutex> lock(mut_);
+	if (build_complete_ || !build_started_) {
+		return;
 	}
+
+	build_cond_.wait(lock, [=] { return build_complete_; });
 }
 
 void DepNode::writeCompDB(const std::string &outDir, compdb::Writer &w) {

src/DepNode.h

@@ -5,14 +5,20 @@
 #include <mutex>
 #include <string>
 #include <ostream>
+#include <atomic>
+#include <condition_variable>
 
 #include "compdb.h"
 
 class DepNode {
 public:
+	virtual ~DepNode() = default;
+
+	const std::vector<std::shared_ptr<DepNode>> &children() { return deps_; }
 	void addChild(std::shared_ptr<DepNode> node);
 	bool hasChanged(const std::string &outDir);
-	void build(const std::string &outDir);
+	void startBuild(const std::string &outDir);
+	void joinBuild();
 	void writeCompDB(const std::string &outDir, compdb::Writer &w);
 	virtual std::vector<std::string> publicLDFlags(const std::string &outDir);
 	virtual std::vector<std::string> publicLDLibs(const std::string &outDir);
@@ -30,11 +36,10 @@ protected:
 	virtual std::vector<std::string> getPublicLDLibs(const std::string &outDir) { return {}; }
 	virtual std::vector<std::string> getPublicObjects(const std::string &outDir) { return {}; }
 
-	bool haveDepsChanged(const std::string &outDir);
-
-	std::mutex mut_;
 	TriState has_changed_ = TriState::UNKNOWN;
-	bool was_rebuilt_ = false;
-	bool was_prepared_ = false;
+	bool build_complete_ = false;
+	bool build_started_ = false;
+	std::mutex mut_;
+	std::condition_variable build_cond_;
 	std::vector<std::shared_ptr<DepNode>> deps_;
 };

src/main.cc

@@ -149,7 +149,8 @@ int main(int argc, char **argv) {
 
 	// Build the project
 	if (root->hasChanged(outDir)) {
-		root->build(outDir);
+		root->startBuild(outDir);
+		root->joinBuild();
 	} else {
 		logger::log("Nothing to do.");
 	}