Нужен совет по С++ (vector + class + mutex + thread)

vector <Device>

Сделай вектор указателей vector <Device*>, тогда копироваться будут указатели, а не объекты. Но тут нужно не забыть освобождать память. Или использовать умные указатели.

мне нужно добавить еще 3 объекта класса Device после объекта с индексом 6

Неправильно выбрана коллекция. Вектор тебе не подходит, тут нужен связный список, например std::list.

Спасибо большое за ответ, только в голову пришла мысль про вектор указателей и Вы ее подтвердили. А вот насчет std::list, даже не думал. Сейчас пойду смотреть, что это за зверь. Набиваю шишки на своем проекте, задумка оказалась проще реализации))

Вектор - непрерывный кусок памяти, в котором объекты идут друг за другом. Чтобы вставить элемент в середину, нужно элементы идущие после места вставки передвинуть на нужное число позиций. Это относительно не быстро. А если новый размер вектора больше чем было зарезервировано, то будет полное копирование, что ещё хуже.

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

В некоторых случаях

Почти всегда.

если мне нужно будет вызывать их в цикле, элементы, то нужно делать это как типа так: list.begin()+i, где i это итератор в цикле, который должен быть равен элементу массива?

У тебя какой стандарт крестов? c++11 есть?
Если нужно пройтись по коллекции, то можно сделать вот так:

vector<Device*> DeviceList;
for (Device* dev : DeviceList)
{
   dev->...
}

с++14. Я имел ввиду если это будет не вектор, а list. Допустим мне нужно вызвать со списка 7 элемент, в векторе можно сделать DeviceList.at(6), как сделать такое же с list?

Этот for будет работать и со списком и с вектором.

Допустим мне нужно вызвать со списка 7 элемент

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

На самом деле, это уже становится похоже на преждевременную оптимизацию, что не есть хорошо. Если ты не ворочаешь миллионы элементов, то можно забить на всё это и использовать вектор. Зря я со списком влез.

DeviceList.at(6)

Метод at() проводит проверку на выход за границу и бросает исключение. Ты это поведение учитываешь? Если оно не нужно, то проще использовать оператор [].

Я почитал чуть-чуть про список, думаю что мне все таки больше подойдет вектор. Но зато я теперь знаю что еще есть список, так что не «зря влез»))) Спасибо. У меня такая хрень со вставкой элементов в разные части вектора будет происходить только в самом начале программы, программа просматривает разные конфиг файлы и решает нужно добавлять элементы или нет и в какую часть вектора. После того как конфигурация закончиться, нужно будет чаще обращаться к элементам по индексу, выходя из всего вышесказанного, мне кажется, лучше в самом начале тупануть, чем потом в процессе работы))) Буду пробовать вектор* указателей

Использую at() как дополнительную страховку от выхода за границы вектора, дальше пока не думал

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

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

В операторе копирования в классе Device, попробовал создавать новый мютекс при копировании объектов

T& operator = (const T& other); // Оператор присваивания
T(const T& other); // конструктор копирования 

Кого из них ты обозвал оператором копирования?:)

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

коллекции

какое же это мерзкое явовское словечко... в мире c++ принято говорить «контейнеры»

Вариант №1: делаете vector<shared_ptr<Device>>. Большой плюс в том, что если вы где-то напишете:

auto dev = devices[i];
devices.push_back(make_shared<Device>(...));
dev->foo(); // Здесь все нормально, никаких висящих указателей.

Если есть желание экономить «на спичках», то можно и vector<unique_ptr<Device>>.

При работе unique_ptr (да и с shared_ptr) можно будет делать вот так:

auto & dev = *devices[i];
devices.push_back(make_unique<Device>(...));
dev.foo(); // Здесь все нормально, никаких висящих указателей.

Вариант №2: использовать в Device идиому PImpl и реализовать для Device конструктор и оператор перемещения (при этом запретив конструктор и оператор копирования). Что-то вроде:

class Device {
  struct Impl {
    std::mutex lock_;
    ...
  };
  std::unique_ptr<Impl> impl_;
public :
  Device() : impl_(std::make_unique<Impl>(...)) {}
  Device(const Device &) = delete;
  Device(Device && o) = default;

  Device & operator=(const Device &) = delete; // Из-за паранойи.
  Device & operator=(Device &&) = default;
  ...
};

Прошу прощения я еще нуб))) Я хотел сказать оператор присваивания

Ну вот, а вектор требует что бы работали и оператор присваивания и конструктор копирования :)

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

А можно пример, если не сложно, буду ооочень благодарен)))

Когда делаешь insert, это нужно делать с std::move()

device_list.insert( it, std::move(Device()) );

Вот работающий пример (собирать g++ -std=c++14 -O3 main.cpp).

#include <vector>
#include <utility>

class Device
{
public:
    Device();
    Device( unsigned id );
    Device( Device&& other ); // noexcept
    Device& operator=( Device&& ); // noexcept

    Device( const Device& ) = delete;
    Device& operator=( const Device& ) = delete;

    unsigned id()
    { return m_device_data; }
private:
    static unsigned counter;

    unsigned m_device_data = 0xAA55;
};

unsigned Device::counter = 0;

Device::Device()
    :m_device_data( Device::counter++ )
{}

Device::Device( unsigned id )
    :m_device_data( id )
{}

Device::Device( Device&& other ) // noexcept
    :m_device_data( std::move(other.m_device_data) )
{}

Device& Device::operator=( Device&& other ) // noexcept
{
    if( &other == this )
        return *this;
    m_device_data = std::move( other.m_device_data );
    return *this;
}

#include <iostream>

static void print_ids( std::vector<Device>& device_list )
{
    std::cout << "device ids: ";
    for( auto& device : device_list )
        std::cout << device.id() << " ";
    std::cout << "\n";
}
int main()
{
    std::vector<Device> device_list(11);
    print_ids( device_list );

    auto it = device_list.begin();
    ++it; ++it; ++it;
    device_list.insert( it, std::move(Device( 19 )) );

    print_ids( device_list );
    return 0;
}

Спасибо!

А как все таки правильнее сделать? Массив указателей на объект или массив объектов? А то если честно много советов я запутался куда двигаться))

А как все таки правильнее сделать? Массив указателей на объект или массив объектов?

Во-первых, с голыми указателями в C++ уже не работают, сейчас не 90-е годы. Используют либо unique_ptr, либо shared_ptr. Во-вторых, это зависит от того, кто владеет объектом (кто его должен уничтожать). Если есть единственный владелец - то либо контейнер с объектами, либо с unique_ptr на объекты. Если владельцев несколько, то shared_ptr.

Тема эта довольно обширная, одним комментарием не отделаться. Кроме того, у тебя еще используется и многопоточность, так что, скорее всего, ты не осилишь решение этой задачи с первого раза (т.е. дойдешь до него за несколько итераций, если не надоест). Кстати, тебе нужно еще позаботиться о том, чтобы твой контейнер (в котором хранятся объекты и по которому ты, наверняка, будешь делать поиск, не менялся, когда другой поток ищет в нем Device).

В общем, если расскажешь, про архитектуру своего приложения (зачем тебе контейнер, кто (какие потоки) и какие операции с ним выполняет, и почему Device должен работать в многопоточном режиме), то тебе можно будет что-то более конкретное сказать. Но скорее всего ответ будет «нужно поменять архитектуру таким-то образом» :), потому что многопоточка тема сложная, и чтобы все работало, ее нужно упрощать. Поэтому в реальной жизни со сложными многопоточными приложениями не работают (построение иерархии mutex'ов сложнее чем shared_mutex для избежания deadlock - это изобретение яйцеголовых задротов-теоретиков, жертв высшего образования с избытком знаний и недостатка опыта, в реальных приложениях так не делают, потому что любое изменение в приложении потребует заново продумывать всю иерархию - обязательно ошибешься, это вопрос времени). Если у тебя система сложнее, чем производитель-потребитель (producer-consumer), то архитектуру меняют таким образом, чтобы остались только куски типа производитель-потребитель, а остальные части работали в однопоточном режиме, причем асинхронно. В результате появились агентно-ориентированный подход, горутины, которые работают с каналами и другие разновидности (которые разбивают приложение на небольшие части, работающие однопоточно, а с другими частями общаются через thread safe очереди и никакие данные между друг другом не делят, иначе dead lock).

Скорее всего тебе нужно сделать, чтобы к твоему контейнеру с объектами могли обращаться из разных потоков, но после получения доступа к объекту, поток получатель становится единоличным пользователем объекта, пока не вернет его обратно. Однако, это все предположения, я не знаю, какая у тебя архитектура, и что делает твое приложение.

Спасибо за обширный ответ. Постараюсь описать что должно делать мое приложение и зачем я начал работать с потоками. Буду очень рад, если надеться время и желание подсказать мне правильный путь)))

Вы знаете, вот попробовал описать что делает моя программа и для чего ей вообще потоки и mutex, пришла в голову идея (даже две) и понимание, что возможно Вы правы и потоки тут не так уж и необходимы. Ведь по сути, все что должна делать эта программа - это парсить конфиг или конфиги, после чего по средством диапазонного for отслеживать изменение состояний устройств (например было 0 - стало 1) и при событии отправлять данные об этом устройстве в другую программу, затем ждать пока это устройство не вернет свое прежнее состояние. Потоки я хотел использовать для параллельной работы, чтобы программа в цикле не тратила время на передачу данных и мониторинг обратного события (с 1 на 0), это, по задумке, должны были делать методы в другом, открепленном, потоке.

Круто, ты догадался, что твою программу можно реализовать, как конечный автомат, когда у нужных объектов есть состояния и внешние события изменяют их состояния в зависимости от текущего состояния. Теперь нужно понять, как мониторить внешние события (постоянный цикл опроса - это ресурсоемко для CPU, он будет постоянно занят).

Обожаю вас, пацаны. Меня ты проигнорировал, но стоило тебе какому-то рандому наплести какой-то муры - ты тут же побежал ему отвечать, при этом приписал ему то, чего он не говорил:

Вы правы и потоки тут не так уж и необходимы.

Он этого не говорил. Он предлагал тебе поверх твоего вектора бахнуть мутекс, а не поверх елемента. Перед этим он тебе ретранслировал хипстерские дворовые поверья про асинхронность. Проблема только в том, что тебя обманули.

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

Потоки я хотел использовать для параллельной работы, чтобы программа в цикле не тратила время на передачу данных и мониторинг обратного события (с 1 на 0),

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

Вторая проблема - проблема синхронизации. Как сообщим другим о том, что ты что-то поменял? На самом деле такой проблемы нет - все кэши и память когерентны. Проблема заключается только в том, что когда это произойдёт.

Мутексы и прочая подобная херня - это просто механизмы заставить программу работать как однопоточную в определённом месте. В некоторых случае их свойство слипать поток бывает полезным и их используют только для этого.

это, по задумке, должны были делать методы в другом, открепленном, потоке.

Я так и не понял что тебе надо.

Ты хочешь сделать что-то типа:

#include <chrono>
#include <cstdio>
#include <vector>
#include <algorithm>
#include <thread>
#include <string>

using namespace std::string_literals;
using namespace std::chrono_literals;

class device {
public:
  device(const std::string & name) : name(name) {}
  
  
  
  void run() {
    state = false;
    std::thread([this]() {
      std::this_thread::sleep_for(std::chrono::milliseconds(rand() & 1000));
      state = true;
    }).detach();
  }
  
  
public:
  std::string name;
  volatile bool state = true;
};



int main() {
  std::vector<device> v;
  
  std::generate_n(std::back_inserter(v), 10, [&] {
    return device{"device"s += std::to_string(v.size())};
  });
  
  while(true) {
    for(auto & x: v) {
      if(x.state) {
        fprintf(stderr, "Готовы, пускаем: %s::run()\n", x.name.c_str());
        x.run();
      } else {
        fprintf(stderr, "Обратная связь из: %s, шепчет: Вы не готовы!\n", x.name.c_str());
      }
    }
    std::this_thread::sleep_for(100ms);
  }
}

Только тригер у тебя будет не «пинг-понг», а что-то левое?

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

(которые разбивают приложение на небольшие части, работающие однопоточно, а с другими частями общаются через thread safe очереди и никакие данные между друг другом не делят, иначе dead lock).

Это натолкнуло меня на мысль, а может я гланды через жопу режу?О_о Что, в принципе, может быть так как я только учусь. Я действительно благодарен каждому, кто оставил тут ответ, так как ни одни занятия и даже литература не в состоянии объяснить кучу разных нюансов, которые всплывают во время написания. Практика и еще раз практика и задачи которые кажутся нереальными для моего уровня.

Смысл программы вы описали верно, как понять

не «пинг-понг», а что-то левое?

Спасибо!

Теперь нужно понять, как мониторить внешние события (постоянный цикл опроса - это ресурсоемко для CPU, он будет постоянно занят).

Подскажите напраление?

Смысл программы вы описали верно, как понять

Ну в данном случае делает run() он тогда, когда предыдущий run() заканчивается. Т.е. отработка run() является тригером для запуска нового run().

Что у вас является тригером для запуска run()?

Подскажите напраление?

Для сетевых сокетов есть select/poll/epoll, для всего остального не знаю. Возможно, в твоей задаче придется делать цикл, который постоянно опрашивает Device, потому что по другому узнать об изменении его состояния нельзя.

Есть ощущение, что вам нужно посмотреть на уже готовые вещи. Вроде QP/C++ и OOSMOS (ну или на CAF и SObjectizer). С большой вероятностью в одном из них уже будет вся необходимая вам обвязка для организации циклов обработки ваших событий.

Анонимус, помни: std::mutex не поддерживает move семантику! А у автора как раз мьютексы внутри объектов.

Обожаю вас, пацаны. Меня ты проигнорировал, но стоило тебе какому-то рандому наплести какой-то муры - ты тут же побежал ему отвечать, при этом приписал ему то, чего он не говорил

Царь, ты чего на людей кидаешься?

Если я правильно понимаю, то run() у меня будет должен запускаться когда состояние какого-то из устройств меняет свое состояние, т.е. если было 0, а стало 1 - запускается run() в параллельном потоке, блокирует устройство для основного потока и ожидает пока состояние не вернется в прежнее, которое было до запуска run(), после этого метод завершаться. Т.е. программа знает, что это устройство активно и блочит его дабы основной поток не тратил время на его опрос, в свою очередь основной поток при попытке прочесть статус этого устройства получает отказ и идет дальше

Спасибо, проштудирую эти либы

состояние какого-то из устройств меняет свое состояние,

Что есть «состояние»?

блокирует устройство для основного потока

Что значит «блокируется»?

Больше отборной лапши. Зачем писать 10строчек - напишет 110, а лучше 1110? Ещё и 100мегабайт лапши засунем в зависимости.

Кстати, перепиши мой тот хелворд на твоё творение, пж. Мне даже интересно.

Кстати, перепиши мой тот хелворд

Какой «тот»?

Этот: Нужен совет по С++ (vector + class + mutex + thread) (комментарий)

А словами описать то, что должен делать код? Мне не понятно какова должна быть связь между работающими на разных нитях Device-ами и главной нитью приложения.

Ты определись. Если хочешь дёшево вставлять в середину, то это одна коллекция. Если быстрый доступ по индексу, то другая.

ага, можно ещё дерево поиска фигануть, но зачем? он же оперирует десятками объектов, там хоть как можно

Делать то же, что и мой. По установленному флагу внутри девайса - пускать обработку над этим девайсом в другом треде, устанавливая флаг «блокировки» внутри девайса, а другой тред должен сообщать об окончании работы путём разблокирования девайса вернув значение флага наместо.

В данном случае флаг пуска обработки совмещён с флагом блокировки и пускает обработку тогда, когда флаг блокировки «снят».

Что конкретно надо пацану и какие там у него взаимосвязи - я не знаю. Пока что мы остановились на таких.

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

Хотел сказать свой статус, было true стало false.

Насчет того, что должна делать программа и зачем там блокировки. Если не лезть в дебри моего проекта, то можно объяснить это таким образом. Есть класс А, в нем есть члены класса int x, string name..., также класс А содержит метод increment, который в цикле делает x++ 20 раз подряд. Теперь есть модуль main, в этом модуле создается Вектор объектов класса А, размер вектора = 15. Далее используя цикл while(true) и диапазонный for, для каждого элемента вектора (объекта класса А) в отдельном потоке вызывается метод increment(), который модифицирует закрытую часть класса. А так как вектор класса А находиться в вечном цикле, есть большая вероятность того, что программа будет пытаться выполнять метод increment несколько раз в одно и тоже время над одним и тем же объектом, что не есть хорошо. Следовательно нужно дать программе понять, что этот объект уже в обработке и его трогать не нужно, его нужно просто пропускать. Конечно можно так же для этих целей использовать что-то типа bool lock(false), если добавить его в класс и перед запуском инкремента тупо спрашивать у объекта lock == true - тогда пропускаем, объект уже в обработке, false - значит посылаем объект на инкремент. Но наверное это не совсем верно.

P.S. Я не стал писать что в вечном цикле предусмотрена передышка и он не грузить процессор, так же не стал описывать реализацию выхода из цикла, я просто постарался описать суть программы.

Всем спасибо!

Ну как-то так.

Создается 10 агентов device, каждый из которых работает на свой собственной нити. Через случайные промежутки времени device просыпаются и отсылаю синхронный запрос device_ready на главную нить (это вместо выставления флагов в твоем варианте). Когда главная нить отвечает на запрос, агент вновь засыпает на случайное время.

Главная нить на обработку каждого запроса берет 100ms, простым sleep-ом.

Если кому-то нужна версия этого примера с поясняющими комментариями, дайте знать. Постараюсь расписать, что здесь к чему.

#include <so_5/all.hpp>

using namespace std;
using namespace so_5;

class device final : public agent_t {
	struct turn_flag final : public signal_t {};
public :
	struct device_ready final : public message_t {
		const string name_;
		const thread::id tid_;

		device_ready(string name, thread::id tid)
			: name_(move(name)), tid_(tid)
		{}
	};

	device(context_t ctx, string name, mbox_t processor)
		:	agent_t(move(ctx)), name_(move(name)), processor_(move(processor)) {
		so_subscribe_self().event(&device::on_turn_flag);
	}

	virtual void so_evt_start() override {
		delay_turn_flag_signal();
	}

private :
	const string name_;
	const mbox_t processor_;

	void delay_turn_flag_signal() {
		send_delayed<turn_flag>(*this, chrono::milliseconds(rand() & 1000));
	}

	void on_turn_flag(mhood_t<turn_flag>) {
		request_value<void, device_ready>(
				processor_, infinite_wait, name_, this_thread::get_id());
		delay_turn_flag_signal();
	}
};

void make_devices(environment_t & env, const mbox_t & processor) {
	auto disp = disp::active_obj::create_private_disp(env);
	for(size_t i = 0u; i != 10u; ++i ) {
		env.introduce_coop(disp->binder(), [&](coop_t & coop) {
			coop.make_agent<device>("device"s + to_string(i), processor);
		});
	}
}

int main() {
	wrapped_env_t sobj;
	auto processor_ch = mchain_master_handle_t::make(
			create_mchain(sobj.environment()),
			mchain_props::close_mode_t::drop_content);

	make_devices(sobj.environment(), (*processor_ch)->as_mbox());

	receive(from(*processor_ch).total_time(chrono::seconds(15)),
			[](mhood_t<device::device_ready> cmd) {
				cout << cmd->name_ << " is ready, tid: " << cmd->tid_ << endl;
				this_thread::sleep_for(chrono::milliseconds(100));
			} );

	return 0;
}

Да, конечно, я даже не представлял что будет настолько ужасно. Это не в обиду тебе.

Создается 10 агентов device, каждый из которых работает на свой собственной нити. Через случайные промежутки времени device просыпаются и отсылаю синхронный запрос device_ready на главную нить (это вместо выставления флагов в твоем варианте). Когда главная нить отвечает на запрос, агент вновь засыпает на случайное время.

А где, собственно, полезная работа в твоём агенте? Слип там не задержка, а эмуляция работы.

Главная нить на обработку каждого запроса берет 100ms, простым sleep-ом.

Обработку должна делать не главная нить.

А где, собственно, полезная работа в твоём агенте?

Царь, блин, иди лекарство прими. Это такой же бесполезный hello_world, как и твой. В нем столько же полезной работы, как и в твоем коде.

Ну и да, как мне использовать твоего «агента» как обычный объект с моим апи?

В упор не вижу твоего API.

Царь, блин, иди лекарство прими. Это такой же бесполезный hello_world, как и твой. В нем столько же полезной работы, как и в твоем коде.

Не, не, не. Ишь ты как похитрить захотел. Ну рандомный пацан не понял бы почему ты сделал так - потому тебе и живётся норм. Значит ты просто обманул и не поставил свои акторы в реальные потоки, а вроде как писал, что у тебя такая фича есть.

В упор не вижу твоего API.

Опять пытаешься юлить. в качестве device, в моём случае, может выступать какой угодно класс. И там может быть какое угодно апи. И оно будет именно таким, каким будет.

#include <chrono>
#include <cstdio>
#include <vector>
#include <algorithm>
#include <thread>
#include <string>

using namespace std::string_literals;
using namespace std::chrono_literals;

class device {
public:
  device(const std::string & name) : name(name) {}
  
  
  void run() {
    state = false;
    std::thread([this]() {
      std::this_thread::sleep_for(std::chrono::milliseconds(rand() & 1000));
      state = true;
    }).detach();
  }
  
  void hello() {
    fprintf(stderr, "Пускаю api: %s\n", __PRETTY_FUNCTION__);
  }
  
  
public:
  std::string name;
  volatile bool state = true;
};



int main() {
  std::vector<device> v;
  
  std::generate_n(std::back_inserter(v), 10, [&] {
    return device{"device"s += std::to_string(v.size())};
  });
  
  while(true) {
    for(auto & x: v) {
      x.hello();
      if(x.state) {
        fprintf(stderr, "Готовы, пускаем: %s::run()\n", x.name.c_str());
        x.run();
      } else {
        fprintf(stderr, "Обратная связь из: %s, шепчет: Вы не готовы!\n", x.name.c_str());
      }
    }
    std::this_thread::sleep_for(100ms);
  }
}

Т.е. интегрировать в уже существующий класс метод, который будет вызваться в отдельном потоке в случае моём - не составляет труда. В твоём случае ты взял какую-то другую фигню и назвал её тем, что нужно. И когда тебе сказали обратное - ты начал меня в чём-то обвинять.

И даже если мы предположим, что твой device можно превратить во вменяемый, нужный нам объект - этого не случится. Ты засрал весь интерфейс полностью кишками своего агента - этим public.

Значит ты просто обманул и не поставил свои акторы в реальные потоки, а вроде как писал, что у тебя такая фича есть.

Дятел, ты в код посмотри. Там ID-шники нитей печатаются.

В общем, царь, ты код просил, я тебе его дал. Вякнешь еще что-нибудь пацанское, пойдешь в игнор, как обычно.

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