Remove mapblock cache for mesh generation. (#13124)

Reduces memory consumption and improves performance

src/client/client.cpp

@@ -339,8 +339,12 @@ Client::~Client()
 	m_mesh_update_manager.wait();
 	
 	MeshUpdateResult r;
-	while (m_mesh_update_manager.getNextResult(r))
+	while (m_mesh_update_manager.getNextResult(r)) {
+		for (auto block : r.map_blocks)
+			if (block)
+				block->refDrop();
 		delete r.mesh;
+	}
 
 	delete m_inventory_from_server;
 
@@ -595,6 +599,10 @@ void Client::step(float dtime)
 
 				blocks_to_ack.emplace_back(r.p);
 			}
+
+			for (auto block : r.map_blocks)
+				if (block)
+					block->refDrop();
 		}
 		if (blocks_to_ack.size() > 0) {
 				// Acknowledge block(s)

src/client/mesh_generator_thread.cpp

@@ -25,17 +25,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "map.h"
 #include "util/directiontables.h"
 
-/*
-	CachedMapBlockData
-*/
-
-CachedMapBlockData::~CachedMapBlockData()
-{
-	assert(refcount_from_queue == 0);
-
-	delete[] data;
-}
-
 /*
 	QueuedMeshUpdate
 */
@@ -50,8 +39,7 @@ QueuedMeshUpdate::~QueuedMeshUpdate()
 */
 
 MeshUpdateQueue::MeshUpdateQueue(Client *client):
-	m_client(client),
+	m_client(client)
-	m_next_cache_cleanup(0)
 {
 	m_cache_enable_shaders = g_settings->getBool("enable_shaders");
 	m_cache_smooth_lighting = g_settings->getBool("smooth_lighting");
@@ -62,39 +50,22 @@ MeshUpdateQueue::~MeshUpdateQueue()
 {
 	MutexAutoLock lock(m_mutex);
 
-	for (auto &i : m_cache) {
-		delete i.second;
-	}
-
 	for (QueuedMeshUpdate *q : m_queue) {
+		for (auto block : q->map_blocks)
+			if (block)
+				block->refDrop();
 		delete q;
 	}
 }
 
 bool MeshUpdateQueue::addBlock(Map *map, v3s16 p, bool ack_block_to_server, bool urgent)
 {
+	MapBlock *main_block = map->getBlockNoCreateNoEx(p);
+	if (!main_block)
+		return false;
+
 	MutexAutoLock lock(m_mutex);
 
-	cleanupCache();
-
-	/*
-		Cache the block data (force-update the center block, don't update the
-		neighbors but get them if they aren't already cached)
-	*/
-	std::vector<CachedMapBlockData*> cached_blocks;
-	size_t cache_hit_counter = 0;
-	CachedMapBlockData *cached_block = cacheBlock(map, p, FORCE_UPDATE);
-	if (!cached_block->data)
-		return false; // nothing to update
-	cached_blocks.reserve(3*3*3);
-	cached_blocks.push_back(cached_block);
-	for (v3s16 dp : g_26dirs)
-		cached_blocks.push_back(cacheBlock(map, p + dp,
-				SKIP_UPDATE_IF_ALREADY_CACHED,
-				&cache_hit_counter));
-	g_profiler->avg("MeshUpdateQueue: MapBlocks from cache [%]",
-			100.0f * cache_hit_counter / cached_blocks.size());
-
 	/*
 		Mark the block as urgent if requested
 	*/
@@ -114,10 +85,34 @@ bool MeshUpdateQueue::addBlock(Map *map, v3s16 p, bool ack_block_to_server, bool
 			q->crack_level = m_client->getCrackLevel();
 			q->crack_pos = m_client->getCrackPos();
 			q->urgent |= urgent;
+			for (std::size_t i = 1; i < q->map_blocks.size(); i++) {
+				if (!q->map_blocks[i]) {
+					MapBlock *block = map->getBlockNoCreateNoEx(q->p + g_26dirs[i - 1]);
+					if (block) {
+						block->refGrab();
+						q->map_blocks[i] = block;
+					}
+				}
+			}
 			return true;
 		}
 	}
 
+	/*
+		Cache the block data (force-update the center block, don't update the
+		neighbors but get them if they aren't already cached)
+	*/
+	std::vector<MapBlock *> cached_blocks;
+	cached_blocks.reserve(3*3*3);
+	cached_blocks.push_back(main_block);
+	main_block->refGrab();
+	for (v3s16 dp : g_26dirs) {
+		MapBlock *block = map->getBlockNoCreateNoEx(p + dp);
+		cached_blocks.push_back(block);
+		if (block)
+			block->refGrab();
+	}
+
 	/*
 		Add the block
 	*/
@@ -127,12 +122,9 @@ bool MeshUpdateQueue::addBlock(Map *map, v3s16 p, bool ack_block_to_server, bool
 	q->crack_level = m_client->getCrackLevel();
 	q->crack_pos = m_client->getCrackPos();
 	q->urgent = urgent;
+	q->map_blocks = std::move(cached_blocks);
 	m_queue.push_back(q);
 
-	// This queue entry is a new reference to the cached blocks
-	for (CachedMapBlockData *cached_block : cached_blocks) {
-		cached_block->refcount_from_queue++;
-	}
 	return true;
 }
 
@@ -140,6 +132,8 @@ bool MeshUpdateQueue::addBlock(Map *map, v3s16 p, bool ack_block_to_server, bool
 // Returns NULL if queue is empty
 QueuedMeshUpdate *MeshUpdateQueue::pop()
 {
+	QueuedMeshUpdate *result = NULL;
+	{
 		MutexAutoLock lock(m_mutex);
 
 		bool must_be_urgent = !m_urgents.empty();
@@ -154,10 +148,15 @@ QueuedMeshUpdate *MeshUpdateQueue::pop()
 			m_queue.erase(i);
 			m_urgents.erase(q->p);
 			m_inflight_blocks.insert(q->p);
-		fillDataFromMapBlockCache(q);
+			result = q;
-		return q;
+			break;
 		}
-	return NULL;
+	}
+
+	if (result)
+		fillDataFromMapBlocks(result);
+
+	return result;
 }
 
 void MeshUpdateQueue::done(v3s16 pos)
@@ -166,113 +165,22 @@ void MeshUpdateQueue::done(v3s16 pos)
 	m_inflight_blocks.erase(pos);
 }
 
-CachedMapBlockData* MeshUpdateQueue::cacheBlock(Map *map, v3s16 p, UpdateMode mode,
-			size_t *cache_hit_counter)
-{
-	CachedMapBlockData *cached_block = nullptr;
-	auto it = m_cache.find(p);
 
-	if (it != m_cache.end()) {
+void MeshUpdateQueue::fillDataFromMapBlocks(QueuedMeshUpdate *q)
-		cached_block = it->second;
-
-		if (mode == SKIP_UPDATE_IF_ALREADY_CACHED) {
-			if (cache_hit_counter)
-				(*cache_hit_counter)++;
-			return cached_block;
-		}
-	}
-
-	if (!cached_block) {
-		// Not yet in cache
-		cached_block = new CachedMapBlockData();
-		m_cache[p] = cached_block;
-	}
-
-	MapBlock *b = map->getBlockNoCreateNoEx(p);
-	if (b) {
-		if (!cached_block->data)
-			cached_block->data =
-					new MapNode[MAP_BLOCKSIZE * MAP_BLOCKSIZE * MAP_BLOCKSIZE];
-		memcpy(cached_block->data, b->getData(),
-				MAP_BLOCKSIZE * MAP_BLOCKSIZE * MAP_BLOCKSIZE * sizeof(MapNode));
-	} else {
-		delete[] cached_block->data;
-		cached_block->data = nullptr;
-	}
-	return cached_block;
-}
-
-CachedMapBlockData* MeshUpdateQueue::getCachedBlock(const v3s16 &p)
-{
-	auto it = m_cache.find(p);
-	if (it != m_cache.end()) {
-		return it->second;
-	}
-	return NULL;
-}
-
-void MeshUpdateQueue::fillDataFromMapBlockCache(QueuedMeshUpdate *q)
 {
 	MeshMakeData *data = new MeshMakeData(m_client, m_cache_enable_shaders);
 	q->data = data;
 
 	data->fillBlockDataBegin(q->p);
 
-	std::time_t t_now = std::time(0);
+	for (MapBlock *block : q->map_blocks)
-
+		if (block)
-	// Collect data for 3*3*3 blocks from cache
+			data->fillBlockData(block->getPos() - q->p, block->getData());
-	for (v3s16 dp : g_27dirs) {
-		v3s16 p = q->p + dp;
-		CachedMapBlockData *cached_block = getCachedBlock(p);
-		if (cached_block) {
-			cached_block->refcount_from_queue--;
-			cached_block->last_used_timestamp = t_now;
-			if (cached_block->data)
-				data->fillBlockData(dp, cached_block->data);
-		}
-	}
 
 	data->setCrack(q->crack_level, q->crack_pos);
 	data->setSmoothLighting(m_cache_smooth_lighting);
 }
 
-void MeshUpdateQueue::cleanupCache()
-{
-	const int mapblock_kB = MAP_BLOCKSIZE * MAP_BLOCKSIZE * MAP_BLOCKSIZE *
-			sizeof(MapNode) / 1000;
-	g_profiler->avg("MeshUpdateQueue MapBlock cache size kB",
-			mapblock_kB * m_cache.size());
-
-	// Iterating the entire cache can get pretty expensive so don't do it too often
-	{
-		constexpr int cleanup_interval = 250;
-		const u64 now = porting::getTimeMs();
-		if (m_next_cache_cleanup > now)
-			return;
-		m_next_cache_cleanup = now + cleanup_interval;
-	}
-
-	// The cache size is kept roughly below cache_soft_max_size, not letting
-	// anything get older than cache_seconds_max or deleted before 2 seconds.
-	const int cache_seconds_max = 10;
-	const int cache_soft_max_size = m_meshgen_block_cache_size * 1000 / mapblock_kB;
-	int cache_seconds = MYMAX(2, cache_seconds_max -
-			m_cache.size() / (cache_soft_max_size / cache_seconds_max));
-
-	int t_now = time(0);
-
-	for (auto it = m_cache.begin(); it != m_cache.end(); ) {
-		CachedMapBlockData *cached_block = it->second;
-		if (cached_block->refcount_from_queue == 0 &&
-				cached_block->last_used_timestamp < t_now - cache_seconds) {
-			it = m_cache.erase(it);
-			delete cached_block;
-		} else {
-			++it;
-		}
-	}
-}
-
 /*
 	MeshUpdateWorkerThread
 */
@@ -302,6 +210,7 @@ void MeshUpdateWorkerThread::doUpdate()
 		r.solid_sides = get_solid_sides(q->data);
 		r.ack_block_to_server = q->ack_block_to_server;
 		r.urgent = q->urgent;
+		r.map_blocks = q->map_blocks;
 
 		m_manager->putResult(r);
 		m_queue_in->done(q->p);

src/client/mesh_generator_thread.h

@@ -29,17 +29,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <vector>
 #include <memory>
 
-struct CachedMapBlockData
-{
-	v3s16 p = v3s16(-1337, -1337, -1337);
-	MapNode *data = nullptr; // A copy of the MapBlock's data member
-	int refcount_from_queue = 0;
-	std::time_t last_used_timestamp = std::time(0);
-
-	CachedMapBlockData() = default;
-	~CachedMapBlockData();
-};
-
 struct QueuedMeshUpdate
 {
 	v3s16 p = v3s16(-1337, -1337, -1337);
@@ -47,6 +36,7 @@ struct QueuedMeshUpdate
 	int crack_level = -1;
 	v3s16 crack_pos;
 	MeshMakeData *data = nullptr; // This is generated in MeshUpdateQueue::pop()
+	std::vector<MapBlock *> map_blocks;
 	bool urgent = false;
 
 	QueuedMeshUpdate() = default;
@@ -90,9 +80,7 @@ private:
 	Client *m_client;
 	std::vector<QueuedMeshUpdate *> m_queue;
 	std::unordered_set<v3s16> m_urgents;
-	std::unordered_map<v3s16, CachedMapBlockData *> m_cache;
 	std::unordered_set<v3s16> m_inflight_blocks;
-	u64 m_next_cache_cleanup; // milliseconds
 	std::mutex m_mutex;
 
 	// TODO: Add callback to update these when g_settings changes
@@ -100,10 +88,7 @@ private:
 	bool m_cache_smooth_lighting;
 	int m_meshgen_block_cache_size;
 
-	CachedMapBlockData *cacheBlock(Map *map, v3s16 p, UpdateMode mode,
+	void fillDataFromMapBlocks(QueuedMeshUpdate *q);
-			size_t *cache_hit_counter = NULL);
-	CachedMapBlockData *getCachedBlock(const v3s16 &p);
-	void fillDataFromMapBlockCache(QueuedMeshUpdate *q);
 	void cleanupCache();
 };
 
@@ -114,6 +99,7 @@ struct MeshUpdateResult
 	u8 solid_sides = 0;
 	bool ack_block_to_server = false;
 	bool urgent = false;
+	std::vector<MapBlock *> map_blocks;
 
 	MeshUpdateResult() = default;
 };

src/network/clientpackethandler.cpp

@@ -322,10 +322,9 @@ void Client::handleCommand_BlockData(NetworkPacket* pkt)
 		/*
 			Create a new block
 		*/
-		block = new MapBlock(&m_env.getMap(), p, this);
+		block = sector->createBlankBlock(p.Y);
 		block->deSerialize(istr, m_server_ser_ver, false);
 		block->deSerializeNetworkSpecific(istr);
-		sector->insertBlock(block);
 	}
 
 	if (m_localdb) {