perf(gamemessage): Replace GameMessageArgument linked lists with vectors (TheSuperHackers#2700)

Author: RikuAnt

Core/GameEngine/Include/GameNetwork/NetCommandMsg.h

@@ -33,6 +33,7 @@
 #include "GameNetwork/NetPacketStructs.h"
 #include "Common/UnicodeString.h"
 
+class GameMessageArgument;
 class NetCommandRef;
 
 //-----------------------------------------------------------------------------
@@ -179,10 +180,8 @@ class NetGameCommandMsg : public NetCommandMsgT<NetPacketGameCommand, SmallNetPa
 	virtual Select getSmallNetPacketSelect() const override;
 
 protected:
-	Int m_numArgs;
-	Int m_argSize;
 	GameMessage::Type m_type;
-	GameMessageArgument *m_argList, *m_argTail;
+	std::vector<GameMessageArgument*> m_argList;
 };
 
 //-----------------------------------------------------------------------------

Core/GameEngine/Source/GameNetwork/NetCommandMsg.cpp

@@ -88,12 +88,8 @@ Int NetCommandMsg::getSortNumber() const {
  * Constructor with no argument, sets everything to default values.
  */
 NetGameCommandMsg::NetGameCommandMsg() {
-	m_argSize = 0;
-	m_numArgs = 0;
 	m_type = (GameMessage::Type)0;
 	m_commandType = NETCOMMANDTYPE_GAMECOMMAND;
-	m_argList = nullptr;
-	m_argTail = nullptr;
 }
 
 /**
@@ -102,23 +98,24 @@ NetGameCommandMsg::NetGameCommandMsg() {
  */
 NetGameCommandMsg::NetGameCommandMsg(GameMessage *msg) {
 	m_commandType = NETCOMMANDTYPE_GAMECOMMAND;
-
 	m_type = msg->getType();
-	Int count = msg->getArgumentCount();
-	for (Int i = 0; i < count; ++i) {
-		addArgument(msg->getArgumentDataType(i), *(msg->getArgument(i)));
+
+	const size_t argsCount = msg->getArgumentCount();
+	m_argList.reserve(argsCount);
+
+	for (size_t i = 0; i < argsCount; ++i) {
+		GameMessageArgumentDataType argType = msg->getArgumentDataType(i);
+		const GameMessageArgumentType* arg = msg->getArgument(i);
+		addArgument(argType, *arg);
 	}
 }
 
 /**
  * Destructor
  */
 NetGameCommandMsg::~NetGameCommandMsg() {
-	GameMessageArgument *arg = m_argList;
-	while (arg != nullptr) {
-		m_argList = m_argList->m_next;
-		deleteInstance(arg);
-		arg = m_argList;
+	for (size_t i = 0; i < m_argList.size(); ++i) {
+		deleteInstance(m_argList[i]);
 	}
 }
 
@@ -127,21 +124,10 @@ NetGameCommandMsg::~NetGameCommandMsg() {
  */
 void NetGameCommandMsg::addArgument(const GameMessageArgumentDataType type, GameMessageArgumentType arg)
 {
-	if (m_argTail == nullptr) {
-		m_argList = newInstance(GameMessageArgument);
-		m_argTail = m_argList;
-		m_argList->m_data = arg;
-		m_argList->m_type = type;
-		m_argList->m_next = nullptr;
-		return;
-	}
-
 	GameMessageArgument *newArg = newInstance(GameMessageArgument);
 	newArg->m_data = arg;
 	newArg->m_type = type;
-	newArg->m_next = nullptr;
-	m_argTail->m_next = newArg;
-	m_argTail = newArg;
+	m_argList.push_back(newArg);
 }
 
 // here's where we figure out which slot corresponds to which player
@@ -171,9 +157,8 @@ GameMessage *NetGameCommandMsg::constructGameMessage() const
 	name.format("player%d", getPlayerID());
 	retval->friend_setPlayerIndex( ThePlayerList->findPlayerWithNameKey(TheNameKeyGenerator->nameToKey(name))->getPlayerIndex());
 
-	GameMessageArgument *arg = m_argList;
-	while (arg != nullptr) {
-
+	for (size_t i = 0; i < m_argList.size(); ++i) {
+		const GameMessageArgument* arg = m_argList[i];
 		switch (arg->m_type) {
 
 		case ARGUMENTDATATYPE_INTEGER:
@@ -211,8 +196,6 @@ GameMessage *NetGameCommandMsg::constructGameMessage() const
 			break;
 
 		}
-
-		arg = arg->m_next;
 	}
 	return retval;
 }

Generals/Code/GameEngine/Include/Common/MessageStream.h

@@ -82,7 +82,6 @@ class GameMessageArgument : public MemoryPoolObject
 {
 	MEMORY_POOL_GLUE_WITH_USERLOOKUP_CREATE(GameMessageArgument, "GameMessageArgument")
 public:
-	GameMessageArgument*				m_next;									///< The next argument
 	GameMessageArgumentType			m_data;									///< The data storage of an argument
 	GameMessageArgumentDataType	m_type;									///< The type of the argument.
 };
@@ -637,7 +636,6 @@ class GameMessage : public MemoryPoolObject
 	GameMessage *prev() { return m_prev; }		///< Return prev message in the stream
 
 	Type getType() const { return m_type; }					///< Return the message type
-	UnsignedByte getArgumentCount() const { return m_argCount; }	///< Return the number of arguments for this msg
 
 	const char *getCommandAsString() const; ///< returns a string representation of the command type.
 	static const char *getCommandTypeAsString(GameMessage::Type t);
@@ -660,8 +658,8 @@ class GameMessage : public MemoryPoolObject
 
 	/**
 	 * Return the given argument union.
-	 * @todo This should be a more list-like interface.  Very inefficient.
 	 */
+	UnsignedByte getArgumentCount() const { return static_cast<UnsignedByte>(m_argList.size()); }
 	const GameMessageArgumentType *getArgument( Int argIndex ) const;
 	GameMessageArgumentDataType getArgumentDataType( Int argIndex ) const;
 
@@ -681,10 +679,7 @@ class GameMessage : public MemoryPoolObject
 
 	Int m_playerIndex;													///< The Player who issued the command
 
-	/// @todo If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's.
-	UnsignedByte m_argCount;										///< The number of arguments of this message
-
-	GameMessageArgument *m_argList, *m_argTail;						///< This message's arguments
+	std::vector<GameMessageArgument*> m_argList;						///< This message's arguments
 
 	/// allocate a new argument, add it to list, return pointer to its data
 	GameMessageArgument *allocArg();

Generals/Code/GameEngine/Source/Common/MessageStream.cpp

@@ -56,9 +56,6 @@ GameMessage::GameMessage( GameMessage::Type type )
 {
 	m_playerIndex = ThePlayerList->getLocalPlayer()->getPlayerIndex();
 	m_type = type;
-	m_argList = nullptr;
-	m_argTail = nullptr;
-	m_argCount = 0;
 	m_list = nullptr;
 }
 
@@ -69,12 +66,8 @@ GameMessage::GameMessage( GameMessage::Type type )
 GameMessage::~GameMessage()
 {
 	// free all arguments
-	GameMessageArgument *arg, *nextArg;
-
-	for( arg = m_argList; arg; arg=nextArg )
-	{
-		nextArg = arg->m_next;
-		deleteInstance(arg);
+	for( size_t i = 0; i < m_argList.size(); ++i ) {
+		deleteInstance(m_argList[i]);
 	}
 
 	// detach message from list
@@ -84,14 +77,11 @@ GameMessage::~GameMessage()
 
 /**
  * Return the given argument union.
- * @todo This should be a more list-like interface.  Very inefficient.
  */
 const GameMessageArgumentType *GameMessage::getArgument( Int argIndex ) const
 {
-	int i=0;
-	for( GameMessageArgument *a = m_argList; a; a=a->m_next, i++ )
-		if (i == argIndex)
-			return &a->m_data;
+	if (static_cast<size_t>(argIndex) < m_argList.size())
+		return &m_argList[argIndex]->m_data;
 
 	DEBUG_CRASH(("argument not found"));
 	static const GameMessageArgumentType zero = { 0 };
@@ -103,17 +93,9 @@ const GameMessageArgumentType *GameMessage::getArgument( Int argIndex ) const
  */
 GameMessageArgumentDataType GameMessage::getArgumentDataType( Int argIndex ) const
 {
-	if (argIndex >= m_argCount) {
-		return ARGUMENTDATATYPE_UNKNOWN;
-	}
-	int i=0;
-	GameMessageArgument *a = m_argList;
-	for (; a && (i < argIndex); a=a->m_next, ++i );
+	if (static_cast<size_t>(argIndex) < m_argList.size())
+		return m_argList[argIndex]->m_type;
 
-	if (a != nullptr)
-	{
-		return a->m_type;
-	}
 	return ARGUMENTDATATYPE_UNKNOWN;
 }
 
@@ -124,21 +106,12 @@ GameMessageArgument *GameMessage::allocArg()
 {
 	// allocate a new argument
 	GameMessageArgument *arg = newInstance(GameMessageArgument);
+	m_argList.push_back(arg);
 
-	// add to end of argument list
-	if (m_argTail)
-		m_argTail->m_next = arg;
-	else
-	{
-		m_argList = arg;
-		m_argTail = arg;
-	}
-
-	arg->m_next = nullptr;
-	m_argTail = arg;
-
-	m_argCount++;
-
+	DEBUG_ASSERTCRASH(
+		m_argList.size() <= 255,
+		("If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's.")
+	);
 	return arg;
 }
 

Generals/Code/GameEngine/Source/Common/Recorder.cpp

@@ -785,15 +785,12 @@ void RecorderClass::writeToFile(GameMessage * msg) {
 		argType = argType->getNext();
 	}
 
-//	UnsignedByte lasttype = (UnsignedByte)ARGUMENTDATATYPE_UNKNOWN;
-	Int numArgs = msg->getArgumentCount();
-	for (Int i = 0; i < numArgs; ++i) {
-//		UnsignedByte type = (UnsignedByte)(msg->getArgumentDataType(i));
-//		if (lasttype != type) {
-//			fwrite(&type, sizeof(type), 1, m_file);
-//			lasttype = type;
-//		}
-		writeArgument(msg->getArgumentDataType(i), *(msg->getArgument(i)));
+	const size_t argsCount = msg->getArgumentCount();
+
+	for (size_t i = 0; i < argsCount; ++i) {
+		GameMessageArgumentDataType argType = msg->getArgumentDataType(i);
+		const GameMessageArgumentType* arg = msg->getArgument(i);
+		writeArgument(argType, *arg);
 	}
 
 	deleteInstance(parser);

GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h

@@ -82,7 +82,6 @@ class GameMessageArgument : public MemoryPoolObject
 {
 	MEMORY_POOL_GLUE_WITH_USERLOOKUP_CREATE(GameMessageArgument, "GameMessageArgument")
 public:
-	GameMessageArgument*				m_next;									///< The next argument
 	GameMessageArgumentType			m_data;									///< The data storage of an argument
 	GameMessageArgumentDataType	m_type;									///< The type of the argument.
 };
@@ -637,7 +636,6 @@ class GameMessage : public MemoryPoolObject
 	GameMessage *prev() { return m_prev; }		///< Return prev message in the stream
 
 	Type getType() const { return m_type; }					///< Return the message type
-	UnsignedByte getArgumentCount() const { return m_argCount; }	///< Return the number of arguments for this msg
 
 	const char *getCommandAsString() const; ///< returns a string representation of the command type.
 	static const char *getCommandTypeAsString(GameMessage::Type t);
@@ -660,8 +658,8 @@ class GameMessage : public MemoryPoolObject
 
 	/**
 	 * Return the given argument union.
-	 * @todo This should be a more list-like interface.  Very inefficient.
 	 */
+	UnsignedByte getArgumentCount() const { return static_cast<UnsignedByte>(m_argList.size()); }
 	const GameMessageArgumentType *getArgument( Int argIndex ) const;
 	GameMessageArgumentDataType getArgumentDataType( Int argIndex ) const;
 
@@ -681,10 +679,7 @@ class GameMessage : public MemoryPoolObject
 
 	Int m_playerIndex;													///< The Player who issued the command
 
-	/// @todo If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's.
-	UnsignedByte m_argCount;										///< The number of arguments of this message
-
-	GameMessageArgument *m_argList, *m_argTail;						///< This message's arguments
+	std::vector<GameMessageArgument*> m_argList;						///< This message's arguments
 
 	/// allocate a new argument, add it to list, return pointer to its data
 	GameMessageArgument *allocArg();

GeneralsMD/Code/GameEngine/Source/Common/MessageStream.cpp

@@ -56,9 +56,6 @@ GameMessage::GameMessage( GameMessage::Type type )
 {
 	m_playerIndex = ThePlayerList->getLocalPlayer()->getPlayerIndex();
 	m_type = type;
-	m_argList = nullptr;
-	m_argTail = nullptr;
-	m_argCount = 0;
 	m_list = nullptr;
 }
 
@@ -69,12 +66,8 @@ GameMessage::GameMessage( GameMessage::Type type )
 GameMessage::~GameMessage()
 {
 	// free all arguments
-	GameMessageArgument *arg, *nextArg;
-
-	for( arg = m_argList; arg; arg=nextArg )
-	{
-		nextArg = arg->m_next;
-		deleteInstance(arg);
+	for( size_t i = 0; i < m_argList.size(); ++i ) {
+		deleteInstance(m_argList[i]);
 	}
 
 	// detach message from list
@@ -84,14 +77,11 @@ GameMessage::~GameMessage()
 
 /**
  * Return the given argument union.
- * @todo This should be a more list-like interface.  Very inefficient.
  */
 const GameMessageArgumentType *GameMessage::getArgument( Int argIndex ) const
 {
-	int i=0;
-	for( GameMessageArgument *a = m_argList; a; a=a->m_next, i++ )
-		if (i == argIndex)
-			return &a->m_data;
+	if (static_cast<size_t>(argIndex) < m_argList.size())
+		return &m_argList[argIndex]->m_data;
 
 	DEBUG_CRASH(("argument not found"));
 	static const GameMessageArgumentType zero = { 0 };
@@ -103,17 +93,9 @@ const GameMessageArgumentType *GameMessage::getArgument( Int argIndex ) const
  */
 GameMessageArgumentDataType GameMessage::getArgumentDataType( Int argIndex ) const
 {
-	if (argIndex >= m_argCount) {
-		return ARGUMENTDATATYPE_UNKNOWN;
-	}
-	int i=0;
-	GameMessageArgument *a = m_argList;
-	for (; a && (i < argIndex); a=a->m_next, ++i );
+	if (static_cast<size_t>(argIndex) < m_argList.size())
+		return m_argList[argIndex]->m_type;
 
-	if (a != nullptr)
-	{
-		return a->m_type;
-	}
 	return ARGUMENTDATATYPE_UNKNOWN;
 }
 
@@ -124,21 +106,12 @@ GameMessageArgument *GameMessage::allocArg()
 {
 	// allocate a new argument
 	GameMessageArgument *arg = newInstance(GameMessageArgument);
+	m_argList.push_back(arg);
 
-	// add to end of argument list
-	if (m_argTail)
-		m_argTail->m_next = arg;
-	else
-	{
-		m_argList = arg;
-		m_argTail = arg;
-	}
-
-	arg->m_next = nullptr;
-	m_argTail = arg;
-
-	m_argCount++;
-
+	DEBUG_ASSERTCRASH(
+		m_argList.size() <= 255,
+		("If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's.")
+	); 
 	return arg;
 }