Remade who list processing #18636
Remade who list processing #18636
Conversation
…r entries are now copied every 5 seconds to dedicated storage (avoids usage of hashmapholder mutex)
| _whoListStorage.clear(); | ||
| _whoListStorage.reserve(sWorld->GetPlayerCount()+1); | ||
|
|
||
| boost::shared_lock<boost::shared_mutex> lock(*HashMapHolder<Player>::GetLock()); |
There was a problem hiding this comment.
I've been thinking a bit about this, is this lock even needed for player storage? entries are only added/removed from the main thread (and you are looping this in main thread as well)
There was a problem hiding this comment.
True, it's unneeded because this is done in main loop and even if other thread could run parallel to this it should not modify the player storage (except for reading)
|
|
||
| std::string aname; | ||
| if (AreaTableEntry const* areaEntry = sAreaTableStore.LookupEntry(itr->second->GetZoneId())) | ||
| aname = areaEntry->area_name[sWorld->GetDefaultDbcLocale()]; |
There was a problem hiding this comment.
you must not store the area name, this will break lookups in other locales
There was a problem hiding this comment.
Is caching names for all locales ok?
There was a problem hiding this comment.
Do you really need to cache the name at all? Just cache the zone/area ID.
Getting the name is already just 2 array lookups which are constant and lockless. (not even a hashmap lookup). Caching probably gives little to no benefit. Or then I just dont see something here.
I guess the real speedup/benefit in this change was from trying to eliminate Player needed in the who list call, but area name is not something that needs player at all, is it?
| uint32 GetZoneId() const { return _zoneid; } | ||
| uint8 GetGender() const { return _gender; } | ||
| bool IsVisible() const { return _visible; } | ||
| std::wstring const& Getwpname() const { return _wpname; } |
| if (AreaTableEntry const* areaEntry = sAreaTableStore.LookupEntry(itr->second->GetZoneId())) | ||
| aname = areaEntry->area_name[sWorld->GetDefaultDbcLocale()]; | ||
|
|
||
| _whoListStorage.push_back( WhoListPlayerInfo(itr->second->GetGUID().GetCounter(), itr->second->GetTeam(), itr->second->GetSession()->GetSecurity(), itr->second->getLevel(), |
There was a problem hiding this comment.
and since we are actually using c++11 features you could just .emplace_back(arg list) it
|
Whats up with the lowguid love? Always prefer full guid unless you are saving it to database (also a bit wishful thinking: if cross-realm lfg/bg ever happens then you need full guid as it will contain realm identifier part) |
|
A habit of mine |
| { | ||
| Player* target = itr->second; | ||
| WhoListPlayerInfo target = *itr; |
ghost
added
the
* Remade who list processing, requests are now processed in maps. Player entries are now copied every 5 seconds to dedicated storage (avoids usage of hashmapholder mutex) (cherrypicked from 88f7469)
Target branch(es): 3.3.5
Issues addressed: References #14274
Tests performed: Compiles and basic ingame tests
Requests are now processed in maps. Player entries are now copied every 5 seconds to dedicated storage (avoids usage of hashmapholder mutex)
Improvements: Move processing to separate thread