TCP server handle Clients, std::map or array?

[opengl]

struct CLIENT_DATA
{
int m_nOnlineTime;
int m_nCredits;
};

for a tcp server which would handle at most 10240 clients.
I want to manage all the clients.there are two solutions
1: use a std:map, and a mutex over all map operation
2: use a C array. and assign an ID(0-10239) for each client if connected,If disconnected,the Id would be recycled.
if I use an array,mutex would only be necessary over the ID operations.such as assign an Id to a client when connected,recycle an Id when a client is disconnected.

in fact,they are not different if I only have to handle connect or disconnect.
but if I want to change the CLIENT_DATA for each client.
for the first solution:I have to lock the whole map;
for the second, I could maintain a mutex in each CLIENT_DATA.


my question is, solution 1 is much more easy to code.solution 2 seems much more efficient.
which one do you prefer?
Thank you.

[coder777]

If you use an c array: How do you find a free Id?

A problem could be that a client may create invalid requests. Therefore a used id should be recycled as late as possible.

My suggestion would be: Use a [64bit] id which will be increased after each connection.

If you want to make sure that you are able to detect invalid request you need to keep the id and thus a map would be the better solution.

[opengl]

Ok,let me explain it in details

1 I have to maintain a collection for all the clients data,such as

struct CLIENT_DATA
{
std::string m_strIp;
int m_nOnlineTime;
int m_nCredits;
};

2 the client data update(by a timer for example) and the socket are in different thread

so If I use a map

bool Connect(std::string strIp)
{
   boost::lock_guard<boost::mutex> lock(m_lock);

  CLIENT_DATA* pData = new CLIENT_DATA;
  pData->m_nOnlineTime = 0;
  pData->m_strIp = strIp;
  m_mapClients.push_back(strIp,pData);
  
  return true;
}

void Disconnect(std::string strIp);
{
 	boost::lock_guard<boost::mutex> lock(m_lock);

        std::map<std::string,CLIENT_DATA*>::iterator iter = m_mapClients.find(strIp);
	if(iter != m_mapClients.end())
		return;

	CLIENT_DATA* pClientData = m_mapClients[strIp];
	if(pClientData != NULL)
        {
           delete pClientData;
           pClientData = NULL;
        }
        m_mapClients.erase(iter);
}

bool UpdateOnlineTime(std::string strIp)
{
 	boost::lock_guard<boost::mutex> lock(m_lock);
	
        std::map<std::string,CLIENT_DATA*>::iterator iter = m_mapClients.find(strIp);
  	if(iter != m_mapClients.end())
		return false;

	CLIENT_DATA* pClientData = m_mapClients[strIp];
	if(pClientData == NULL)
               return false;
	pClientData->m_nOnlineTime += 10;
	
	return true;
}

I have to lock the map use the same mutex in connect,disconnect,updateonlinetime

if I use a array

void PrepareIds()
{
	for(int i=0;i<10240;i++)
	   m_vecAvailableIds.push_back(i);
	
}

int FetchId()
{
	boost::lock_guard<boost::mutex> lock(m_lock);
	if(m_vecAvailableIds.size()<=0)
            return -1;

	int nId =  m_vecAvailableIds.back();
	m_vecAvailableIds.pop_front();
        return nId;
}

int ReturnId(int nId)
{
      boost::lock_guard<boost::mutex> lock(m_lock);
      m_vecAvailableIds.push_back(nId);
}

bool Connected(std::string strIp)
{
	int nNewId = FetchId();
	if(nNewId == -1)
             return false;

	CLIENT_DATA* pData = m_arrClients[nNewId];
        boost::lock_guard<boost::mutex> lock(pData->m_mutex_lock);
        pData->m_bInUse = true;
	pData->m_nOnlineTime = 0;	
	pData->m_strIP = strIp;
}

bool Disconnected(int nClientId)
{
	Assert(nClientId>=0 && nClientId<10240);
	
        CLIENT_DATA* pData = m_arrClients[nClientId];
        boost::lock_guard<boost::mutex> lock(pData->m_mutex_lock);
        pData->m_bInUse = false;

        ReturnId(nClientId);

	return true;
}

Now,the UpdateOnlineTime function and the CLIENT_DATA structure are changed

struct CLIENT_DATA
{
std::string m_strIp;
int m_nOnlineTime;
int m_nCredits;
bool m_bInUse;
boost::mutex m_mutex_data;
};

bool UpdateOnlineTime(int nClientId)
{
	Assert(nClientId>=0 && nClientId<10240);
	
	CLIENT_DATA* pData = m_arrClients[nClientId];
	
 	boost::lock_guard<boost::mutex> lock(pData->m_mutex_lock);
	if(! pClientData->m_bIsInUse)
             return false;
	
	pClientData->m_nOnlineTime += 10;
	
	return true;
}

There is only need to lock one CLIENT_DATA in case we may update the m_nCredits in some other place.
Now there are two kinds of mutex,one for Ids,one for each CLIENT_DATA

the most important point is when I iterate all the client datas
if I use a map:

UpdateClients()
{
    boost::lock_guard<boost::mutex> lock(m_lock); //lock the whole map
   for(int i=0;i<m_mapClients.size();i++)
   {

      Update();
    }
}

if I use an array

UpdateClients()
{
    //there is no need to lock
    for(int i=0;i<m_arrClients.size();i++)
   {

    Update();
    }
}

if the map size if very large,such as 10240;
lock the whole map would block the connect or disconnect operation.

[coder777]

Well, this Update(); requires to lock the client data, doesn't it?

If so you have a permanent lock/unlock for each client which most likely means that the loop consumes more time.

Such an expensive loop should be avoide anyway.

Why do you want to update the clients? Do you want an automatic disconnect when the online time is expired? This can be done with a queue that does not require to update all the clients. In that case m_nOnlineTime should be a real time stamp (for instance using the time() function if the resolution seconds is good enough).

[opengl]

Thank you
The m_nOnlineTime member is only a demostration.

let me explain:

I need a collection to maintain all the Client_DATA

There are two threads;
Thread A would get some CLIENT_DATA pointer to do something frequently,but would not change the COLLECTION
Thread B is the Update thread ,include handling connect and disconnect,the COLLECTION would only be changed here.

So If I use a std::map, in Thread B, the map should be locked while Update()
it would block the Thread A at somewhere like GetCLientData(int nClientId);

Solution 2 is designed to avoid locking the whole CLIENT_DATE collection.

If the COLLECTION is not very large,use a std::map is fine.
If not, I doubt that

[coder777]

So if you insist that you need an array you can have that array.

How about combining both techniques:

class Server
{
  vector<Client> clients;
  mutex ConnMutex;
  map<long int, int> ConnectedSlot;
  vector<long int, int> FreeSlot;

  void Connect()
  {
    lock ConnMutex;
    static long int id = 0;
    int free_slot;
    if(m_FreeSlot.empty())
    {
      free_slot = clients.size();
      clients.push_back(...);
    }
    else
    {
      free_slot = m_FreeSlot.back();
      m_FreeSlot.pop_back();
    }
    ConnectedSlot[id] = free_slot;
    ++id;
  }
  Disconnect(long int id)
  {
    lock ConnMutex;
    m_FreeSlot.push_back(ConnectedSlot[id]);
    ConnectedSlot.erease(id);
  } 
};

This is more or less pseudocode so that you get the idea

[opengl]

thank you
I don't insist to use a array.

A COLLECTION contains all CLIENT_DATA

ThreadA: get some CLIENT_DATA pointer frequently
THreadB: change the COLLECTION, eg: add,remove,update

if I use a std::map, A map-wide lock should be used in Get,Add,Remove.Update interface.
If I use a array, I can maintain a lock for each CLIENT_DATA.

So while I iterate the whole COLLECTION:
If I use a std::map,the whole map should be locked;
If I use a array,only the current CLIENT_DATA should be locked.

my question is , as my need (10240 clients at most), If I use a std::map,would it be fine?

the Update in ThreadB would iterate the Whole map and lock it.
But the Get mathod would be called a lot in threadA,which also need the lock.

[coder777]

my question is , as my need (10240 clients at most), If I use a std::map,would it be fine?

If you ask this as a black and white question: no.

How do you find the free slot for a new client within the plain array?

[opengl]

I can use another vector to maintain the available position in a C array.
check the code above