Replace thread spawning with tbb pool for adventure AI

AI/Nullkiller/AIGateway.cpp

@@ -68,10 +68,10 @@ struct SetGlobalState
 AIGateway::AIGateway()
 {
 	LOG_TRACE(logAi);
-	makingTurn = nullptr;
 	destinationTeleport = ObjectInstanceID();
 	destinationTeleportPos = int3(-1);
 	nullkiller.reset(new Nullkiller());
+	asyncTasks = std::make_unique<tbb::task_group>();
 }
 
 AIGateway::~AIGateway()
@@ -163,7 +163,7 @@ void AIGateway::showTavernWindow(const CGObjectInstance * object, const CGHeroIn
 	NET_EVENT_HANDLER;
 
 	status.addQuery(queryID, "TavernWindow");
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("showTavernWindow", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void AIGateway::showThievesGuildWindow(const CGObjectInstance * obj)
@@ -296,7 +296,7 @@ void AIGateway::heroExchangeStarted(ObjectInstanceID hero1, ObjectInstanceID her
 
 	status.addQuery(query, boost::str(boost::format("Exchange between heroes %s (%d) and %s (%d)") % firstHero->getNameTranslated() % firstHero->tempOwner % secondHero->getNameTranslated() % secondHero->tempOwner));
 
-	requestActionASAP([this, firstHero, secondHero, query]()
+	executeActionAsync("heroExchangeStarted", [this, firstHero, secondHero, query]()
 	{
 		auto transferFrom2to1 = [this](const CGHeroInstance * h1, const CGHeroInstance * h2) -> void
 		{
@@ -335,7 +335,7 @@ void AIGateway::showRecruitmentDialog(const CGDwelling * dwelling, const CArmedI
 
 	status.addQuery(queryID, "RecruitmentDialog");
 
-	requestActionASAP([this, dwelling, dst, queryID](){
+	executeActionAsync("showRecruitmentDialog", [this, dwelling, dst, queryID](){
 		recruitCreatures(dwelling, dst);
 		answerQuery(queryID, 0);
 	});
@@ -454,7 +454,7 @@ void AIGateway::showUniversityWindow(const IMarket * market, const CGHeroInstanc
 	NET_EVENT_HANDLER;
 
 	status.addQuery(queryID, "UniversityWindow");
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("showUniversityWindow", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void AIGateway::heroManaPointsChanged(const CGHeroInstance * hero)
@@ -530,7 +530,7 @@ void AIGateway::showMarketWindow(const IMarket * market, const CGHeroInstance *
 	NET_EVENT_HANDLER;
 
 	status.addQuery(queryID, "MarketWindow");
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("showMarketWindow", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void AIGateway::showWorldViewEx(const std::vector<ObjectPosInfo> & objectPositions, bool showTerrain)
@@ -586,14 +586,17 @@ void AIGateway::yourTurn(QueryID queryID)
 	NET_EVENT_HANDLER;
 	nullkiller->invalidatePathfinderData();
 	status.addQuery(queryID, "YourTurn");
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("yourTurn", [this, queryID](){ answerQuery(queryID, 0); });
 	status.startedTurn();
 
-	if (makingTurn && makingTurn->joinable())
-		makingTurn->join(); // leftover from previous turn
-
 	nullkiller->makingTurnInterrupption.reset();
-	makingTurn = std::make_unique<std::thread>(&AIGateway::makeTurn, this);
+
+	asyncTasks->run([this]()
+	{
+		ScopedThreadName guard("NKAI::makingTurn");
+		makeTurn();
+	});
+	tbb::this_task_arena::enqueue([this](){asyncTasks->wait();});
 }
 
 void AIGateway::heroGotLevel(const CGHeroInstance * hero, PrimarySkill pskill, std::vector<SecondarySkill> & skills, QueryID queryID)
@@ -604,7 +607,7 @@ void AIGateway::heroGotLevel(const CGHeroInstance * hero, PrimarySkill pskill, s
 	status.addQuery(queryID, boost::str(boost::format("Hero %s got level %d") % hero->getNameTranslated() % hero->level));
 	HeroPtr hPtr = hero;
 
-	requestActionASAP([this, hPtr, skills, queryID]()
+	executeActionAsync("heroGotLevel", [this, hPtr, skills, queryID]()
 	{ 
 		int sel = 0;
 
@@ -626,7 +629,7 @@ void AIGateway::commanderGotLevel(const CCommanderInstance * commander, std::vec
 	LOG_TRACE_PARAMS(logAi, "queryID '%i'", queryID);
 	NET_EVENT_HANDLER;
 	status.addQuery(queryID, boost::str(boost::format("Commander %s of %s got level %d") % commander->name % commander->armyObj->nodeName() % (int)commander->level));
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("commanderGotLevel", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void AIGateway::showBlockingDialog(const std::string & text, const std::vector<Component> & components, QueryID askID, const int soundID, bool selection, bool cancel, bool safeToAutoaccept)
@@ -641,7 +644,7 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 
 	if(!selection && cancel)
 	{
-		requestActionASAP([this, hero, target, askID]()
+		executeActionAsync("showBlockingDialog", [this, hero, target, askID]()
 		{
 			//yes&no -> always answer yes, we are a brave AI :)
 			bool answer = true;
@@ -689,7 +692,7 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 		return;
 	}
 
-	requestActionASAP([this, selection, components, hero, askID]()
+	executeActionAsync("showBlockingDialog", [this, selection, components, hero, askID]()
 	{
 		int sel = 0;
 
@@ -751,7 +754,7 @@ void AIGateway::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelI
 		}
 	}
 
-	requestActionASAP([this, askID, chosenExit]()
+	executeActionAsync("showTeleportDialog", [this, askID, chosenExit]()
 	{
 		answerQuery(askID, chosenExit);
 	});
@@ -768,7 +771,7 @@ void AIGateway::showGarrisonDialog(const CArmedInstance * up, const CGHeroInstan
 	status.addQuery(queryID, boost::str(boost::format("Garrison dialog with %s and %s") % s1 % s2));
 
 	//you can't request action from action-response thread
-	requestActionASAP([this, up, down, removableUnits, queryID]()
+	executeActionAsync("showGarrisonDialog", [this, up, down, removableUnits, queryID]()
 	{
 		if(removableUnits && up->tempOwner == down->tempOwner && nullkiller->settings->isGarrisonTroopsUsageAllowed() && !cb->getStartInfo()->isRestorationOfErathiaCampaign())
 		{
@@ -783,7 +786,7 @@ void AIGateway::showMapObjectSelectDialog(QueryID askID, const Component & icon,
 {
 	NET_EVENT_HANDLER;
 	status.addQuery(askID, "Map object select query");
-	requestActionASAP([this, askID](){ answerQuery(askID, selectedObject.getNum()); });
+	executeActionAsync("showMapObjectSelectDialog", [this, askID](){ answerQuery(askID, selectedObject.getNum()); });
 }
 
 bool AIGateway::makePossibleUpgrades(const CArmedInstance * obj)
@@ -1218,10 +1221,10 @@ void AIGateway::battleEnd(const BattleID & battleID, const BattleResult * br, Qu
 	{
 		status.addQuery(queryID, "Confirm battle query");
 
-		requestActionASAP([this, queryID]()
-			{
-				answerQuery(queryID, 0);
-			});
+		executeActionAsync("battleEnd", [this, queryID]()
+		{
+			answerQuery(queryID, 0);
+		});
 	}
 }
 
@@ -1592,24 +1595,26 @@ void AIGateway::finish()
 {
 	nullkiller->makingTurnInterrupption.interruptThread();
 
-	if(makingTurn)
+	if (asyncTasks)
 	{
-		makingTurn->join();
-		makingTurn.reset();
+		asyncTasks->wait();
+		asyncTasks.reset();
 	}
 }
 
-void AIGateway::requestActionASAP(std::function<void()> whatToDo)
+void AIGateway::executeActionAsync(const std::string & description, const std::function<void()> & whatToDo)
 {
-	std::thread newThread([this, whatToDo]()
+	if (!asyncTasks)
+		throw std::runtime_error("Attempt to execute task on shut down AI state!");
+
+	asyncTasks->run([this, description, whatToDo]()
 	{
-		setThreadName("AIGateway::requestActionASAP::whatToDo");
+		ScopedThreadName guard("NKAI::" + description);
 		SET_GLOBAL_STATE(this);
 		std::shared_lock gsLock(CGameState::mutex);
 		whatToDo();
 	});
-
-	newThread.detach();
+	tbb::this_task_arena::enqueue([this](){asyncTasks->wait();});
 }
 
 void AIGateway::lostHero(HeroPtr h)

AI/Nullkiller/AIGateway.h

@@ -22,6 +22,9 @@
 #include "Pathfinding/AIPathfinder.h"
 #include "Engine/Nullkiller.h"
 
+#include <tbb/task_group.h>
+#include <tbb/task_arena.h>
+
 namespace NKAI
 {
 
@@ -71,7 +74,7 @@ public:
 	AIStatus status;
 	std::string battlename;
 	std::shared_ptr<CCallback> myCb;
-	std::unique_ptr<std::thread> makingTurn;
+	std::unique_ptr<tbb::task_group> asyncTasks;
 
 public:
 	ObjectInstanceID selectedObject;
@@ -79,7 +82,7 @@ public:
 	std::unique_ptr<Nullkiller> nullkiller;
 
 	AIGateway();
-	virtual ~AIGateway();
+	~AIGateway();
 
 	//TODO: extract to appropriate goals
 	void tryRealize(Goals::DigAtTile & g);
@@ -178,7 +181,7 @@ public:
 	void requestSent(const CPackForServer * pack, int requestID) override;
 	void answerQuery(QueryID queryID, int selection);
 	//special function that can be called ONLY from game events handling thread and will send request ASAP
-	void requestActionASAP(std::function<void()> whatToDo);
+	void executeActionAsync(const std::string & description, const std::function<void()> & whatToDo);
 };
 
 }

AI/VCAI/VCAI.cpp

@@ -76,7 +76,7 @@ struct SetGlobalState
 VCAI::VCAI()
 {
 	LOG_TRACE(logAi);
-	makingTurn = nullptr;
+	asyncTasks = std::make_unique<tbb::task_group>();
 	destinationTeleport = ObjectInstanceID();
 	destinationTeleportPos = int3(-1);
 
@@ -175,7 +175,7 @@ void VCAI::showTavernWindow(const CGObjectInstance * object, const CGHeroInstanc
 	NET_EVENT_HANDLER;
 
 	status.addQuery(queryID, "TavernWindow");
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("showTavernWindow", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void VCAI::showThievesGuildWindow(const CGObjectInstance * obj)
@@ -312,7 +312,7 @@ void VCAI::heroExchangeStarted(ObjectInstanceID hero1, ObjectInstanceID hero2, Q
 
 	status.addQuery(query, boost::str(boost::format("Exchange between heroes %s (%d) and %s (%d)") % firstHero->getNameTranslated() % firstHero->tempOwner % secondHero->getNameTranslated() % secondHero->tempOwner));
 
-	requestActionASAP([this, firstHero, secondHero, query]()
+	executeActionAsync("heroExchangeStarted", [this, firstHero, secondHero, query]()
 	{
 		float goalpriority1 = 0;
 		float goalpriority2 = 0;
@@ -371,7 +371,7 @@ void VCAI::showRecruitmentDialog(const CGDwelling * dwelling, const CArmedInstan
 	NET_EVENT_HANDLER;
 
 	status.addQuery(queryID, "RecruitmentDialog");
-	requestActionASAP([this, dwelling, dst, queryID](){
+	executeActionAsync("showRecruitmentDialog", [this, dwelling, dst, queryID](){
 		recruitCreatures(dwelling, dst);
 		checkHeroArmy(dynamic_cast<const CGHeroInstance*>(dst));
 		answerQuery(queryID, 0);
@@ -470,7 +470,7 @@ void VCAI::showHillFortWindow(const CGObjectInstance * object, const CGHeroInsta
 	LOG_TRACE(logAi);
 	NET_EVENT_HANDLER;
 
-	requestActionASAP([=]()
+	executeActionAsync("showHillFortWindow", [=]()
 	{
 		makePossibleUpgrades(visitor);
 	});
@@ -533,7 +533,7 @@ void VCAI::showUniversityWindow(const IMarket * market, const CGHeroInstance * v
 	NET_EVENT_HANDLER;
 
 	status.addQuery(queryID, "UniversityWindow");
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("showUniversityWindow", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void VCAI::heroManaPointsChanged(const CGHeroInstance * hero)
@@ -601,7 +601,7 @@ void VCAI::showMarketWindow(const IMarket * market, const CGHeroInstance * visit
 	NET_EVENT_HANDLER;
 
 	status.addQuery(queryID, "MarketWindow");
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("showMarketWindow", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void VCAI::showWorldViewEx(const std::vector<ObjectPosInfo> & objectPositions, bool showTerrain)
@@ -647,14 +647,16 @@ void VCAI::yourTurn(QueryID queryID)
 	LOG_TRACE_PARAMS(logAi, "queryID '%i'", queryID);
 	NET_EVENT_HANDLER;
 	status.addQuery(queryID, "YourTurn");
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("yourTurn", [this, queryID](){ answerQuery(queryID, 0); });
 	status.startedTurn();
 
-	if (makingTurn && makingTurn->joinable())
-		makingTurn->join(); // leftover from previous turn
-
 	makingTurnInterrupption.reset();
-	makingTurn = std::make_unique<std::thread>(&VCAI::makeTurn, this);
+	asyncTasks->run([this]()
+	{
+		ScopedThreadName guard("VCAI::makingTurn");
+		makeTurn();
+	});
+	tbb::this_task_arena::enqueue([this](){asyncTasks->wait();});
 }
 
 void VCAI::heroGotLevel(const CGHeroInstance * hero, PrimarySkill pskill, std::vector<SecondarySkill> & skills, QueryID queryID)
@@ -662,7 +664,7 @@ void VCAI::heroGotLevel(const CGHeroInstance * hero, PrimarySkill pskill, std::v
 	LOG_TRACE_PARAMS(logAi, "queryID '%i'", queryID);
 	NET_EVENT_HANDLER;
 	status.addQuery(queryID, boost::str(boost::format("Hero %s got level %d") % hero->getNameTranslated() % hero->level));
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("heroGotLevel", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void VCAI::commanderGotLevel(const CCommanderInstance * commander, std::vector<ui32> skills, QueryID queryID)
@@ -670,7 +672,7 @@ void VCAI::commanderGotLevel(const CCommanderInstance * commander, std::vector<u
 	LOG_TRACE_PARAMS(logAi, "queryID '%i'", queryID);
 	NET_EVENT_HANDLER;
 	status.addQuery(queryID, boost::str(boost::format("Commander %s of %s got level %d") % commander->name % commander->armyObj->nodeName() % (int)commander->level));
-	requestActionASAP([this, queryID](){ answerQuery(queryID, 0); });
+	executeActionAsync("commanderGotLevel", [this, queryID](){ answerQuery(queryID, 0); });
 }
 
 void VCAI::showBlockingDialog(const std::string & text, const std::vector<Component> & components, QueryID askID, const int soundID, bool selection, bool cancel, bool safeToAutoaccept)
@@ -687,7 +689,7 @@ void VCAI::showBlockingDialog(const std::string & text, const std::vector<Compon
 	if(!selection && cancel) //yes&no -> always answer yes, we are a brave AI :)
 		sel = 1;
 
-	requestActionASAP([this, askID, sel]()
+	executeActionAsync("showBlockingDialog", [this, askID, sel]()
 	{
 		answerQuery(askID, sel);
 	});
@@ -732,7 +734,7 @@ void VCAI::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID cha
 		}
 	}
 
-	requestActionASAP([this, askID, chosenExit]()
+	executeActionAsync("showTeleportDialog", [this, askID, chosenExit]()
 	{
 		answerQuery(askID, chosenExit);
 	});
@@ -749,7 +751,7 @@ void VCAI::showGarrisonDialog(const CArmedInstance * up, const CGHeroInstance *
 	status.addQuery(queryID, boost::str(boost::format("Garrison dialog with %s and %s") % s1 % s2));
 
 	//you can't request action from action-response thread
-	requestActionASAP([this, down, up, removableUnits, queryID]()
+	executeActionAsync("showGarrisonDialog", [this, down, up, removableUnits, queryID]()
 	{
 		if(removableUnits && !cb->getStartInfo()->isRestorationOfErathiaCampaign())
 			pickBestCreatures(down, up);
@@ -762,7 +764,7 @@ void VCAI::showMapObjectSelectDialog(QueryID askID, const Component & icon, cons
 {
 	NET_EVENT_HANDLER;
 	status.addQuery(askID, "Map object select query");
-	requestActionASAP([this, askID](){ answerQuery(askID, selectedObject.getNum()); });
+	executeActionAsync("showMapObjectSelectDialog", [this, askID](){ answerQuery(askID, selectedObject.getNum()); });
 }
 
 void makePossibleUpgrades(const CArmedInstance * obj)
@@ -2495,24 +2497,26 @@ void VCAI::finish()
 {
 	makingTurnInterrupption.interruptThread();
 
-	if(makingTurn)
+	if (asyncTasks)
 	{
-		makingTurn->join();
-		makingTurn.reset();
+		asyncTasks->wait();
+		asyncTasks.reset();
 	}
 }
 
-void VCAI::requestActionASAP(std::function<void()> whatToDo)
+void VCAI::executeActionAsync(const std::string & description, const std::function<void()> & whatToDo)
 {
-	std::thread newThread([this, whatToDo]()
+	if (!asyncTasks)
+		throw std::runtime_error("Attempt to execute task on shut down AI state!");
+
+	asyncTasks->run([this, description, whatToDo]()
 	{
-		setThreadName("VCAI::requestActionASAP::whatToDo");
+		ScopedThreadName guard("VCAI::" + description);
 		SET_GLOBAL_STATE(this);
 		std::shared_lock gsLock(CGameState::mutex);
 		whatToDo();
 	});
-
-	newThread.detach();
+	tbb::this_task_arena::enqueue([this](){asyncTasks->wait();});
 }
 
 void VCAI::lostHero(HeroPtr h)

AI/VCAI/VCAI.h

@@ -23,6 +23,9 @@
 #include "../../lib/spells/CSpellHandler.h"
 #include "Pathfinding/AIPathfinder.h"
 
+#include <tbb/task_group.h>
+#include <tbb/task_arena.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 struct QuestInfo;
@@ -105,7 +108,7 @@ public:
 
 	std::shared_ptr<CCallback> myCb;
 
-	std::unique_ptr<std::thread> makingTurn;
+	std::unique_ptr<tbb::task_group> asyncTasks;
 	ThreadInterruption makingTurnInterrupption;
 
 public:
@@ -262,7 +265,7 @@ public:
 	void requestSent(const CPackForServer * pack, int requestID) override;
 	void answerQuery(QueryID queryID, int selection);
 	//special function that can be called ONLY from game events handling thread and will send request ASAP
-	void requestActionASAP(std::function<void()> whatToDo);
+	void executeActionAsync(const std::string & description, const std::function<void()> & whatToDo);
 };
 
 class cannotFulfillGoalException : public std::exception

client/renderSDL/SDLImage.cpp

@@ -256,8 +256,6 @@ std::shared_ptr<const ISharedImage> SDLImageShared::scaleInteger(int factor, SDL
 
 SDLImageShared::SDLImageShared(const SDLImageShared * from, int integerScaleFactor, EScalingAlgorithm algorithm)
 {
-	static tbb::task_arena upscalingArena;
-
 	upscalingInProgress = true;
 
 	auto scaler = std::make_shared<SDLImageScaler>(from->surf, Rect(from->margins, from->fullSize), true);
@@ -273,7 +271,7 @@ SDLImageShared::SDLImageShared(const SDLImageShared * from, int integerScaleFact
 	};
 
 	if(settings["video"]["asyncUpscaling"].Bool())
-		upscalingArena.enqueue(scalingTask);
+		tbb::this_task_arena::enqueue(scalingTask);
 	else
 		scalingTask();
 }

docs/developers/Code_Structure.md

@@ -101,13 +101,15 @@ If you're creating new project part, place `VCMI_LIB_USING_NAMESPACE` in its `St
 
 ## Artificial Intelligence
 
-### StupidAI
+### Combat AI
 
-Stupid AI is recent and used battle AI.
+- Battle AI is recent and used combat AI.
+- Stupid AI is old and deprecated version of combat AI
 
 ### Adventure AI
 
-VCAI module is currently developed agent-based system driven by goals and heroes.
+- NullkillerAI module is currently developed agent-based system driven by goals and heroes.
+- VCAI is old and deprecated version of adventure map AI
 
 ## Threading Model
 
@@ -131,13 +133,11 @@ Here is list of threads including their name that can be seen in logging or in d
 - NullkillerAI parallelizes a lot of its tasks using TBB methods, mostly parallel_for
 - Random map generator actively uses thread pool provided by TBB
 - Client performs image upscaling in background thread to avoid visible freezes
+- AI main task (`NKAI::makeTurn`). This TBB task is created whenever AI stars a new turn, and ends when AI ends its turn. Majority of AI event processing is done in this thread, however some actions are either offloaded entirely as tbb task, or parallelized using methods like parallel_for.
+- AI helper tasks (`NKAI::<various>`). Adventure AI creates such tasks whenever it receives event that requires processing without locking network thread that initiated the call.
 
 ## Short-living threads
 
-- AI thread (`AIGateway::makeTurn`). Adventure AI creates its thread whenever it stars a new turn, and terminates it when turn ends. Majority of AI event processing is done in this thread, however some actions are either offloaded entirely as tbb task, or parallelized using methods like parallel_for.
-
-- AI helper thread (`AIGateway::doActionASAP`). Adventure AI creates such thread whenever it receives event that requires processing without locking network thread that initiated the call.
-
 - Autocombat initiation thread (`autofightingAI`). Combat AI usually runs on network thread, as reaction on unit taking turn netpack event. However initial activation of AI when player presses hotkey or button is done in input processing (`MainGUI`) thread. To avoid freeze when AI selects its first action, this action is done on a temporary thread
 
 - Initializition thread (`initialize`). On game start, to avoid delay in game loading, most of game library initialization is done in separate thread while main thread is playing intro movies.

lib/CThreadHelper.h

@@ -21,4 +21,17 @@ void DLL_LINKAGE setThreadNameLoggingOnly(const std::string &name);
 /// Returns human-readable thread name that was set before, or string form of system-provided thread ID if no human-readable name was set
 std::string DLL_LINKAGE getThreadName();
 
+class DLL_LINKAGE ScopedThreadName : boost::noncopyable
+{
+public:
+	ScopedThreadName(const std::string & name)
+	{
+		setThreadName(name);
+	}
+	~ScopedThreadName()
+	{
+		setThreadName({});
+	}
+};
+
 VCMI_LIB_NAMESPACE_END