Потоки в классе С++ (проблема)

Модератор: Модераторы разделов

Аватара пользователя
AlphaGh0St
Сообщения: 41

Потоки в классе С++ (проблема)

Сообщение AlphaGh0St »

Всем привет! Пытаюсь разобраться в работе с потоками в классах С++.
Если писать на Си, проблем нет, но при использовании классов С++, они появляются...
Предположим, есть следующий код:

Код: Выделить всё

class thread{
private:
    pthread_t threads[3];
    pthread_mutex_t mutex;
    int k;

public:
    thread();
    void *inc_k(void *arg);
    void print_k();
};

thread::thread() : k(0){
    for(int i = 0; i < 3; i++)
        pthread_create(&threads[i], NULL, inc_k, NULL);

    for(int i = 0; i < 3; i++)
        pthread_join(threads[i], NULL);
}

void thread::*inc_k(void *arg){
    pthread_mutex_lock(&mutex);
    k++;
    pthread_mutex_unlock(&mutex);

    print_k();
}

void thread::print_k(){
    cout << "k = " << k << endl;
}


Здесь в конструкторе запускается 3 потока в качестве обработчика для потоков передаётся метод inc_k().
Но компилятор выдаёт ошибку, следующего содержания:

Код: Выделить всё

threads.cpp: In constructor «thread::thread()»:
threads.cpp:23:48: ошибка: аргумент типа «void* (thread::)(void*)» не соответствует типу «void* (*)(void*)»


Подскажите, что здесь не так? И как было бы правильно?
Спасибо сказали:
NickLion
Сообщения: 3408
Статус: аватар-невидимка
ОС: openSUSE Tumbleweed x86_64

Re: Потоки в классе С++ (проблема)

Сообщение NickLion »

Любой нестатический метод класса имеет неявный параметр this. Поэтому по параметрам не сходится. Как правильно: объявляете inc_k статическим, а как аргумент потоку передаёте this. внутри данного статического метода работаете с объектом static_cast<thread*>(arg).
Например, так:

Код: Выделить всё

#include <pthread.h>
#include <iostream>

class thread{
private:
    pthread_t threads[3];
    pthread_mutex_t mutex;
    int k;

public:
    thread();
    void *inc_k();
    static void* inc_k_(void* arg);
    void print_k();
};

thread::thread() : k(0){
    for(int i = 0; i < 3; i++)
        pthread_create(&threads[i], NULL, inc_k_, this);

    for(int i = 0; i < 3; i++)
        pthread_join(threads[i], NULL);
}

void* thread::inc_k_( void* arg )
{
    static_cast< thread* >( arg )->inc_k();
}

void* thread::inc_k(){
    pthread_mutex_lock(&mutex);
    k++;
    pthread_mutex_unlock(&mutex);

    print_k();
}

void thread::print_k(){
    std::cout << "k = " << k << std::endl;
}
Спасибо сказали:
Аватара пользователя
AlphaGh0St
Сообщения: 41

Re: Потоки в классе С++ (проблема)

Сообщение AlphaGh0St »

Вопрос о производительности...
Пишу небольшую программку, работающую с потоками в целях самообучения.

Программка простенькая, она (однопоточный режим) открывает файл, в котором содержатся записи, типа ip:port, считывает эти записи в вектор. Затем (многопоточный) считывает из вектора запись, отделят Ip и port в разные переменные.

Не так важно содержимое файла, не так важно разделение, больше всего интересует производительность.
Вот код программки:

Код: Выделить всё

#include <iostream>
#include <stdlib.h>
#include <pthread.h>
#include <vector>
#include <string.h>
#include <fstream>

#define THREADS_COUNT 3 // кол-во потоков

using namespace std;

class NodeList : public vector<string>{
private:
    pthread_t pl_threads[THREADS_COUNT];
    pthread_mutex_t pl_mutexLock;
    ifstream pl_file;
    string pl_node, pl_addr, pl_port;
    NodeList::iterator pl_it;

    static unsigned short count;

public:
    NodeList(char *file);
    ~NodeList();

    void readNodeFromFile();
    void getAddrAndPort();
    void testMethod();
    static void *threadsStart(void *arg);
    void *threadsWork();
};
unsigned short NodeList::count = 0;
//-----------------------------------------------------------------
NodeList::NodeList(char *file) : pl_mutexLock(PTHREAD_MUTEX_INITIALIZER){
    // открываем файл
    pl_file.open(file);
    if(!pl_file){
        perror("[!] Cannot open file");
        exit(1);
        }

    // считываем все записи из файла в вектор
    readNodeFromFile();

    for(int i = 0; i < THREADS_COUNT; i++)
        if(pthread_create(&pl_threads[i], NULL, threadsStart, this) != 0)
            perror("[!] Cannot create thread");

    for(int i = 0; i < THREADS_COUNT; i++)
        pthread_join(pl_threads[i], NULL);
}
//-----------------------------------------------------------------
NodeList::~NodeList(){ cout << "count -> " << count << endl; }
//-----------------------------------------------------------------
// считываем все данные из файла и сохраняем их в вектор
void NodeList::readNodeFromFile(){
    while(!pl_file.eof()){
        pl_file >> pl_node;
        this->push_back(pl_node);
        }

    pl_file.close();
}
//-----------------------------------------------------------------
/*
  в вектор считались записи типа ip:port
  этот метод отделит ip и port
  и удалит из вектора считанную запись
*/
void NodeList::getAddrAndPort(){
    size_t len;
    int i;
    string tmp;

    if(this->empty()){
        cout << "[!] No more nodes in list\n";
        exit(1);
        }

    pl_it = this->begin();
    pl_addr.clear();
    pl_port.clear();
    tmp = *pl_it;
    len = tmp.find(":");

    for(i = 0; i < len; i++)
        pl_addr.push_back(tmp[i]);

    i++;
    for(; i < tmp.size(); i++)
        pl_port.push_back(tmp[i]);

    this->erase(pl_it);
}
//-----------------------------------------------------------------
void NodeList::testMethod(){ count++; }
//-----------------------------------------------------------------
void *NodeList::threadsStart(void *arg){ static_cast<NodeList *>(arg)->threadsWork(); }
//-----------------------------------------------------------------
/*
  каждый поток вызовет этот метод
  пока в векторе есть записи типа ip:port
  мьютекс заблокируется, поток извлечёт первую запись
  мьютекс разблокируется, поток пойдёт дальше
*/
void *NodeList::threadsWork(){
    while(!this->empty()){
        pthread_mutex_lock(&pl_mutexLock);
            getAddrAndPort();
        pthread_mutex_unlock(&pl_mutexLock);

        testMethod();
        }
}
//-----------------------------------------------------------------
int main(int argc, char *argv[]){
    NodeList nList(argv[1]);

    return 0;
}


Что-то здесь не так...
У меня на ноуте ОС Ubuntu 11.10 32-bit, процессор Intel двухядерный по 1.73Ghz на каждом.
В передаваемом программе файле, содержится 24048 записей типа ip:port.

Запуская программу с одним потоком (THREADS_COUNT 1), время её выполнения ~7 секунд (+2 мл сек).
Запуская программу с тремя потоками (THREADS_COUNT 3), время её выполнения также ~7 секунд (+2 мл сек).
Даже запуская программу с шестью потоками (THREADS_COUNT 6), время её выполнения также, как и раньше составляет ~7 секунд (+2 мл сек).

Т.е., сколько потоков не запускай, разницы не ощущается.
Единственное "узкое" место в программе, которое я вижу, это в методе threadsWork(), когда мьютекс блокируется. Но не может же такого быть, чтобы использование мьютексов сводило на "нет" всю пользу многопоточности.

Подскажите, в чём здесь дело? Я уже просто не могу понять, в чём "весь прикол"...
Спасибо сказали:
Аватара пользователя
/dev/random
Администратор
Сообщения: 5495
ОС: Gentoo

Re: Потоки в классе С++ (проблема)

Сообщение /dev/random »

AlphaGh0St писал(а):
21.10.2011 22:31
Единственное "узкое" место в программе, которое я вижу, это в методе threadsWork(), когда мьютекс блокируется. Но не может же такого быть, чтобы использование мьютексов сводило на "нет" всю пользу многопоточности.

Мьютексами следует огораживать операции, занимающие незначительное время. У вас же почти вся работа, выполняемая потоками, выполняется именно между блокировкой и разблокировкой мьютекса. Да, _такое_ использование мьютексов, действительно, сводит на нет всю пользу многопоточности.
Спасибо сказали:
Аватара пользователя
AlphaGh0St
Сообщения: 41

Re: Потоки в классе С++ (проблема)

Сообщение AlphaGh0St »

А как же быть в данной ситуации?
Что Вы можете посоветовать в плане изменения кода?
Спасибо сказали:
Аватара пользователя
RasenHerz
Сообщения: 1341
ОС: Arch Linux amd64

Re: Потоки в классе С++ (проблема)

Сообщение RasenHerz »

Почему класс NodeList, являющийся классом для работы со списком строк содержит логику управления потоками? Это серьезная ошибка проектирования. Максимум что в нем должно быть в данном случае - мьютекс, который будет запираться при добавлении/удалении/модификации элемента списка. Примерный алгоритм работы программы должен быть такой.

Код: Выделить всё

1. Создаете список NodeList (в виде статической переменной или любым другим способом, главное чтобы каждый поток мог получить доступ к экземпляру класса)
2. Считываете в него данные.
3. Создаете потоки которые:
   3.1 Извлекают элемент из списка //тут происходит запирание мьютекса в списке
   3.2 Обрабатывают данные. // а здесь мьютекс списка уже не заперт
   3.3 Если список не пуст, то повторяется шаг 3.1, иначе - поток завершается. // Стоит отметить, что функция проверяющая пуст ли список должна запирать мьютекс
4. Программа ждет завершения работы потоков.
Спасибо сказали:
aumit
Сообщения: 28

Re: Потоки в классе С++ (проблема)

Сообщение aumit »

Переписать код, без удаления элемента в векторе.
Спасибо сказали:
NickLion
Сообщения: 3408
Статус: аватар-невидимка
ОС: openSUSE Tumbleweed x86_64

Re: Потоки в классе С++ (проблема)

Сообщение NickLion »

Добавлю к тому, что сказал RasenHerz. Если данные, находящиеся в файле "тяжёлые" и для чтения, и для обработки, то пункты 2 и 3 лучше совместить: поток 1 читает и помещает данные в очередь, а обработчики выбирают из очереди по мере появляения там данных.

НО! Всё это имеет смысл при хорошем коде. Уж извините, но посмотрев на NodeList::getAddrAndPort в данной программе я немного опешил. Вот это:

Код: Выделить всё

    tmp = *pl_it;
    len = tmp.find(":");

    for(i = 0; i < len; i++)
        pl_addr.push_back(tmp[i]);

    i++;
    for(; i < tmp.size(); i++)
        pl_port.push_back(tmp[i]);

НУЖНО заменить на:

Код: Выделить всё

    len = pl_it->find(':');
    pl_addr = pl_it->substr(0, len);
    pl_port = pl_it->substr(len + 1);


Второе - абсолютно непонятна мотивация использовать std::vector вместо подходящего намного более (по производительности push_back, erase) в данной задаче std::list.
Спасибо сказали:
Аватара пользователя
AlphaGh0St
Сообщения: 41

Re: Потоки в классе С++ (проблема)

Сообщение AlphaGh0St »

Следуя вышеперечисленным советам, переделал код.
В итоге получилось так:

Код: Выделить всё

#include <iostream>
#include <stdlib.h>
#include <pthread.h>
#include <list>
#include <string.h>
#include <fstream>

using namespace std;

#define THREADS_COUNT 3
//-----------------------------------------------------------------------------------------------------------------------------
// структура для хранения адреса и порта
struct NodeAddrAndPort{
    string na_addr;
    string na_port;
};
//-----------------------------------------------------------------------------------------------------------------------------
class NodeList : protected list<struct NodeAddrAndPort>{
private:
    pthread_mutex_t pl_mutexLock;
    ifstream nd_nodeFile;
    string nd_node;
    NodeList::iterator pl_it;
    struct NodeAddrAndPort pl_tmp;

    static unsigned int count; // кол-во считанных записей

public:
    NodeList(char *prList);
    ~NodeList();

    // считываем данные в список
    void readNodeFromFile();

    // вернуть первую запись из списка
    // а потом удалить её
    struct NodeAddrAndPort getNode();
    void showInfo(string ip, string port);

private:
    // переопределённый метод
    bool empty();
};
unsigned int NodeList::count = 0;
//-----------------------------------------------------------------------------------------------------------------------------
NodeList::NodeList(char *file) : pl_mutexLock(PTHREAD_MUTEX_INITIALIZER){
    nd_nodeFile.open(file);
    if(!nd_nodeFile){
        perror("[!] Cannot open file");
        exit(1);
        }
}
//-----------------------------------------------------------------------------------------------------------------------------
NodeList::~NodeList(){ cout << "count -> " << count << endl; }
//-----------------------------------------------------------------------------------------------------------------------------
struct NodeAddrAndPort NodeList::getNode(){
    if(this->empty()){
        cout << "[!] vector is empty!\n";
        exit(1);
        }

    pthread_mutex_lock(&pl_mutexLock);
        NodeAddrAndPort pa_tmp;

        pl_it = this->begin();
        pa_tmp = *pl_it;

        this->erase(pl_it);
    pthread_mutex_unlock(&pl_mutexLock);

    return pa_tmp;
}
//-----------------------------------------------------------------------------------------------------------------------------
void NodeList::readNodeFromFile(){
    size_t len;

    while(!nd_nodeFile.eof()){
        nd_nodeFile >> nd_node;

        pl_tmp.na_addr.clear();
        pl_tmp.na_port.clear();

        len = nd_node.find(":");

        pl_tmp.na_addr = nd_node.substr(0, len);
        pl_tmp.na_port = nd_node.substr(len + 1);

        this->push_back(pl_tmp);
        }

    nd_nodeFile.close();

    /* последняя запись заносится
       в список дважды, по этому
       удаляем её
    */
    this->pop_back();
}
//-----------------------------------------------------------------------------------------------------------------------------
void NodeList::showInfo(string ip, string port){
    cout << "\nvec.size() = " << this->size() << endl
    << "addr: " << ip << "\nport: " << port << "\n\n";

    count++;
}
//-----------------------------------------------------------------------------------------------------------------------------
bool NodeList::empty(){
    pthread_mutex_lock(&pl_mutexLock);
        bool ret = list<struct NodeAddrAndPort>::empty();
    pthread_mutex_unlock(&pl_mutexLock);

    return ret;
}
//-----------------------------------------------------------------------------------------------------------------------------
void *threadsStart(void *arg){
    NodeAddrAndPort tmpSt;
    NodeList *tmpCl = (NodeList *)arg;

    while(1){
        tmpSt = tmpCl->getNode();
        tmpCl->showInfo(tmpSt.na_addr, tmpSt.na_port);
        }
}
//-----------------------------------------------------------------------------------------------------------------------------
int main(int argc, char *argv[]){
    pthread_t threads[THREADS_COUNT];
    static NodeList nList(argv[1]);

    nList.readNodeFromFile();

    for(int i = 0; i < THREADS_COUNT; i++)
        if(pthread_create(&threads[i], NULL, threadsStart, (void *)&nList) != 0)
            perror("[!] Cannot create thread");

    for(int i = 0; i < THREADS_COUNT; i++)
        pthread_join(threads[i], NULL);

    return 0;
}


Для учебного примера, считаю, что этого будет вполне достаточно.
Всем спасибо за помощь!
Спасибо сказали:
NickLion
Сообщения: 3408
Статус: аватар-невидимка
ОС: openSUSE Tumbleweed x86_64

Re: Потоки в классе С++ (проблема)

Сообщение NickLion »

AlphaGh0St, самое главное не сказали - что там с многопоточностью? Получилось сделать более производительным?
Спасибо сказали:
Аватара пользователя
AlphaGh0St
Сообщения: 41

Re: Потоки в классе С++ (проблема)

Сообщение AlphaGh0St »

Что касается производительности, тут всё не совсем так, как хотелось бы.
Метод GetNode() запирается мьютексом, а вся последующая обработка выбранных данных происходит в методе showInfo(), при обработке, мьютекс не запирается, т.е. по идее потоки должны выполняться параллельно.
Для простоты, метод обработки лишь выводит на экран <размер списка> <адрес> <порт> и инкриминирует переменную count. Вот он:

Код: Выделить всё

void showInfo(string ip, string port){
    cout << "\nsize() = " << this->size() << endl
    << "addr: " << ip << "\nport: " << port << "\n\n";

    count++;
}


Результаты выполнения следующие:
1 поток => ~13.7 сек
2 потока => ~10.7 сек
3 потока => ~10.7 сек
4 потока => ~11.0 сек
Не особо, могло бы быть и лучше...

Ещё смущает одна деталь, при работе в нескольких потоках, происходит ошибка сегментации. Не всегда, но редко. И не на конкретном месте.
Например:
Запустил программу раз - всё нормально.
Запустил второй раз - ошибка на 5000-ой записи.
Запустил третий раз - ошибка на 2000-ой записи.
Запустил четвёртый раз - всё нормально.
Спасибо сказали:
aumit
Сообщения: 28

Re: Потоки в классе С++ (проблема)

Сообщение aumit »

сделай core file и в gdb смотреть.
Спасибо сказали:
Аватара пользователя
AlphaGh0St
Сообщения: 41

Re: Потоки в классе С++ (проблема)

Сообщение AlphaGh0St »

проблему с ошибкой сегментации решили.
нужно убрать вызов this->size() из showInfo().

а вот увеличения производительности за счёт многопоточности не наблюдается...
Спасибо сказали:
Аватара пользователя
/dev/random
Администратор
Сообщения: 5495
ОС: Gentoo

Re: Потоки в классе С++ (проблема)

Сообщение /dev/random »

AlphaGh0St писал(а):
24.10.2011 17:22
а вот увеличения производительности за счёт многопоточности не наблюдается...

А с чего бы ей наблюдаться? Вашу программу можно условно разделить на 4 операции: чтение из файла, парсинг с формированием списка, получение и удаление каждого элемента из списка, вывод этого элемента. Так вот, из всех этих операций у вас параллельно выполняется только вывод. Чтение производится до почкования потоков, парсинг совмещён с чтением, получение/удаление элемента огорожено мьютексами. Кстати, ещё интересно, нет ли каких-нибудь flock-блокировок внутри операций ввода-вывода в стандартной библиотеке? Если есть, то у вас параллельно не выполняется вообще ничего.
Спасибо сказали: