Оптимизация многопоточного сервера

[merke]

Всем привет!
В общем накидал многопоточный сервер на Qt, реализовал всё без использования QThread.
В потоках я не очень, поэтому хочу посоветоваться, какие функции раскидать по потокам. Очень нужна Ваша помощь.

ВОт привожу весь код сервера:

MainWindow.h

Код:

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QtGui/QMainWindow>
#include <QTcpServer>
#include <QTcpSocket>
#include <QBuffer>
#include <QMessageBox>

namespace Ui
{
    class MainWindowClass;
}

class MainWindow : public QMainWindow
{
    Q_OBJECT
protected:
        void sendToClients(const QByteArray& line);
public:
    MainWindow(QWidget *parent = 0);
    ~MainWindow();

private:
    Ui::MainWindowClass *ui;
    QTcpServer *server;
    QList<QTcpSocket*> connections;
    QHash<QTcpSocket*, QBuffer*> buffers;
    QTcpSocket *serverSocket;
    QTcpServer *tcpServer;
    QStringList tmpClientAddressPort;
    QMap<QString, quint16> adr;
private slots:
    void on_pushButton_3_clicked();
    void slotStartClicked();
    void slotStopClicked();
    void slotNewClient();
    void slotClientDisconnected();
    void slotSocketRead();
};

#endif // MAINWINDOW_H 

MainWindow.cpp

Код:

#include "mainwindow.h"
#include "ui_mainwindow.h"
#include <QBuffer>

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent), ui(new Ui::MainWindowClass)
{
    ui->setupUi(this);
    tcpServer = new QTcpServer(this);
    serverSocket = new QTcpSocket(this);
    connect(ui->pushButton, SIGNAL(clicked()), this, SLOT(slotStartClicked()));
    connect(ui->pushButton_2, SIGNAL(clicked()), this, SLOT(slotStopClicked()));
    connect(tcpServer, SIGNAL(newConnection()), this, SLOT(slotNewClient()));
    QRegExp regExp("[1-9][0-9]{0,4}");
    ui->lineEdit->setValidator(new QRegExpValidator(regExp, this));
    if (!tcpServer->listen(QHostAddress::Any, 1234)) {
        QMessageBox::critical(this, tr("Server"), tr("Unable to start the server: %1.").arg(tcpServer->errorString()));
        return;
    }
}

MainWindow::~MainWindow()
{
    delete ui;
}

void MainWindow::sendToClients(const QByteArray& line)
{
        foreach (QTcpSocket* connection, connections)
        {
                connection->write(line);
        }
}

void MainWindow::slotStartClicked()
{
        ui->label->setText(tr("Server starting"));
        if (ui->lineEdit->text() == "")
        {
                QMessageBox::critical(this, tr("Server"), tr("Please enter a valid port number"));
                return;
        }
        if (ui->lineEdit->text().toInt() > 65535)
        {
                ui->lineEdit->setText("65535");
        }

    if (!tcpServer->listen(QHostAddress::Any, ui->lineEdit->text().toInt())) {
        QMessageBox::critical(this, tr("Server"), tr("Unable to start the server: %1.").arg(tcpServer->errorString()));
        return;
    }
        ui->pushButton->setDisabled(true);
        ui->pushButton_2->setDisabled(false);
        ui->lineEdit->setDisabled(true);
        ui->label->setText(tr("Server started at port %1").arg(tcpServer->serverPort()));
}

void MainWindow::slotStopClicked()
{
        ui->pushButton->setEnabled(true);
        ui->pushButton_2->setEnabled(false);
        ui->lineEdit->setDisabled(false);

        if (tcpServer->isListening())
        {
                tcpServer->close();
        }

        if (!connections.isEmpty())
        {
                foreach (QTcpSocket* connection, connections)
                {
                                connection->close();
                }
                connections.clear();
        }

        if (serverSocket->isValid())
        {
                serverSocket->close();
        }
        ui->label->setText(tr("Server stopped"));
}

void MainWindow::slotNewClient()
{
        if (!connections.isEmpty())
        {
                QByteArray msg = "New Client connected";
                foreach (QTcpSocket* connection, connections)
                {
                                connection->write(msg);
                }
        }
        serverSocket = tcpServer->nextPendingConnection();
        QString str_buf;
        //tmpClientAddressPort << tr("%1").arg(serverSocket->peerAddress().toString()) << tr("%1").arg(serverSocket->peerPort());
        tmpClientAddressPort << tr("%1").arg(serverSocket->peerPort());
        str_buf = tr("%1").arg(serverSocket->peerPort());
        ui->label->setNum(serverSocket->socketDescriptor());
        ui->listWidget->addItem(ui->label->text());
        connections.append(serverSocket);

        QBuffer* buffer = new QBuffer(this);
        ui->label->setText(serverSocket->peerAddress().toString());
        buffer->open(QIODevice::ReadWrite);
        buffers.insert(serverSocket, buffer); 
        connect(serverSocket, SIGNAL(disconnected()), this, SLOT(slotClientDisconnected()));
        connect(serverSocket, SIGNAL(readyRead()), this, SLOT(slotSocketRead()));

}

void MainWindow::slotClientDisconnected()
{
        ui->label->setText("Client disconnected");
        QTcpSocket* socket = static_cast<QTcpSocket*>(sender());
        QBuffer* buffer = buffers.take(socket);
        buffer->close();
        buffer->deleteLater();
        connections.removeAll(socket);
        socket->deleteLater();

        QByteArray msg = "Client disconnected";
        foreach (QTcpSocket* connection, connections)
        {
                connection->write(msg);
        }
}

void MainWindow::slotSocketRead()
{
 QTcpSocket* socket = static_cast<QTcpSocket*>(sender());
 QBuffer* buffer = buffers.value(socket);
 ui->label->setText("readyread");
 qint64 bytes = buffer->write(socket->readAll());
 buffer->seek(buffer->pos() - bytes);
 QByteArray line = buffer->readAll();
 ui->textEdit->append(line);
 sendToClients(line);
}

void MainWindow::on_pushButton_3_clicked()
{
    QByteArray ba3;
    ba3.append(ui->lineEdit_2->text());
    foreach (QTcpSocket* connection, connections)
    {
        if (connection->socketDescriptor() == ui->listWidget->item(ui->listWidget->currentRow())->text().toInt())
            connection->write(ba3);
    }
}

[SABROG]

В общем накидал многопоточный сервер на Qt

Асинхронный - да. Многопоточный - нет. Главный недостаток такой реализации в том, что любой метод, который выполняется дольше всех - блокирует основной цикл событий и как следствие все клиенты вынуждены ждать ответа от сервера до тех пор пока управление не вернется в этот цикл событий. Пример: сервер пытается передать файл клиенту. В этот момент срабатывает Касперский и просит пользователя принять решение, что делать с этим файлом. Если пользователя в этот момент нет за компьютером, то диалог Касперского будет висеть вечно, а операционная система не возвратит управление с ошибкой в приложение, которое пытается открыть файл до того момента пока антивирус этого не разрешит. В случае с многопоточной системой несколько циклов событий в каждом потоке могли бы запускать таймер по истечении которого грохать нерадивый поток, застравший на чтении файла, а клиентов просить переподключитсья, чтобы поймать их в новом потоке. Файл можно пометить как "плохой" и не предпринимать попытки чтения до того как придет хозяин и не разберется с этим.

Эти инклуды замедляют процесс компиляции:

Код

C++ (Qt)
#include <QTcpServer>
#include <QTcpSocket>
#include <QBuffer>
#include <QMessageBox>
 

На эти классы QTcpServer, QTcpSocket, QBuffer в заголовках используются только указатели, на которые известны размеры компилятору. Поэтому достаточно Forward Declaration:

Код

C++ (Qt)
class QTcpServer;
class QTcpSocket;
class QBuffer;
 

Но в .cpp файле нужно будет сделать включение их инклудов, а инклуд QMessageBox перенести вообще в .cpp файл. Часто получается так, что используется один заголовок на несколько .cpp файлов, причем некоторым из них не нужны инклуды классов, которые они используют.

Код

C++ (Qt)
foreach (QTcpSocket* connection, connections)

foreach делает неявную копию всех элементов списка. В случае указателей наверно это небольшая потеря производительности, но она нам нужна?

Код

C++ (Qt)
if (ui->lineEdit->text() == "")

Возможно компилятор это и сможет оптимизировать, но может лучше isEmpty()?

Код

C++ (Qt)
ui->lineEdit->setValidator(new QRegExpValidator(regExp, this));
 

Может лучше QIntValidator? Он позволяет задать и минимум, и максимум, и отсеять все, что не целочисленное. Да и от этих дополнительных проверок избавится:

Код

C++ (Qt)
if (ui->lineEdit->text().toInt() > 65535)
 

А вообще, может лучше обычный QSpinBox поставить, чтобы не приходилось столько проверок делать?:

Код

C++ (Qt)
if (ui->lineEdit->text() == "")

Вместо пустоты там вполне может находится минимальное значение для порта и проверка не понадобится.

Может быть сделать смену состояний через QStateMachine и вынести в конструктор? Будет легче поддерживать приложение, если добавятся или пропадут кнопки/виджеты.

Код:

void MainWindow::slotStartClicked()
{
...
        ui->pushButton->setDisabled(true);
        ui->pushButton_2->setDisabled(false);
        ui->lineEdit->setDisabled(true);
...
}

void MainWindow::slotStopClicked()
{
        ui->pushButton->setEnabled(true);
        ui->pushButton_2->setEnabled(false);
        ui->lineEdit->setDisabled(false);

Аналогично вслед за сменой состояния можно динамически менять текст на QLabel:

Код

C++ (Qt)
ui->label->setText(tr("Server stopped"));
 

Item based widgets довольно медленны на больших объемах элементов. Лучше уж QPlainTextEdit, там хоть можно выставить setMaximumBlockCount() и сервер не рухнет, если ты его решишь на недельку другую оставить без присмотра.

Код

C++ (Qt)
ui->listWidget->addItem(ui->label->text());

Не нужно распихивать tr() для динамических строк. Я не думаю, что ты потом захочешь перевести английский текст типа "%1" на русский, например "Процент 1", а когда речь идет о цифре (peerPort()), то я думаю американцев в школе тоже учат арабским цифрам. Или ты хочешь обозначать цифры буквами как в Древней Руси?

Код

C++ (Qt)
        tmpClientAddressPort << tr("%1").arg(serverSocket->peerPort());
        str_buf = tr("%1").arg(serverSocket->peerPort());

Код

C++ (Qt)
    QList<QTcpSocket*> connections;
    QHash<QTcpSocket*, QBuffer*> buffers;
 

Вместо хранения обычных указателей в списках можно хранить QSharedPointer, при удалении любого из таких элементов из списка память освободится сама и в этом коде потребность отпадет:

Код

C++ (Qt)
        buffer->deleteLater();
        ...
        socket->deleteLater();
 

Базовый класс в QTcpSocket это QObject, думаю тут правильней будет использовать qobject_cast вместо static_cast:

Код

C++ (Qt)
QTcpSocket* socket = static_cast<QTcpSocket*>(sender());
 

Следующий код по меньшей мере странный:

Код

C++ (Qt)
void MainWindow::slotSocketRead()
{
 QTcpSocket* socket = static_cast<QTcpSocket*>(sender());
 QBuffer* buffer = buffers.value(socket);
 ui->label->setText("readyread");
 qint64 bytes = buffer->write(socket->readAll());
 buffer->seek(buffer->pos() - bytes);
 QByteArray line = buffer->readAll();
 ui->textEdit->append(line);
 sendToClients(line);
}
 

Когда приходят данные ты добавляешь их в буффер и выводишь в textEdit. Запросто может произойти ситуация, когда данные приходят частями, тогда твой textEdit будет выглядеть так:

Код:

He
Hell
Hello
Hello W
Hello World

Это уже касается самого протокола. Обычно люди шлют размер данных, который собираются передавать и лишь когда буффер станет равный этому размеру выводят результат. Стоит учесть, что в Unicode один символ - 2 байта, поэтому QString::length() или size() вернут значение в 2 раза меньше.

Попытка записи в закрытый сокет? Или "извини пользователь, я не могу тебе дать отключиться, твой компьютер мой раб навеки!"?

Код

C++ (Qt)
void MainWindow::slotClientDisconnected()
{
        ...
        QByteArray msg = "Client disconnected";
        foreach (QTcpSocket* connection, connections)
        {
                connection->write(msg);
        }
}
 

В потоках я не очень, поэтому хочу посоветоваться, какие функции раскидать по потокам. Очень нужна Ваша помощь.

Возвращаясь к главному вопросу. Потоков сделай изначально 5 (вообще нужно писать систему, которая бы сама вычисляла производительность сервера в зависимости от количества доступной памяти и потоков, но для начала можно поставить и фиксированное значение). Клиентов раскидывать равномерно по потокам. Соединение/разъединение всех клиентов можно осуществлять в одном потоке, в то время как обработку данных в остальных пяти. Наша задача вынести самые долги функции в отдельные потоки. Как правило это функции подготовки данных на запись в поток. Чтение с винчестера, долгие математические вычесления и тому подобное. В общем всё то, что может заморозить основной цикл событий и как следствие привести к отказу обслуживания сервером вместо того, чтобы "кормить" клиента обещаниями "вот вот, уже скоро, еще 5 минут и я тебе пришлю твои фотки". Исходя из этого у тебя сейчас только один, потенциальный, участок, который в будущем может стать "узким горлышком":

Код

C++ (Qt)
void MainWindow::slotSocketRead()
{
 ...
 QByteArray line = buffer->readAll();
 ui->textEdit->append(line);
 sendToClients(line);
}
 

Тут у тебя данные читаются из сокета и затем отправляются обратно. То есть получился echo server. На его месте могла бы быть пересылка тяжелого файла, поэтому в строках предшествующих buffer->readAll() должен происходить эмит сигнала о необходимости подготовки данных, который отправляется другому потоку. Соответственно строчка sendToClients() должна будет перенесена в слот, который будет ответом на сигнал из подготавливающего потока о том, что данные готовы к пересылке. Кстати убери весь код, относящийся к работе с GUI из слотов обрабатывающих сокеты. Все общение с GUI нужно реализовать посредством сигналов, а не прямых вызовов слотов виджетов. Только в этом случае мы можем гарантировать то, что никакое взаимодействие с виджетами не тормозит работу сервера, так как сигналы будут вставать в очередь и гарантировать нам то, что их обработка основым, GUI'шным потоком не блокируют работу, то есть не будет необходимости ждать завершения работы слотов типа ui->textEdit->append(line); Нам ведь не нужен возвращаемый результат? Тогда нафига ждать?

[zeonET]

спасибо, очень много нового узнал, занесу в закладки и буду перечитывать ))

[niXman]

SABROG, merke, работа идет? или уже забили?

[SABROG]

SABROG, merke, работа идет? или уже забили?

На данном этапе эта тема выходит за рамки моих интересов.