Merge meshing and generation thread pools, expecting better usage of available threads

doc/classes/VoxelServer.xml

@@ -16,15 +16,22 @@
 				The returned dictionary has the following structure:
 				[codeblock]
 				{
-					"streaming": {
+					"pools": {
-						"tasks": int,
+						"streaming": {
-						"active_threads": int,
+							"tasks": int,
-						"thread_count": int
+							"active_threads": int,
+							"thread_count": int
+						},
+						"general": {
+							"tasks": int,
+							"active_threads": int,
+							"thread_count": int
+						}
 					},
-					"meshing": {
+					"tasks": {
-						"tasks": int,
+						"streaming": int,
-						"active_threads": int,
+						"meshing": int,
-						"thread_count": int
+						"generation": int
 					}
 				}
 				[/codeblock]

doc/source/changelog.md

@@ -33,6 +33,7 @@ Ongoing development - `master`
 - Breaking changes
     - `VoxelBuffer`: channels `DATA3` and `DATA4` were renamed `INDICES` and `WEIGHTS`
     - `VoxelInstanceGenerator`: `EMIT_FROM_FACES` got renamed `EMIT_FROM_FACES_FAST`. `EMIT_FROM_FACES` still exists but is a different algorithm.
+    - `VoxelServer`: `get_stats()` format has changed, check documentation
 
 - Fixes
     - `VoxelGeneratorGraph`: changes to node properties are now saved properly

editor/terrain/voxel_terrain_editor_plugin.cpp

@@ -42,9 +42,9 @@ public:
 
 	void update_stats(int main_thread_tasks) {
 		const VoxelServer::Stats stats = VoxelServer::get_singleton()->get_stats();
-		set_stat(STAT_STREAM_TASKS, stats.streaming.tasks);
+		set_stat(STAT_STREAM_TASKS, stats.streaming_tasks);
-		set_stat(STAT_GENERATE_TASKS, stats.generation.tasks);
+		set_stat(STAT_GENERATE_TASKS, stats.generation_tasks);
-		set_stat(STAT_MESH_TASKS, stats.meshing.tasks);
+		set_stat(STAT_MESH_TASKS, stats.meshing_tasks);
 		set_stat(STAT_MAIN_THREAD_TASKS, main_thread_tasks);
 	}
 
@@ -71,7 +71,7 @@ private:
 	}
 
 	struct Stat {
-		int value;
+		int value = 0;
 		Label *label = nullptr;
 	};
 

server/voxel_server.cpp

@@ -9,7 +9,11 @@
 
 namespace {
 VoxelServer *g_voxel_server = nullptr;
-}
+// Could be atomics, but it's for debugging so I don't bother for now
+int g_debug_generate_tasks_count = 0;
+int g_debug_stream_tasks_count = 0;
+int g_debug_mesh_tasks_count = 0;
+} // namespace
 
 template <typename T>
 inline std::shared_ptr<T> gd_make_shared() {
@@ -42,18 +46,14 @@ VoxelServer::VoxelServer() {
 	_streaming_thread_pool.set_name("Voxel streaming");
 	_streaming_thread_pool.set_thread_count(1);
 	_streaming_thread_pool.set_priority_update_period(300);
+	// Batching is only to give a chance for file I/O tasks to be grouped and reduce open/close calls.
+	// But in the end it might be better to move this idea to the tasks themselves?
 	_streaming_thread_pool.set_batch_count(16);
 
-	_generation_thread_pool.set_name("Voxel generation");
+	_general_thread_pool.set_name("Voxel general");
-	_generation_thread_pool.set_thread_count(2);
+	_general_thread_pool.set_thread_count(4);
-	_generation_thread_pool.set_priority_update_period(300);
+	_general_thread_pool.set_priority_update_period(200);
-	_generation_thread_pool.set_batch_count(1);
+	_general_thread_pool.set_batch_count(1);
-
-	// This pool works on visuals so it must have lower latency
-	_meshing_thread_pool.set_name("Voxel meshing");
-	_meshing_thread_pool.set_thread_count(2);
-	_meshing_thread_pool.set_priority_update_period(64);
-	_meshing_thread_pool.set_batch_count(1);
 
 	// Init world
 	_world.shared_priority_dependency = gd_make_shared<PriorityDependencyShared>();
@@ -73,13 +73,11 @@ VoxelServer::~VoxelServer() {
 
 void VoxelServer::wait_and_clear_all_tasks(bool warn) {
 	_streaming_thread_pool.wait_for_all_tasks();
-	_generation_thread_pool.wait_for_all_tasks();
+	_general_thread_pool.wait_for_all_tasks();
 
 	// Wait a second time because the generation pool can generate streaming requests
 	_streaming_thread_pool.wait_for_all_tasks();
 
-	_meshing_thread_pool.wait_for_all_tasks();
-
 	_streaming_thread_pool.dequeue_completed_tasks([warn](IVoxelTask *task) {
 		if (warn) {
 			WARN_PRINT("Streaming tasks remain on module cleanup, "
@@ -88,13 +86,9 @@ void VoxelServer::wait_and_clear_all_tasks(bool warn) {
 		memdelete(task);
 	});
 
-	_meshing_thread_pool.dequeue_completed_tasks([](IVoxelTask *task) {
+	_general_thread_pool.dequeue_completed_tasks([warn](IVoxelTask *task) {
-		memdelete(task);
-	});
-
-	_generation_thread_pool.dequeue_completed_tasks([warn](IVoxelTask *task) {
 		if (warn) {
-			WARN_PRINT("Generator tasks remain on module cleanup, "
+			WARN_PRINT("General tasks remain on module cleanup, "
 					   "this could become a problem if they reference scripts");
 		}
 		memdelete(task);
@@ -260,7 +254,7 @@ void VoxelServer::request_block_mesh(uint32_t volume_id, const BlockMeshInput &i
 			r->priority_dependency, input.render_block_position, input.lod, volume, volume.render_block_size);
 
 	// We'll allocate this quite often. If it becomes a problem, it should be easy to pool.
-	_meshing_thread_pool.enqueue(r);
+	_general_thread_pool.enqueue(r);
 }
 
 void VoxelServer::request_block_load(uint32_t volume_id, Vector3i block_pos, int lod, bool request_instances) {
@@ -285,17 +279,16 @@ void VoxelServer::request_block_load(uint32_t volume_id, Vector3i block_pos, int
 		// Directly generate the block without checking the stream
 		ERR_FAIL_COND(volume.stream_dependency->generator.is_null());
 
-		BlockGenerateRequest r;
+		BlockGenerateRequest *r = memnew(BlockGenerateRequest);
-		r.volume_id = volume_id;
+		r->volume_id = volume_id;
-		r.position = block_pos;
+		r->position = block_pos;
-		r.lod = lod;
+		r->lod = lod;
-		r.block_size = volume.data_block_size;
+		r->block_size = volume.data_block_size;
-		r.stream_dependency = volume.stream_dependency;
+		r->stream_dependency = volume.stream_dependency;
 
-		init_priority_dependency(r.priority_dependency, block_pos, lod, volume, volume.data_block_size);
+		init_priority_dependency(r->priority_dependency, block_pos, lod, volume, volume.data_block_size);
 
-		BlockGenerateRequest *rp = memnew(BlockGenerateRequest(r));
+		_general_thread_pool.enqueue(r);
-		_generation_thread_pool.enqueue(rp);
 	}
 }
 
@@ -355,7 +348,7 @@ void VoxelServer::request_block_generate_from_data_request(BlockDataRequest &src
 	r.priority_dependency = src.priority_dependency;
 
 	BlockGenerateRequest *rp = memnew(BlockGenerateRequest(r));
-	_generation_thread_pool.enqueue(rp);
+	_general_thread_pool.enqueue(rp);
 }
 
 void VoxelServer::request_block_save_from_generate_request(BlockGenerateRequest &src) {
@@ -461,14 +454,8 @@ void VoxelServer::process() {
 		memdelete(task);
 	});
 
-	// Receive generation updates
+	// Receive generation and meshing results
-	_generation_thread_pool.dequeue_completed_tasks([this](IVoxelTask *task) {
+	_general_thread_pool.dequeue_completed_tasks([this](IVoxelTask *task) {
-		task->apply_result();
-		memdelete(task);
-	});
-
-	// Receive mesh updates
-	_meshing_thread_pool.dequeue_completed_tasks([this](IVoxelTask *task) {
 		task->apply_result();
 		memdelete(task);
 	});
@@ -519,8 +506,10 @@ static VoxelServer::Stats::ThreadPoolStats debug_get_pool_stats(const VoxelThrea
 VoxelServer::Stats VoxelServer::get_stats() const {
 	Stats s;
 	s.streaming = debug_get_pool_stats(_streaming_thread_pool);
-	s.generation = debug_get_pool_stats(_generation_thread_pool);
+	s.general = debug_get_pool_stats(_general_thread_pool);
-	s.meshing = debug_get_pool_stats(_meshing_thread_pool);
+	s.generation_tasks = g_debug_generate_tasks_count;
+	s.meshing_tasks = g_debug_mesh_tasks_count;
+	s.streaming_tasks = g_debug_stream_tasks_count;
 	return s;
 }
 
@@ -534,6 +523,14 @@ void VoxelServer::_bind_methods() {
 
 //----------------------------------------------------------------------------------------------------------------------
 
+VoxelServer::BlockDataRequest::BlockDataRequest() {
+	++g_debug_stream_tasks_count;
+}
+
+VoxelServer::BlockDataRequest::~BlockDataRequest() {
+	--g_debug_stream_tasks_count;
+}
+
 void VoxelServer::BlockDataRequest::run(VoxelTaskContext ctx) {
 	VOXEL_PROFILE_SCOPE();
 
@@ -684,6 +681,14 @@ void VoxelServer::BlockDataRequest::apply_result() {
 
 //----------------------------------------------------------------------------------------------------------------------
 
+VoxelServer::BlockGenerateRequest::BlockGenerateRequest() {
+	++g_debug_generate_tasks_count;
+}
+
+VoxelServer::BlockGenerateRequest::~BlockGenerateRequest() {
+	--g_debug_generate_tasks_count;
+}
+
 void VoxelServer::BlockGenerateRequest::run(VoxelTaskContext ctx) {
 	VOXEL_PROFILE_SCOPE();
 
@@ -828,6 +833,14 @@ static void copy_block_and_neighbors(Span<Ref<VoxelBuffer>> blocks, VoxelBuffer
 	}
 }
 
+VoxelServer::BlockMeshRequest::BlockMeshRequest() {
+	++g_debug_mesh_tasks_count;
+}
+
+VoxelServer::BlockMeshRequest::~BlockMeshRequest() {
+	--g_debug_mesh_tasks_count;
+}
+
 void VoxelServer::BlockMeshRequest::run(VoxelTaskContext ctx) {
 	VOXEL_PROFILE_SCOPE();
 	CRASH_COND(meshing_dependency == nullptr);

server/voxel_server.h

@@ -152,14 +152,22 @@ public:
 		};
 
 		ThreadPoolStats streaming;
-		ThreadPoolStats generation;
+		ThreadPoolStats general;
-		ThreadPoolStats meshing;
+		int generation_tasks;
+		int streaming_tasks;
+		int meshing_tasks;
 
 		Dictionary to_dict() {
+			Dictionary pools;
+			pools["streaming"] = streaming.to_dict();
+			pools["general"] = streaming.to_dict();
+			Dictionary tasks;
+			tasks["streaming"] = generation_tasks;
+			tasks["generation"] = generation_tasks;
+			tasks["meshing"] = meshing_tasks;
 			Dictionary d;
-			d["streaming"] = streaming.to_dict();
+			d["pools"] = pools;
-			d["generation"] = generation.to_dict();
+			d["tasks"] = tasks;
-			d["meshing"] = meshing.to_dict();
 			return d;
 		}
 	};
@@ -250,6 +258,9 @@ private:
 			TYPE_FALLBACK_ON_GENERATOR
 		};
 
+		BlockDataRequest();
+		~BlockDataRequest();
+
 		void run(VoxelTaskContext ctx) override;
 		int get_priority() override;
 		bool is_cancelled() override;
@@ -274,6 +285,9 @@ private:
 
 	class BlockGenerateRequest : public IVoxelTask {
 	public:
+		BlockGenerateRequest();
+		~BlockGenerateRequest();
+
 		void run(VoxelTaskContext ctx) override;
 		int get_priority() override;
 		bool is_cancelled() override;
@@ -293,6 +307,9 @@ private:
 
 	class BlockMeshRequest : public IVoxelTask {
 	public:
+		BlockMeshRequest();
+		~BlockMeshRequest();
+
 		void run(VoxelTaskContext ctx) override;
 		int get_priority() override;
 		bool is_cancelled() override;
@@ -313,9 +330,10 @@ private:
 	// TODO multi-world support in the future
 	World _world;
 
+	// Pool specialized in file I/O
 	VoxelThreadPool _streaming_thread_pool;
-	VoxelThreadPool _generation_thread_pool;
+	// Pool for every other task
-	VoxelThreadPool _meshing_thread_pool;
+	VoxelThreadPool _general_thread_pool;
 
 	VoxelFileLocker _file_locker;
 };

storage/voxel_buffer.cpp

@@ -160,7 +160,6 @@ void VoxelBuffer::create(Vector3i size) {
 }
 
 void VoxelBuffer::clear() {
-	VOXEL_PROFILE_SCOPE();
 	for (unsigned int i = 0; i < MAX_CHANNELS; ++i) {
 		Channel &channel = _channels[i];
 		if (channel.data) {