Repository: pmmp/PocketMine-MP (language: php)

Issue:
Add timings for AsyncTask onCompletion handlers
<!--- tell us what you want -->
## Description
Currently, timings from AsyncTask completion handlers are dumped into Server Mid-Tick Processing, with some random subtimers from paths they happen to activate (e.g. Player Send Chunks).

We need timings for individual `onCompletion` handlers to make it clear which tasks are doing heavy work on the main thread.

<!--- (optional) describe alternative methods you've explored to achieve your goal -->
## Alternative methods
We can allow plugins to do their own timings, but thus far they haven't been very good at it, and this still doesn't account for core tasks.


Discussion / hints:

Fix patch:
diff --git a/src/scheduler/AsyncPool.php b/src/scheduler/AsyncPool.php
index 6125c62cec3..4cbe0889e92 100644
--- a/src/scheduler/AsyncPool.php
+++ b/src/scheduler/AsyncPool.php
@@ -27,6 +27,7 @@
 use pocketmine\snooze\SleeperHandler;
 use pocketmine\thread\log\ThreadSafeLogger;
 use pocketmine\thread\ThreadSafeClassLoader;
+use pocketmine\timings\Timings;
 use pocketmine\utils\Utils;
 use function array_keys;
 use function array_map;
@@ -231,7 +232,9 @@ public function collectTasksFromWorker(int $worker) : bool{
 
 				if($task->isCrashed()){
 					$this->logger->critical("Could not execute asynchronous task " . (new \ReflectionClass($task))->getShortName() . ": Task crashed");
-					$task->onError();
+					Timings::getAsyncTaskErrorTimings($task)->time(function() use ($task) : void{
+						$task->onError();
+					});
 				}elseif(!$task->hasCancelledRun()){
 					/*
 					 * It's possible for a task to submit a progress update and then finish before the progress
@@ -242,11 +245,13 @@ public function collectTasksFromWorker(int $worker) : bool{
 					 * lost. Thus, it's necessary to do one last check here to make sure all progress updates have
 					 * been consumed before completing.
 					 */
-					$task->checkProgressUpdates();
-					$task->onCompletion();
+					$this->checkTaskProgressUpdates($task);
+					Timings::getAsyncTaskCompletionTimings($task)->time(function() use ($task) : void{
+						$task->onCompletion();
+					});
 				}
 			}else{
-				$task->checkProgressUpdates();
+				$this->checkTaskProgressUpdates($task);
 				$more = true;
 				break; //current task is still running, skip to next worker
 			}
@@ -294,4 +299,10 @@ public function shutdown() : void{
 		}
 		$this->workers = [];
 	}
+
+	private function checkTaskProgressUpdates(AsyncTask $task) : void{
+		Timings::getAsyncTaskProgressUpdateTimings($task)->time(function() use ($task) : void{
+			$task->checkProgressUpdates();
+		});
+	}
 }
diff --git a/src/timings/Timings.php b/src/timings/Timings.php
index aa05ffb2e1b..0d8a256dea5 100644
--- a/src/timings/Timings.php
+++ b/src/timings/Timings.php
@@ -29,6 +29,7 @@
 use pocketmine\network\mcpe\protocol\ClientboundPacket;
 use pocketmine\network\mcpe\protocol\ServerboundPacket;
 use pocketmine\player\Player;
+use pocketmine\scheduler\AsyncTask;
 use pocketmine\scheduler\TaskHandler;
 use function get_class;
 use function str_starts_with;
@@ -115,6 +116,17 @@ abstract class Timings{
 	/** @var TimingsHandler[][] */
 	private static array $eventHandlers = [];
 
+	private static TimingsHandler $asyncTaskProgressUpdateParent;
+	private static TimingsHandler $asyncTaskCompletionParent;
+	private static TimingsHandler $asyncTaskErrorParent;
+
+	/** @var TimingsHandler[] */
+	private static array $asyncTaskProgressUpdate = [];
+	/** @var TimingsHandler[] */
+	private static array $asyncTaskCompletion = [];
+	/** @var TimingsHandler[] */
+	private static array $asyncTaskError = [];
+
 	public static function init() : void{
 		if(self::$initialized){
 			return;
@@ -168,7 +180,11 @@ public static function init() : void{
 		self::$itemEntityBaseTick = new TimingsHandler("Entity Base Tick - ItemEntity", group: self::GROUP_BREAKDOWN);
 
 		self::$schedulerSync = new TimingsHandler("Scheduler - Sync Tasks", group: self::GROUP_BREAKDOWN);
+
 		self::$schedulerAsync = new TimingsHandler("Scheduler - Async Tasks", group: self::GROUP_BREAKDOWN);
+		self::$asyncTaskProgressUpdateParent = new TimingsHandler("Async Tasks - Progress Updates", self::$schedulerAsync, group: self::GROUP_BREAKDOWN);
+		self::$asyncTaskCompletionParent = new TimingsHandler("Async Tasks - Completion Handlers", self::$schedulerAsync, group: self::GROUP_BREAKDOWN);
+		self::$asyncTaskErrorParent = new TimingsHandler("Async Tasks - Error Handlers", self::$schedulerAsync, group: self::GROUP_BREAKDOWN);
 
 		self::$playerCommand = new TimingsHandler("Player Command", group: self::GROUP_BREAKDOWN);
 		self::$craftingDataCacheRebuild = new TimingsHandler("Build CraftingDataPacket Cache", group: self::GROUP_BREAKDOWN);
@@ -299,4 +315,46 @@ public static function getEventHandlerTimings(string $event, string $handlerName
 
 		return self::$eventHandlers[$event][$handlerName];
 	}
+
+	public static function getAsyncTaskProgressUpdateTimings(AsyncTask $task, string $group = self::GROUP_BREAKDOWN) : TimingsHandler{
+		$taskClass = $task::class;
+		if(!isset(self::$asyncTaskProgressUpdate[$taskClass])){
+			self::init();
+			self::$asyncTaskProgressUpdate[$taskClass] = new TimingsHandler(
+				"AsyncTask - " . self::shortenCoreClassName($taskClass, "pocketmine\\") . " - Progress Updates",
+				self::$asyncTaskProgressUpdateParent,
+				$group
+			);
+		}
+
+		return self::$asyncTaskProgressUpdate[$taskClass];
+	}
+
+	public static function getAsyncTaskCompletionTimings(AsyncTask $task, string $group = self::GROUP_BREAKDOWN) : TimingsHandler{
+		$taskClass = $task::class;
+		if(!isset(self::$asyncTaskCompletion[$taskClass])){
+			self::init();
+			self::$asyncTaskCompletion[$taskClass] = new TimingsHandler(
+				"AsyncTask - " . self::shortenCoreClassName($taskClass, "pocketmine\\") . " - Completion Handler",
+				self::$asyncTaskCompletionParent,
+				$group
+			);
+		}
+
+		return self::$asyncTaskCompletion[$taskClass];
+	}
+
+	public static function getAsyncTaskErrorTimings(AsyncTask $task, string $group = self::GROUP_BREAKDOWN) : TimingsHandler{
+		$taskClass = $task::class;
+		if(!isset(self::$asyncTaskError[$taskClass])){
+			self::init();
+			self::$asyncTaskError[$taskClass] = new TimingsHandler(
+				"AsyncTask - " . self::shortenCoreClassName($taskClass, "pocketmine\\") . " - Error Handler",
+				self::$asyncTaskErrorParent,
+				$group
+			);
+		}
+
+		return self::$asyncTaskError[$taskClass];
+	}
 }


Test patch: