PDA

Просмотр полной версии : Overhead в базовой функции SendPacket()


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

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

--- 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);



--- 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(sendPacketC ount)/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
26.03.2011, 09:00
Не путайте capacity() и size(). Первая возвращает число выделенных блоков под элементы вектора, вторая - число элементов.
Следовательно ваш патч ничего не исправляет.

xex
26.03.2011, 09:07
Не путайте capacity() и size(). Первая возвращает число выделенных блоков под элементы вектора, вторая - число элементов.
Следовательно ваш патч ничего не исправляет.
Зерг, при всём к тебе уважении (заслуженном), но ты не прав.
Посмотри внимательно код ByteBuffer.h, там переопределено.

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

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

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

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

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

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

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

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

Ambal
26.03.2011, 10:21
Если использовать ее для определения размера пакета можно напороться на неприятные баги.

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

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

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

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

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

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

zergtmn
26.03.2011, 11:01
xex, изучайте std::vector...

xex
26.03.2011, 11:53
Давайте вы не будете зря сотрясать воздух, а поставите ассерты и посмотрите, возможна ли в принципе та ситуация, о которой вы говорите. А она может быть возможна только в случае если неправильно используются функции size_t wpos(size_t wpos_) и void resize(size_t newsize)
Да мне-то зачем? Выявил проблему, а вы посмотреть детально и разобраться не можете.
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

Dvlpr
26.03.2011, 11:54
эм, кагбэ 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
26.03.2011, 11:54
Ещё добавлю для ядерщиков:

int WorldSocket::SendPacket (const WorldPacket& pct)

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

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

Dvlpr
26.03.2011, 12:04
if (pct.size() != pct.wpos()) throw "FAIL!";

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

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

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

TOM_RUS
26.03.2011, 12:37
pct.size() - размер выделенной памяти.


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

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