Value copy / allocation optimizations mostly in server, SAO and seria…

…lize code
minetest · May 27, 2020 · 471e567 · 471e567
1 parent 2fd5f38
commit 471e567
Show file tree

Hide file tree

Showing 16 changed files with 52 additions and 64 deletions.
diff --git a/src/client/game.cpp b/src/client/game.cpp
@@ -2806,7 +2806,7 @@ void Game::handleClientEvent_SetSky(ClientEvent *event, CameraOrientation *cam)
 		// Shows the mesh skybox
 		sky->setVisible(true);
 		// Update mesh based skybox colours if applicable.
-		sky->setSkyColors(*event->set_sky);
+		sky->setSkyColors(event->set_sky->sky_color);
 		sky->setHorizonTint(
 			event->set_sky->fog_sun_tint,
 			event->set_sky->fog_moon_tint,

diff --git a/src/client/sky.cpp b/src/client/sky.cpp
@@ -907,9 +907,9 @@ void Sky::setStarCount(u16 star_count, bool force_update)
 	}
 }
 
-void Sky::setSkyColors(const SkyboxParams sky)
+void Sky::setSkyColors(const SkyColor &sky_color)
 {
-	m_sky_params.sky_color = sky.sky_color;
+	m_sky_params.sky_color = sky_color;
 }
 
 void Sky::setHorizonTint(video::SColor sun_tint, video::SColor moon_tint,

diff --git a/src/client/sky.h b/src/client/sky.h
@@ -94,7 +94,7 @@ class Sky : public scene::ISceneNode
 		m_bgcolor = bgcolor;
 		m_skycolor = skycolor;
 	}
-	void setSkyColors(const SkyboxParams sky);
+	void setSkyColors(const SkyColor &sky_color);
 	void setHorizonTint(video::SColor sun_tint, video::SColor moon_tint,
 		std::string use_sun_tint);
 	void setInClouds(bool clouds) { m_in_clouds = clouds; }

diff --git a/src/content/mods.cpp b/src/content/mods.cpp
@@ -167,7 +167,7 @@ std::map<std::string, ModSpec> getModsInPath(
 	return result;
 }
 
-std::vector<ModSpec> flattenMods(std::map<std::string, ModSpec> mods)
+std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods)
 {
 	std::vector<ModSpec> result;
 	for (const auto &it : mods) {

diff --git a/src/content/mods.h b/src/content/mods.h
@@ -68,7 +68,7 @@ std::map<std::string, ModSpec> getModsInPath(
 		const std::string &path, bool part_of_modpack = false);
 
 // replaces modpack Modspecs with their content
-std::vector<ModSpec> flattenMods(std::map<std::string, ModSpec> mods);
+std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods);
 
 // a ModConfiguration is a subset of installed mods, expected to have
 // all dependencies fullfilled, so it can be used as a list of mods to

diff --git a/src/script/cpp_api/s_node.cpp b/src/script/cpp_api/s_node.cpp
@@ -94,7 +94,7 @@ struct EnumString ScriptApiNode::es_NodeBoxType[] =
 	};
 
 bool ScriptApiNode::node_on_punch(v3s16 p, MapNode node,
-		ServerActiveObject *puncher, PointedThing pointed)
+		ServerActiveObject *puncher, const PointedThing &pointed)
 {
 	SCRIPTAPI_PRECHECKHEADER
 

diff --git a/src/script/cpp_api/s_node.h b/src/script/cpp_api/s_node.h
@@ -36,7 +36,7 @@ class ScriptApiNode
 	virtual ~ScriptApiNode() = default;
 
 	bool node_on_punch(v3s16 p, MapNode node,
-			ServerActiveObject *puncher, PointedThing pointed);
+			ServerActiveObject *puncher, const PointedThing &pointed);
 	bool node_on_dig(v3s16 p, MapNode node,
 			ServerActiveObject *digger);
 	void node_on_construct(v3s16 p, MapNode node);

diff --git a/src/server.cpp b/src/server.cpp
@@ -718,34 +718,35 @@ void Server::AsyncRunStep(bool initial_step)
 		std::unordered_map<u16, std::vector<ActiveObjectMessage>*> buffered_messages;
 
 		// Get active object messages from environment
+		ActiveObjectMessage aom(0);
+		u32 aom_count = 0;
 		for(;;) {
-			ActiveObjectMessage aom = m_env->getActiveObjectMessage();
-			if (aom.id == 0)
+			if (!m_env->getActiveObjectMessage(&aom))
 				break;
 
 			std::vector<ActiveObjectMessage>* message_list = nullptr;
-			std::unordered_map<u16, std::vector<ActiveObjectMessage>* >::iterator n;
-			n = buffered_messages.find(aom.id);
+			auto n = buffered_messages.find(aom.id);
 			if (n == buffered_messages.end()) {
 				message_list = new std::vector<ActiveObjectMessage>;
 				buffered_messages[aom.id] = message_list;
-			}
-			else {
+			} else {
 				message_list = n->second;
 			}
-			message_list->push_back(aom);
+			message_list->push_back(std::move(aom));
+			aom_count++;
 		}
 
-		m_aom_buffer_counter->increment(buffered_messages.size());
+		m_aom_buffer_counter->increment(aom_count);
 
 		m_clients.lock();
 		const RemoteClientMap &clients = m_clients.getClientList();
 		// Route data to every client
+		std::string reliable_data, unreliable_data;
 		for (const auto &client_it : clients) {
+			reliable_data.clear();
+			unreliable_data.clear();
 			RemoteClient *client = client_it.second;
 			PlayerSAO *player = getPlayerSAO(client->peer_id);
-			std::string reliable_data;
-			std::string unreliable_data;
 			// Go through all objects in message buffer
 			for (const auto &buffered_message : buffered_messages) {
 				// If object does not exist or is not known by client, skip it
@@ -770,19 +771,15 @@ void Server::AsyncRunStep(bool initial_step)
 								client->m_known_objects.end())
 							continue;
 					}
-					// Compose the full new data with header
-					std::string new_data;
-					// Add object id
-					char buf[2];
-					writeU16((u8*)&buf[0], aom.id);
-					new_data.append(buf, 2);
-					// Add data
-					new_data += serializeString(aom.datastring);
-					// Add data to buffer
-					if (aom.reliable)
-						reliable_data += new_data;
-					else
-						unreliable_data += new_data;
+
+					// Add full new data to appropriate buffer
+					std::string &buffer = aom.reliable ? reliable_data : unreliable_data;
+					char idbuf[2];
+					writeU16((u8*) idbuf, aom.id);
+					// u16 id
+					// std::string data
+					buffer.append(idbuf, sizeof(idbuf));
+					buffer.append(serializeString(aom.datastring));
 				}
 			}
 			/*

diff --git a/src/server/luaentity_sao.cpp b/src/server/luaentity_sao.cpp
@@ -119,8 +119,7 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 		m_properties_sent = true;
 		std::string str = getPropertyPacket();
 		// create message and add to list
-		ActiveObjectMessage aom(getId(), true, str);
-		m_messages_out.push(aom);
+		m_messages_out.emplace(getId(), true, str);
 	}
 
 	// If attached, check that our parent is still there. If it isn't, detach.
@@ -228,16 +227,14 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 		m_animation_sent = true;
 		std::string str = generateUpdateAnimationCommand();
 		// create message and add to list
-		ActiveObjectMessage aom(getId(), true, str);
-		m_messages_out.push(aom);
+		m_messages_out.emplace(getId(), true, str);
 	}
 
 	if (!m_animation_speed_sent) {
 		m_animation_speed_sent = true;
 		std::string str = generateUpdateAnimationSpeedCommand();
 		// create message and add to list
-		ActiveObjectMessage aom(getId(), true, str);
-		m_messages_out.push(aom);
+		m_messages_out.emplace(getId(), true, str);
 	}
 
 	if (!m_bone_position_sent) {
@@ -247,17 +244,15 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 			std::string str = generateUpdateBonePositionCommand((*ii).first,
 					(*ii).second.X, (*ii).second.Y);
 			// create message and add to list
-			ActiveObjectMessage aom(getId(), true, str);
-			m_messages_out.push(aom);
+			m_messages_out.emplace(getId(), true, str);
 		}
 	}
 
 	if (!m_attachment_sent) {
 		m_attachment_sent = true;
 		std::string str = generateUpdateAttachmentCommand();
 		// create message and add to list
-		ActiveObjectMessage aom(getId(), true, str);
-		m_messages_out.push(aom);
+		m_messages_out.emplace(getId(), true, str);
 	}
 }
 

diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp
@@ -223,8 +223,7 @@ void PlayerSAO::step(float dtime, bool send_recommended)
 		m_properties_sent = true;
 		std::string str = getPropertyPacket();
 		// create message and add to list
-		ActiveObjectMessage aom(getId(), true, str);
-		m_messages_out.push(aom);
+		m_messages_out.emplace(getId(), true, str);
 		m_env->getScriptIface()->player_event(this, "properties_changed");
 	}
 
@@ -324,10 +323,8 @@ void PlayerSAO::step(float dtime, bool send_recommended)
 
 	if (!m_attachment_sent) {
 		m_attachment_sent = true;
-		std::string str = generateUpdateAttachmentCommand();
 		// create message and add to list
-		ActiveObjectMessage aom(getId(), true, str);
-		m_messages_out.push(aom);
+		m_messages_out.emplace(getId(), true, generateUpdateAttachmentCommand());
 	}
 }
 

diff --git a/src/server/serveractiveobject.cpp b/src/server/serveractiveobject.cpp
@@ -75,7 +75,7 @@ std::string ServerActiveObject::generateUpdateNametagAttributesCommand(const vid
 void ServerActiveObject::dumpAOMessagesToQueue(std::queue<ActiveObjectMessage> &queue)
 {
 	while (!m_messages_out.empty()) {
-		queue.push(m_messages_out.front());
+		queue.push(std::move(m_messages_out.front()));
 		m_messages_out.pop();
 	}
 }
diff --git a/src/server/serverinventorymgr.h b/src/server/serverinventorymgr.h
@@ -57,4 +57,4 @@ class ServerInventoryManager : public InventoryManager
 	ServerEnvironment *m_env = nullptr;
 
 	std::unordered_map<std::string, DetachedInventory> m_detached_inventories;
-};
+};
diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp
@@ -1603,14 +1603,14 @@ void ServerEnvironment::setStaticForActiveObjectsInBlock(
 	}
 }
 
-ActiveObjectMessage ServerEnvironment::getActiveObjectMessage()
+bool ServerEnvironment::getActiveObjectMessage(ActiveObjectMessage *dest)
 {
 	if(m_active_object_messages.empty())
-		return ActiveObjectMessage(0);
+		return false;
 
-	ActiveObjectMessage message = m_active_object_messages.front();
+	*dest = std::move(m_active_object_messages.front());
 	m_active_object_messages.pop();
-	return message;
+	return true;
 }
 
 void ServerEnvironment::getSelectedActiveObjects(

diff --git a/src/serverenvironment.h b/src/serverenvironment.h
@@ -289,9 +289,9 @@ class ServerEnvironment : public Environment
 
 	/*
 		Get the next message emitted by some active object.
-		Returns a message with id=0 if no messages are available.
+		Returns false if no messages are available, true otherwise.
 	*/
-	ActiveObjectMessage getActiveObjectMessage();
+	bool getActiveObjectMessage(ActiveObjectMessage *dest);
 
 	virtual void getSelectedActiveObjects(
 		const core::line3d<f32> &shootline_on_map,

diff --git a/src/tool.cpp b/src/tool.cpp
@@ -130,7 +130,7 @@ void ToolCapabilities::serializeJson(std::ostream &os) const
 	root["punch_attack_uses"] = punch_attack_uses;
 
 	Json::Value groupcaps_object;
-	for (auto groupcap : groupcaps) {
+	for (const auto &groupcap : groupcaps) {
 		groupcap.second.toJson(groupcaps_object[groupcap.first]);
 	}
 	root["groupcaps"] = groupcaps_object;

diff --git a/src/util/serialize.cpp b/src/util/serialize.cpp
@@ -110,6 +110,7 @@ std::string serializeString(const std::string &plain)
 
 	if (plain.size() > STRING_MAX_LEN)
 		throw SerializationError("String too long for serializeString");
+	s.reserve(2 + plain.size());
 
 	writeU16((u8 *)&buf[0], plain.size());
 	s.append(buf, 2);
@@ -131,13 +132,11 @@ std::string deSerializeString(std::istream &is)
 	if (s_size == 0)
 		return s;
 
-	Buffer<char> buf2(s_size);
-	is.read(&buf2[0], s_size);
+	s.resize(s_size);
+	is.read(&s[0], s_size);
 	if (is.gcount() != s_size)
 		throw SerializationError("deSerializeString: couldn't read all chars");
 
-	s.reserve(s_size);
-	s.append(&buf2[0], s_size);
 	return s;
 }
 
@@ -152,6 +151,7 @@ std::string serializeWideString(const std::wstring &plain)
 
 	if (plain.size() > WIDE_STRING_MAX_LEN)
 		throw SerializationError("String too long for serializeWideString");
+	s.reserve(2 + 2 * plain.size());
 
 	writeU16((u8 *)buf, plain.size());
 	s.append(buf, 2);
@@ -196,13 +196,14 @@ std::wstring deSerializeWideString(std::istream &is)
 
 std::string serializeLongString(const std::string &plain)
 {
+	std::string s;
 	char buf[4];
 
 	if (plain.size() > LONG_STRING_MAX_LEN)
 		throw SerializationError("String too long for serializeLongString");
+	s.reserve(4 + plain.size());
 
 	writeU32((u8*)&buf[0], plain.size());
-	std::string s;
 	s.append(buf, 4);
 	s.append(plain);
 	return s;
@@ -227,13 +228,11 @@ std::string deSerializeLongString(std::istream &is)
 			"string too long: " + itos(s_size) + " bytes");
 	}
 
-	Buffer<char> buf2(s_size);
-	is.read(&buf2[0], s_size);
+	s.resize(s_size);
+	is.read(&s[0], s_size);
 	if ((u32)is.gcount() != s_size)
 		throw SerializationError("deSerializeLongString: couldn't read all chars");
 
-	s.reserve(s_size);
-	s.append(&buf2[0], s_size);
 	return s;
 }