From 471e567657dfd75a994a1b54d7a23cf4541a6bed Mon Sep 17 00:00:00 2001 From: sfan5 Date: Tue, 26 May 2020 17:38:31 +0200 Subject: [PATCH] Value copy / allocation optimizations mostly in server, SAO and serialize code --- src/client/game.cpp | 2 +- src/client/sky.cpp | 4 +-- src/client/sky.h | 2 +- src/content/mods.cpp | 2 +- src/content/mods.h | 2 +- src/script/cpp_api/s_node.cpp | 2 +- src/script/cpp_api/s_node.h | 2 +- src/server.cpp | 43 ++++++++++++++----------------- src/server/luaentity_sao.cpp | 15 ++++------- src/server/player_sao.cpp | 7 ++--- src/server/serveractiveobject.cpp | 2 +- src/server/serverinventorymgr.h | 2 +- src/serverenvironment.cpp | 8 +++--- src/serverenvironment.h | 4 +-- src/tool.cpp | 2 +- src/util/serialize.cpp | 17 ++++++------ 16 files changed, 52 insertions(+), 64 deletions(-) diff --git a/src/client/game.cpp b/src/client/game.cpp index e7663a113..cdf4da21e 100644 --- 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 index d21b56fcc..2e0cbca86 100644 --- 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 index 8637f96d4..3227e8f59 100644 --- a/src/client/sky.h +++ b/src/client/sky.h @@ -94,7 +94,7 @@ public: 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 index 676666f78..95ab0290a 100644 --- a/src/content/mods.cpp +++ b/src/content/mods.cpp @@ -167,7 +167,7 @@ std::map getModsInPath( return result; } -std::vector flattenMods(std::map mods) +std::vector flattenMods(const std::map &mods) { std::vector result; for (const auto &it : mods) { diff --git a/src/content/mods.h b/src/content/mods.h index 6e2506dbf..b3500fbc8 100644 --- a/src/content/mods.h +++ b/src/content/mods.h @@ -68,7 +68,7 @@ std::map getModsInPath( const std::string &path, bool part_of_modpack = false); // replaces modpack Modspecs with their content -std::vector flattenMods(std::map mods); +std::vector flattenMods(const std::map &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 index d93a4c3ad..e0f9bcd78 100644 --- 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 index e7c0c01d1..81b44f0f0 100644 --- a/src/script/cpp_api/s_node.h +++ b/src/script/cpp_api/s_node.h @@ -36,7 +36,7 @@ public: 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 index 8c62584c8..6ecbd7097 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -718,34 +718,35 @@ void Server::AsyncRunStep(bool initial_step) std::unordered_map*> 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* message_list = nullptr; - std::unordered_map* >::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; 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 index 51e1ca90e..8174da265 100644 --- 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,8 +244,7 @@ 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); } } @@ -256,8 +252,7 @@ void LuaEntitySAO::step(float dtime, bool send_recommended) 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 index a4d0f4ce7..3ea3536e2 100644 --- 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 index 8345ebd47..fdcb13bd8 100644 --- 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 &queue) { while (!m_messages_out.empty()) { - queue.push(m_messages_out.front()); + queue.push(std::move(m_messages_out.front())); m_messages_out.pop(); } } \ No newline at end of file diff --git a/src/server/serverinventorymgr.h b/src/server/serverinventorymgr.h index d0aac4dae..ccf6d3b2e 100644 --- a/src/server/serverinventorymgr.h +++ b/src/server/serverinventorymgr.h @@ -57,4 +57,4 @@ private: ServerEnvironment *m_env = nullptr; std::unordered_map m_detached_inventories; -}; \ No newline at end of file +}; diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp index d485c32e8..222b4d203 100644 --- 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 index e2f1a3784..4b453d39a 100644 --- a/src/serverenvironment.h +++ b/src/serverenvironment.h @@ -289,9 +289,9 @@ public: /* 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 &shootline_on_map, diff --git a/src/tool.cpp b/src/tool.cpp index d911c518f..22e41d28e 100644 --- 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 index f0e177d57..5b276668d 100644 --- 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 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 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; }