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

[xex]

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

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

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) 


[zergtmn]

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

[xex]

Сообщение от zergtmn

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

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

[zhenya]

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

[Ambal]

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

[Ambal]

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

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

[xex]

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

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

[Ambal]

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

[zergtmn]

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

[Ambal]

Сообщение от zergtmn

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

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

[xex]

Сообщение от zergtmn

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

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

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

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

[Ambal]

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

[zergtmn]

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

[xex]

Сообщение от Ambal

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

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

Сообщение от zergtmn

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

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

У меня всё работает больше недели. трафик снизился на серверах...
Не хотите в ядро внедрить, ну ладно
Я например и такие вещи использую в буффере, перенёс с 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

[Dvlpr]

эм, кагбэ 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(), но этот факт ничего не меняет)

[xex]

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

int WorldSocket::SendPacket (const WorldPacket& pct)

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

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

[Dvlpr]

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

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

[Ambal]

Сообщение от xex

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

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

[TOM_RUS]

Сообщение от xex

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

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

[Ambal]

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