Daten über einen socket mit recv empfangen führt zu 100% cpu last

StefanKittel

Hallo,
ich habe eine Klasse mit der ich mittels IMAP Emails runterlade und speichere.
Visual Studio 2008, c++, non blocking sockets.

Das Problem ist, recv immer 100% cpu last erzeugt. Auch mit select und sleep wird das nicht besser. Während des Sleeps geht die Last runter, aber beim recv sofort wieder rauf.

Da IMAP Daten im Klartext sendet ist es recht aufwendig diese zu empfangen. Ich weiß halt nicht wie groß ein Datenpacket ist. Ich weiß nur somit die letzte Zeile beginnt und endet.

Hat da mal Jemand einen Tipp für mich?

Danke

Stefan

bool SocketReadable(SOCKET socket)
{
    FD_SET fdSet;
    TIMEVAL timeout;
    timeout.tv_sec = 2;
    timeout.tv_usec = 0;
    long status;

    FD_ZERO(&fdSet);
    FD_SET(socket,&fdSet);
    status = select(0,&fdSet,0,0,&timeout);
    if(status <= 0)
        {
                FD_ZERO(&fdSet);
        }
        if(!FD_ISSET(socket,&fdSet))
        {
                return false;
        }
    return true;
}

CString MailClasses_Socket::RecvIntoBuffer()
{
        int                     recvResult;
        char            buffer[100*1024];

        if (SocketReadable(m_Socket) == TRUE)
        {
                memset(buffer, 0x0, sizeof(buffer));
                recvResult = recv(m_Socket, buffer, sizeof(buffer) - 1, 0);

                //if we have an error return
                if (recvResult == SOCKET_ERROR)
                        if (WSAGetLastError() != WSAEWOULDBLOCK)
                                return GetLastErrorAsString(GetLastError());

                //if we received data add it to buffer
                if (recvResult > 0)
                {
                        //add to buffer
                        EnterCriticalSection(&m_CriticalSection_RecvBuffer);
                        m_RecvBuffer += buffer;
                        LeaveCriticalSection(&m_CriticalSection_RecvBuffer);
                }
        }

        return "";
}

CString MailClasses_Socket::ReadFromBuffer_String(CString &_Buffer, CString _StringToSearchFor)
{
        CString         resultString;
        int                     pos;

        while (TRUE)
        {
                EnterCriticalSection(&m_CriticalSection_RecvBuffer);
                pos = m_RecvBuffer.Find(_StringToSearchFor);
                if (pos >= 0)
                {
                        _Buffer = m_RecvBuffer.Left(pos + _StringToSearchFor.GetLength());
                        m_RecvBuffer = m_RecvBuffer.Mid(pos + _StringToSearchFor.GetLength());
                        LeaveCriticalSection(&m_CriticalSection_RecvBuffer);
                        return "";
                }
                LeaveCriticalSection(&m_CriticalSection_RecvBuffer);

                resultString = RecvIntoBuffer();
                if (resultString.IsEmpty() == FALSE)
                        return resultString;

//              Sleep(100);
        }

        return "";
}

Christoph

Die hohe CPU-Last kommt nicht vom recv(), die kommt von deinem while(TRUE). Warum benutzt du keine blocking sockets, wenn du sowieso in einer Schleife wartest, bis alles da ist?

Btw: Bei deinen critical sections gibt es eine race condition, denk ich. In Zeile 31 liest du vom Socket, ohne vorher die critical section zu holen.

hustbaer

Einen eigenen Empfangspuffer zu verwenden ist schonmal grundsätzlich keine schlechte Idee. Obwohl es zum Mails Abrufen vermutlich nicht nötig ist, und die Dinge etwas verkompliziert.

Ansonsten...
Dein Code hat Redundanzen, einiges mutet seltsam an, und bei einigem frage ich ob es denn nötig ist.

* Der memset vor dem recv ist NICHT nötig. Klar, du verlässt dich darauf dass der String Null-terminiert ist, aber da recv zurückmeldet wieviel empfangen wurde, kannst du genau das eine Byte nach dem empfangenen Block selbst Null setzen

* Du verlässt dich darauf, dass niemals Null-Bytes übertragen werden. Selbst wenn das Protokoll das vorschreibt würde ich das niemals tun.

* Du verwendest überall Critival-Sections. Wozu? Darf die Socket Klasse gleichzeitig aus mehreren Threads verwendet werden? Wenn ja: wozu?

* Wieso gibt RecvIntoBuffer() einen CString als Error-Code zurück. CString hat zwar soweit ich weiss eine Empty-String-Optimization, d.h. einen Leerstring zurückgeben (Kein Fehler -> Normalfall) kostet nicht viel. Ganz allgemein empfinde ich CString für diese Aufgabe aber als unpassend.

* Du durchsuchst m_RecvBuffer bei jedem empfangenen Block erneut ganz von Anfang an nach _StringToSearchFor. Wenn _StringToSearchFor, wie ich vermute, meist ein Zeilenumbruch ist ("\r\n"), geht das wesentlich effizienter.

* Das Zurechtschneiden von Strings mit .Left()/.Mid() ist relativ teuer - das geht viel billiger, wenn man z.B. Ringpuffer verwendet

All das erklärt für mich allerdings noch nicht, dass deine CPU Auslastung (für längere Zeit) auf 100% raufgeht. Keine Ahnunf was da so schief geht.

@Christoph:
Innerhalb des "while(true)" wird ja immer RecvIntoBuffer() aufgerufen. Und in
RecvIntoBuffer() wird wiederum SocketReadable() aufgerufen. Und SocketReadable() macht ein select() mit Timeout von 2 Sekunden. Und 2 Sekunden sind lange
Wenn ich nichts übersehen habe, sollte es also hier keine Möglichkeit geben, bei der die while Schleife sehr sehr "busy" wäre.
Ausser eben wenn der Server relativ schnell relativ viel Daten schickt. Und dann ist das Problem nicht die Schleife ansich, sondern die ineffiziente Verarbeitung der Daten.

StefanKittel

Christoph schrieb:

Die hohe CPU-Last kommt nicht vom recv(), die kommt von deinem while(TRUE). Warum benutzt du keine blocking sockets, wenn du sowieso in einer Schleife wartest, bis alles da ist?

Hallo Christoph,
Ich hatte vorher blocking sockets. Damit trat das Problem genau so auf. Auf anraten von anderer Seite habe ich das dann in non blocking sockets und select umgestellt. Kein Unterschied.
Das while(TRUE) wird ja vom select mit einem Timeout von 2 Sekunden unterbrochen.

Christoph schrieb:

Btw: Bei deinen critical sections gibt es eine race condition, denk ich. In Zeile 31 liest du vom Socket, ohne vorher die critical section zu holen.

Nein. Die critical section bezieht sich auf den Buffer und nicht den Socket.

Im meinem Fall rufe ich emails von einem lokalen IMAP Server ab. Es sind ca. 50 Mails mit jeweils 3MB größe. Also sollten Timeouts eigentlich keine Rolle spielen, da er die ganze Zeit empfängt.

StefanKittel

Schon mal vielen Dank Euch Beiden.
Ich habe schon an verschiedenen Stellen gepostet, aber noch keine sinnvollen Antworten erhalten.

Stefan

hustbaer schrieb:

Einen eigenen Empfangspuffer zu verwenden ist schonmal grundsätzlich keine schlechte Idee. Obwohl es zum Mails Abrufen vermutlich nicht nötig ist, und die Dinge etwas verkompliziert.

Leider doch. Ich weiß ja nicht wie lang die Mail ist. Und ich benötige alles was ich vor dem Ende-String empfangen habe und alles danach. Das können 2KB oder auch 20MB sein.

hustbaer schrieb:

Ansonsten...
Dein Code hat Redundanzen, einiges mutet seltsam an, und bei einigem frage ich ob es denn nötig ist.

Ja.

hustbaer schrieb:

* Der memset vor dem recv ist NICHT nötig. Klar, du verlässt dich darauf dass der String Null-terminiert ist, aber da recv zurückmeldet wieviel empfangen wurde, kannst du genau das eine Byte nach dem empfangenen Block selbst Null setzen

Ja. Ändere ich

hustbaer schrieb:

* Du verlässt dich darauf, dass niemals Null-Bytes übertragen werden. Selbst wenn das Protokoll das vorschreibt würde ich das niemals tun.

Ja. Ändere ich

hustbaer schrieb:

* Du verwendest überall Critival-Sections. Wozu? Darf die Socket Klasse gleichzeitig aus mehreren Threads verwendet werden? Wenn ja: wozu?

Ist ein Test-relikt wo es einen Empfangs-Thread und einen Warte-Thread gibt. Kommt weg

hustbaer schrieb:

* Wieso gibt RecvIntoBuffer() einen CString als Error-Code zurück. CString hat zwar soweit ich weiss eine Empty-String-Optimization, d.h. einen Leerstring zurückgeben (Kein Fehler -> Normalfall) kostet nicht viel. Ganz allgemein empfinde ich CString für diese Aufgabe aber als unpassend.

Ich verwende dies aus Gewohnheit. Bei neueren Klassen verwende ich Exceptions.

hustbaer schrieb:

* Du durchsuchst m_RecvBuffer bei jedem empfangenen Block erneut ganz von Anfang an nach _StringToSearchFor. Wenn _StringToSearchFor, wie ich vermute, meist ein Zeilenumbruch ist ("\r\n"), geht das wesentlich effizienter.

Leider ist der Suchstring eher "\r\nR: C1 " (siehe Unten). Und danach können auch noch bis zu 200 Bytes folgen. Ich kann also auch nicht einfach vom Ende her vergleichen und auch nicht nur den empfangen buffer.
Ich könnte natürlich ab postion = länge alter puffer - länge suchpuffer verwenden. Das probier ich mal aus.

hustbaer schrieb:

* Das Zurechtschneiden von Strings mit .Left()/.Mid() ist relativ teuer - das geht viel billiger, wenn man z.B. Ringpuffer verwendet

Leider weiß ich ja nicht wie groß die email wird. Es können halt auch 20MB oder mehr sein. Deshalb kam ich auf CString.

hustbaer schrieb:

All das erklärt für mich allerdings noch nicht, dass deine CPU Auslastung (für längere Zeit) auf 100% raufgeht. Keine Ahnunf was da so schief geht.

Mir leider auch nicht.

hustbaer schrieb:

Ausser eben wenn der Server relativ schnell relativ viel Daten schickt. Und dann ist das Problem nicht die Schleife ansich, sondern die ineffiziente Verarbeitung der Daten.

Das ändere ich mal.

Beispiel für Daten
Send:
IMAP_COMMAND_12 OK FETCH completed

Receive:
IMAP_COMMAND_ID_12 FETCH 1:55 (FLAGS UID INTERNALDATE RFC822.SIZE)
* 1 FETCH (UID 60175 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 2 FETCH (UID 60181 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 3 FETCH (UID 60182 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 4 FETCH (UID 60183 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 5 FETCH (UID 60184 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 6 FETCH (UID 60185 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 7 FETCH (UID 60186 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 8 FETCH (UID 60187 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
...
* 51 FETCH (UID 60264 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 52 FETCH (UID 60265 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 53 FETCH (UID 60266 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 54 FETCH (UID 60267 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
* 55 FETCH (UID 60268 RFC822.SIZE 2762123 FLAGS (\Seen) INTERNALDATE " 1-Jun-2009 13:23:42 +0100")
IMAP_COMMAND_ID_12 OK FETCH completed
* Statistics

StefanKittel

Neue Infos:

Das abschalten des AV-Programmes (Email-Scan) hat schon viel gebracht
Das zusammen mit den Optimierungen führt dazu, dass die CPU Last über 30% nicht hinauskommt.

Dafür ist die Übertragungsrate nun bei nur 1.2MB/Sek.
Wenn ich 8 Threads verwende komme ich auf 4-6MB dafür ist die CPU Last dann wieder bei 100% (30*80=100)?

Nachtrag: Das Problem wird durch

m_RecvBuffer += buffer;

verursacht. Ich dachte nicht, dass dies so viel ausmachen würde. Naja.

Wie umgehe ich das am schlauesten? Kann man speicherbereiche erweitern? Quasi immer die 10KB hinten dranhängen?

Stefan

hustbaer

ich gehe mal davon aus dass m_RecvBuffer auch ein CString ist. und CString ist für "stückerlweise immer länger machen" nicht gerade optimal geeignet.

als "quick fix" kannst du probieren den CString gleich von anfang an gross genug zu machen, z.B. mit m_RecvBuffer.Preallocate(1024 * 1024).

oder exponentiell wachsen lassen:

if (m_RecvBuffer.GetAllocLength() < (m_RecvBuffer.GetLength() + sizeof(buffer) + 1))
    m_RecvBuffer.Preallocate(m_RecvBuffer.GetAllocLength() * 2);
m_RecvBuffer += ...;

ich würde das ganze aber etwas anders angehen - wozu du allerdings mehr umschreiben müsstest.
ich würde vielleicht erstmal alles in zeilen zerteilen. in der art:

class foo
{
public:
    foo() :
        m_carriage_return_seen(false)
    {
    }

    void push(char const* begin, char const* end)
    {
        while (begin != end)
        {
            push(*begin);
            ++begin;
        }
    }

    void push(char ch)
    {
        m_buffer.push_back(ch);

        if (m_carriage_return_seen && ch == '\n')
        {
            emit_line();
            m_buffer.clear();
        }

        m_carriage_return_seen = (ch == '\r');
    }

    void emit_line()
    {
        // m_buffer enthält jetzt genau eine zeile text
        // hier sollte diese zeile jetzt ans nächste glied in der kette weitergereicht werden
    }

private:
    std::vector<char> m_buffer;
    bool m_carriage_return_seen;
}

dadurch musst du nicht jedes mal alle bereits empfangenen bytes erneut nach einem zeilenumbruch durchsuchen, was einiges an zeit spart.

in der emit_line() funktion leitest du das dann z.b. weiter an eine andere klasse, die sich ums IMAP protokoll ansich kümmert, und die z.b. auf eine zeile in einem bestimmten format wartet.
dazu musst du auch kein "find" verwenden - die meisten protokolle sind so aufgebaut, dass man die "haben fertig" zeilen sehr einfach erkennen kann. zumindest die die ich kenne.

beispiel:

void bar::push_line(char const* begin, char const* end)
{
    m_reply_lines.push_back(std::string(begin, end));

    size_t const line_size = end - begin;

    // erster check, einfach, für schnelle "rejection" der meisten unpassenden zeilen
    if (line_size >= 5
        && begin[0] == 'I'    // nur ein beispiel, ich hab keine ahnung vom IMAP protokoll
        && begin[1] == 'M'
        && begin[2] == 'A'
        && begin[3] == 'P'
        && begin[4] == '_')
    {
        // genauer check ob es die gesuchte zeile ist
        if (...)
        {
            process_command_reply();
            m_reply_lines.clear();
        }
    }
}

ich will damit nur grob ne richtung andeuten, wie man halbwegs schnell und halbwegs sauber (und IMO mit vertretbarem aufwand) zeilenbasierte protokolle parsen kann.

ich hoffe du kannst damit was anfangen.

StefanKittel

Hallo,

ich habes gefunden. Es ist ein CString::Trim("\r\n ").
Wenn diesen Befehl auf einen 23MB String losläßt dauert das 15Sekunden.
QuadCore 3GHz, 8GB RAM, 64Bit OS. Ramdurchsatz ca. 32GByte/Sekunde

Ich wußte ja das CString nicht so schnell ist, aber das ist doch lächerlich.

Ich lese nun direkt in den Buffer des CString mittels GetBufferSetLength und ReleaseBuffer. Wenn der Buffer zu klein wird kopiere ich ihn in einen doppelt so großen. Das optimiere ich dann später.

Bei RecvIntoBuffer lese ich nun solange Daten kommen mit einen Select Timeout von 250ms.

while (SocketReadable(m_Socket) == TRUE)
{
}

Vielen Dank für Eure Hilfe!
Das hätte ich so wohl nicht gefunden!

Stefan

PS: ich mag keine Klartextprotkolle!

hustbaer

Klartextprotokolle sind das Beste wo gibt

hustbaer

Ramdurchsatz ca. 32GByte/Sekunde

Gerücht!

StefanKittel

hustbaer schrieb:

Klartextprotokolle sind das Beste wo gibt

Sagt Jemand der eine Taschenlampe für ein 500 Euro Smartphone programmiert hat :xmas2:

hustbaer

StefanKittel schrieb:

hustbaer schrieb:

Klartextprotokolle sind das Beste wo gibt

Sagt Jemand der eine Taschenlampe für ein 500 Euro Smartphone programmiert hat :xmas2:

Von mir ist nur der Windows Port

BTW: doch kein Gerücht. Hab mich grad schlau gelesen. Wahnsinn. Will auch einen i7

drakon

hustbaer schrieb:

Klartextprotokolle sind das Beste wo gibt

Du bist altmodisch.
Das heisst: "Klartextprotokoll sind s'bescht, wos je häts gits." (ca. 3:50)

@hustbaer
Ich finde es wahnsinnig, wie Konsistent sich dein Programm auf den verschiedenen System verhält. Wie kriegst du das nur hin, dass gleich aussieht? :p

volkard

drakon schrieb:

@hustbaer
Ich finde es wahnsinnig, wie Konsistent sich dein Programm auf den verschiedenen System verhält. Wie kriegst du das nur hin, dass gleich aussieht? :p

In der Tat. Pixelgenau gleich.

D:\Downloads>fc screen-java-windows.png screen-java-linux.png
Vergleichen der Dateien screen-java-windows.png und SCREEN-JAVA-LINUX.PNG
FC: Keine Unterschiede gefunden

D:\Downloads>fc screen-java-windows.png screen-java-macosx.png
Vergleichen der Dateien screen-java-windows.png und SCREEN-JAVA-MACOSX.PNG
FC: Keine Unterschiede gefunden

Will auch einen i7.

drakon

volkard schrieb:

drakon schrieb:

@hustbaer
Ich finde es wahnsinnig, wie Konsistent sich dein Programm auf den verschiedenen System verhält. Wie kriegst du das nur hin, dass gleich aussieht? :p

In der Tat. Pixelgenau gleich.

Phu.. Ich habe es vermutet, aber das es wirklich so gleich ist, hätte ich nie erwartet..