Ru-MaNGOS - Overhead в базовой функции SendPacket()

Ru-MaNGOS (http://mangos.ytdb.ru/index.php)

- Отвергнутые патчи (http://mangos.ytdb.ru/forumdisplay.php?f=50)

- - Overhead в базовой функции SendPacket() (http://mangos.ytdb.ru/showthread.php?t=3989)

Overhead в базовой функции SendPacket()

Отправка клиенту больше данных чем нужно.
Отправляем - сколько выделили памяти для буфера, а не сколько туда положили. Баг, как вижу старый, на трине присутствует тоже :yes3: ...
У меня трафик снизился после исправления.

Собственно код для чистого ядра:

PHP код:


		
			
--- WorldSocket.cpp
+++ WorldSocket.cpp
@@ -164,36 +164,36 @@
     if (closing_)
         return -1;

     // Dump outgoing packet.
     sLog.outWorldPacketDump(uint32(get_handle()), pct.GetOpcode(), LookupOpcodeName(pct.GetOpcode()), &pct, false);

-    ServerPktHeader header(pct.size()+2, pct.GetOpcode());
+    ServerPktHeader header(pct.wpos()+ 2, pct.GetOpcode());
     m_Crypt.EncryptSend ((uint8*)header.header, header.getHeaderLength());

-    if (m_OutBuffer->space () >= pct.size () + header.getHeaderLength() && msg_queue()->is_empty())
+    if (m_OutBuffer->space() >= pct.wpos()+ header.getHeaderLength() && msg_queue()->is_empty())
     {
         // Put the packet on the buffer.
-        if (m_OutBuffer->copy ((char*) header.header, header.getHeaderLength()) == -1)
+        if (m_OutBuffer->copy((char*)header.header, header.getHeaderLength()) == -1)
             MANGOS_ASSERT (false);

-        if (!pct.empty ())
-            if (m_OutBuffer->copy ((char*) pct.contents (), pct.size ()) == -1)
+        if (pct.wpos() > 0)
+            if (m_OutBuffer->copy((char*)pct.contents(), pct.wpos()) == -1)
-                MANGOS_ASSERT (false);
+                MANGOS_ASSERT(false);
     }
     else
     {
         // Enqueue the packet.
         ACE_Message_Block* mb;

-        ACE_NEW_RETURN(mb, ACE_Message_Block(pct.size () + header.getHeaderLength()), -1);
+        ACE_NEW_RETURN(mb, ACE_Message_Block(pct.wpos()+ header.getHeaderLength()), -1);

         mb->copy((char*) header.header, header.getHeaderLength());

-        if (!pct.empty ())
-            mb->copy((const char*)pct.contents(), pct.size ());
+        if (pct.wpos() > 0)
+            mb->copy((const char*)pct.contents(), pct.wpos());

         if(msg_queue()->enqueue_tail(mb,(ACE_Time_Value*)&ACE_Time_Value::zero) == -1)
         {
             sLog.outError("WorldSocket::SendPacket enqueue_tail");
             mb->release();
             return -1;
@@ -774,13 +774,13 @@
     {
         packet.Initialize (SMSG_AUTH_RESPONSE, 1);
         packet << uint8 (AUTH_VERSION_MISMATCH);

         SendPacket (packet);

-        sLog.outError ("WorldSocket::HandleAuthSession: Sent Auth Response (version mismatch).");
+        sLog.outError("WorldSocket::HandleAuthSession: Sent Auth Response (version mismatch). Client build: %u", ClientBuild);
         return -1;
     }

     // Get the account information from the realmd database
     std::string safe_account = account; // Duplicate, else will screw the SHA hash verification below
     LoginDatabase.escape_string (safe_account);

PHP код:


		
			
--- WorldSession.cpp
+++ WorldSession.cpp
@@ -35,12 +35,15 @@
 #include "BattleGroundMgr.h"
 #include "MapManager.h"
 #include "SocialMgr.h"
 #include "Auth/AuthCrypt.h"
 #include "Auth/HMACSHA1.h"
 #include "zlib/zlib.h"

 // select opcodes appropriate for processing in Map::Update context for current session state
 static bool MapSessionFilterHelper(WorldSession* session, OpcodeHandler const& opHandle)
 {
     // we do not process thread-unsafe packets
     if (opHandle.packetProcessing == PROCESS_THREADUNSAFE)
@@ -147,16 +150,16 @@

     time_t cur_time = time(NULL);

     if((cur_time - lastTime) < 60)
     {
         sendPacketCount+=1;
-        sendPacketBytes+=packet->size();
+        sendPacketBytes += packet->wpos();

         sendLastPacketCount+=1;
-        sendLastPacketBytes+=packet->size();
+        sendLastPacketBytes += packet->wpos();
     }
     else
     {
         uint64 minTime = uint64(cur_time - lastTime);
         uint64 fullTime = uint64(lastTime - firstTime);
         DETAIL_LOG("Send all time packets count: " UI64FMTD " bytes: " UI64FMTD " avr.count/sec: %f avr.bytes/sec: %f time: %u",sendPacketCount,sendPacketBytes,float(sendPacketCount)/fullTime,float(sendPacketBytes)/fullTime,uint32(fullTime));
@@ -179,22 +182,22 @@
     _recvQueue.add(new_packet);
 }

 /// Logging helper for unexpected opcodes
 void WorldSession::LogUnexpectedOpcode(WorldPacket* packet, const char *reason)
 {
-    sLog.outError( "SESSION: received unexpected opcode %s (0x%.4X) %s",
+    sLog.outDebug("SESSION: received unexpected opcode %s (0x%.4X) %s",
         LookupOpcodeName(packet->GetOpcode()),
         packet->GetOpcode(),
         reason);
 }

 /// Logging helper for unexpected opcodes
 void WorldSession::LogUnprocessedTail(WorldPacket *packet)
 {
-    sLog.outError( "SESSION: opcode %s (0x%.4X) have unprocessed tail data (read stop at " SIZEFMTD " from " SIZEFMTD ")",
+    sLog.outDebug("SESSION: opcode %s (0x%.4X) have unprocessed tail data (read stop at "SIZEFMTD" from "SIZEFMTD")",
         LookupOpcodeName(packet->GetOpcode()),
         packet->GetOpcode(),
         packet->rpos(),packet->wpos());
 }

 /// Update the WorldSession (triggered by World update)

Не путайте capacity() и size(). Первая возвращает число выделенных блоков под элементы вектора, вторая - число элементов.
Следовательно ваш патч ничего не исправляет.

Цитата:

Сообщение от zergtmn (Сообщение 20333)

Зерг, при всём к тебе уважении (заслуженном), но ты не прав.
Посмотри внимательно код ByteBuffer.h, там переопределено.

size_t size() const { return _storage.size(); }
и там же
const static size_t DEFAULT_SIZE = 0x1000;
_storage.reserve(DEFAULT_SIZE);
Если не указан размер..

Я, если честно, не вижу в каких случаях мы можем так жестоко прокалываться... У нас _storage.size() обязан быть равен _wpos. Вы пробовали ставить ассерты дабы проверить ваши догадки?

zhenya, _storage.reserve() != _storage.resize(). первое изменяет значение, возвращаемое функцией capacity(), а второе еще и size().

P.S. xex, поставьте такой ассерт MANGOS_ASSERT(pct.size() == pct.wpos()); в функции WorldSocket::SendPacket() и отпишитесь, если он сработает. Надо быть уверенными в том, что трабла действительно имеет место т.к. по коду на первый взгляд проблем не видно

Да вы чего? внимательно в код посмотрите-то.
size_t size() const { return _storage.size(); }
размер ХРАНИЛИЩА (сколько мозгов выделили)
size_t wpos() const { return _wpos; }
размер сколько данных влили.

Вообще неправильно класс в этом отношении написан. Ядерщики - исправляйте =)

Не морочьте голову, поставьте ассерт как я сказал в предыдущем посте и если он сработает, отпишитесь каким образом у вас это вышло: номер пакета, настройки компрессии трафика и т.д.

wpos() - позиция записи в буфер, ее можно передвинуть чтобы перезаписать какие-то данные. Если использовать ее для определения размера пакета можно напороться на неприятные баги.
_storage.size() - возвращает число байт, которые положили в буффер. Т.е. то что нужно, бага нет.

Цитата:

Сообщение от zergtmn (Сообщение 20348)

Если использовать ее для определения размера пакета можно напороться на неприятные баги.

Вот именно эту ситуевину мы сейчас и пытаемся отловить: кто может делать wpos() != size() перед отправкой пакета.

Цитата:

Сообщение от zergtmn (Сообщение 20348)

Да, но когда пакет готов и передаётся в функцию передачи данных, size сейчас указывает на РЕАЛЬНЫЙ размер что выделили, а не положили туда.

WorldPacket data(опкод, 1киллобайт).
data << гайд;
data << uint8(0);

Киллобайт и передаётся, хотя там например гайд и 0ль - итого максимум 9 байт, ну плюс опкод 2 байта =).
Сам класс немного криво сделан...

Давайте вы не будете зря сотрясать воздух, а поставите ассерты и посмотрите, возможна ли в принципе та ситуация, о которой вы говорите. А она может быть возможна только в случае если неправильно используются функции size_t wpos(size_t wpos_) и void resize(size_t newsize)

xex, изучайте std::vector...

Цитата:

Сообщение от Ambal (Сообщение 20351)

Да мне-то зачем? Выявил проблему, а вы посмотреть детально и разобраться не можете.

Цитата:

Сообщение от zergtmn (Сообщение 20353)

xex, изучайте std::vector...

Это сюда никак...

У меня всё работает больше недели. трафик снизился на серверах...
Не хотите в ядро внедрить, ну ладно :resent:
Я например и такие вещи использую в буффере, перенёс с Delphi:

Код:

// boxa

inline uint32 AlignMemW(uint32 size)

{

    if (size & 1)

        return (size | 1) + 1;

    else

        return size;

}



inline uint32 AlignMemD(uint32 size)

{

    if (size & 3)

        return (size | 3) + 1;

    else

        return size;

}



inline uint32 AlignMemQ(uint32 size)

{

    if (size & 7)

        return (size | 7) + 1;

    else

        return size;

}

// boxa

эм, кагбэ http://www.devx.com/tips/Tip/5328

Особенно замечаем " In addition, reserve() does not change the size of a vector; it only changes the vector's capacity."

ВЫВОД: size() вернет столько, сколько туда записали... (либо ресайзнули вручную через resize(), но этот факт ничего не меняет)

Ещё добавлю для ядерщиков:

int WorldSocket::SendPacket (const WorldPacket& pct)

пакет готов для отправки клиенту? - да. мы здесь меняем данные? - нет.

pct.size() - размер выделенной памяти.
pct.wpos() - сколько записали И НУЖНО ПЕРЕДАТЬ.

if (pct.size() != pct.wpos()) throw "FAIL!";

может на плюсах понятнее.

Цитата:

Сообщение от xex (Сообщение 20357)

Да мне-то зачем? Выявил проблему, а вы посмотреть детально и разобраться не можете.

Юмор как раз заключается в том, что за час игры с этими ассертами у меня сервер так и не упал. Посему исходя из вашего последнего комментария смею предположить что TROLL DETECTED.

Цитата:

Сообщение от xex (Сообщение 20362)

pct.size() - размер выделенной памяти.

Неверно. Тема ниочем...

Закрою я эту тему от греха подальше. Пусть наш Delphi-программист почитает про разницу между зарезервированной и использованной памятью в STL-контейнерах. А также поймет, что существование багов надо доказывать, а не тыкать пальцем в небо и кричать "там где-то есть бага, я знаю!".