Pull to refresh
62
0
Алексей Кутумов @prograholic

Руководитель команды разработки

Send message
Так, давайте еще раз.
Не делайте засыпание на 1 мс. Это еще хуже, да — вы уменьшите время отклика, но если ваша очередь будет много времени пустой, то все равно будет тратиться время на вызовы try_pop. В лучшем случае вы не просадите производительность. Рекомендую сделать упор именно на concurrent_bounded_queue

Что касается исключений, давайте разберемся почему оно происходит. Вы создаете объект NamedPipeServer, и у вас все хорошо, но когда вы вызываете метод run, то в нем происходит исключение, и ваш метод уже не знает что с ним делать. По факту, ваша программа остается в неконсистентном состоянии (вот здесь как раз нарушается инвариант), и вы снаружи никогда не узнаете, о том что произошла ошибка.

Теперь рассмотрим другой вариант, конструктор NamedPipeServer принимает один аргумент — pipe (который должен быть создан выше по коду). Таким образом в момент создания объекта NamedPipeServer, вы уже точно знаете, что пайп открыт и метод run можно спокойно вызывать, и более того, вся эта чехарда с исключениями из метода run просто выкинется (как и вызов ConnectOrOpen. И теперь нам уже не страшно исключение, потому что мы можем написать код такого вида:
try {
  std::auto_ptr<NamedPipe> pipe(new NamedPipe(name, 1));
  NamedPipeServer server(pipe);
  server.Start();
} catch(const std::exception & e) {
  //logging, shutting down, anything else
}


Кстати, что касается исключений, то опять же, можно запускать не сами boost::thread, а использовать boost::future + boost::packaged_task, таким образом вы всегда сможете переместить исключение из потока в основной (main) поток программы.
По замечаниям: вы пишете что у вам требовалась высокая производительность, хотя по факту в коде сервера встречаются строки вида: boost::this_thread::sleep(boost::posix_time::milliseconds(100)); Что убивает ваше стремление к производительности полностью.

Для того чтобы избежать ненужного поллинга можно заменить тип очереди на concurrent_bounded_queue Тогда вы избавить от постоянной долбежки функции try_pop и будете дожидаться сообщения без поллинга.

Еще очень смущает наличие кода вида:
catch(const std::exception& e)
{
}
catch(...)
{
}

Теряется всякая информация об ошибках, что не есть хорошо.

Также есть несколько логических ошибок в коде (NamedPipeServer.cpp:12) — почему если будет выброшено исключение, то вы все равно продолжаете работу и пытаетесь обрабатывать сообщения. По моему гораздо логичней передавать в объект класса NamedPipeServer уже созданное соединение (или создавать в конструкторе), тогда бы вы не нарушали внутренних инвариантов класса (ваш объект _pipe был бы всегда создан и активен).

Ну вот вроде все.

P.S. Рекомендую прочесть книгу Энтони Вильямся concurrency in action c++. Там все эти моменты тщательно разобраны.
А почему не использовали boost.asio, там это уже все есть:
Для unix
Для win32: 1 или 2

Также, если у вас текстовый протокол, то у вас из коробки бы работали asio streams, вместо чтения записи побайтно
Если честно, я не совсем понял что вы имеете ввиду, errc — это обычное перечисление (enum). std::system_error — это класс, который содержит в себе код ошибки.

Есть несколько замечательных статей, о том как устроены system_error в C++ (на английском) от автора библиотеки boost.asio, если вам интересно, можете прочитать у него:
blog.think-async.com/2010/04/system-error-support-in-c0x-part-1.html
blog.think-async.com/2010/04/system-error-support-in-c0x-part-2.html
blog.think-async.com/2010/04/system-error-support-in-c0x-part-3.html
blog.think-async.com/2010/04/system-error-support-in-c0x-part-4.html
blog.think-async.com/2010/04/system-error-support-in-c0x-part-5.html
void * — это плохой вариант, сразу добавляет кучу проблем, что будет, если я скажу delete для него?
К тому же exception_ptr инкапсулирует внутри себя всю работу с исключениями, и предоставляет универсальный интерфейс по передаче исключений — вполне себе ООП-style (в отличие от того же void *)
Часть информации брал с cppreference.com, но там не полное описание, пришлось в стандарт заглядывать
Кстати, я написал следующий тест
void check4()
{
	typedef function<int (void)> int_function_t;

	int_function_t f1;

	Bar bar;

	f1 = std::bind(&Bar::callMe, bar, 10);
	cout << "calling binded member function with signature int (void): " <<  f1() << endl;
}

И он успешно выполнился. Здесь произошла следующая последовательность действий:
Сначала вызвался конструктор без аргументов
Затем создался временный объект с помощью конструктора free function (в качестве параметра был передан созданный с помощью std::bind объект)
Затем вызвался оператор присваивания.

Если пометить конструктор free function как explicit, то мы увидим следующую ошибку:
error: no match for ‘operator=’ in ‘f1 = std::bind(_Func&&, _BoundArgs&& ...) [with _Func = int (Bar::*)(int); _BoundArgs = {Bar&, int}; typename std::_Bind_helper<std::__is_socketlike<_Func>::value, _Func, _BoundArgs ...>::type = std::_Bind<std::_Mem_fn<int (Bar::*)(int)>(Bar, int)>]((* & bar), (* &10))’


Так что, в принципе все должно работать
Я вроде и не призывал курить исходный код stl, я лишь выразил свое сожаление по поводу их coding style guide. Кроме этого я добавил, что мне самому интересно ковыряться в исходниках, но это тоже на призыв никак не тянет.
С присваиванием накладка вышла, я забыл написать реализацию для произвольного типа, в любом случае, она будет тривиальна. Вам спасибо за то что нашли ошибку!
Полностью поддерживает, можно создать функтор с помощью std::bind, или написать лямбда функцию. Все это полностью покрывается шаблонным конструктором, в котором создается объект класса free_function_holder.

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

Кстати, если открыть исходный код того же std::function, то можно увидеть кучу doxygen комментариев, а они то компилятору уж точно бесполезны.
Обезьян с гранатой здесь рассматривать неуместно, не дай бог, прочитают про ручное управление памятью, или возможность перегрузки глобальных операторов new/delete. Мне, например, всегда было интересно, как реализованы разные вещи в программировании. Другое дело, что не всегда хватает уровня знаний, например, я бы эту статью года 3-4 назад просто бы не понял.

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

К тому же в полноценных реализациях много вспомогательного кода: debug assertions, обработка пограничных ситуаций и пр. Мне кажется, что нужно рассказывать именно общую идею (плюс несколько ключевых особенностей), и тогда станет проще понимать уже полноценные реализации
Для программистов C++, которые хотят знать, как устроены некоторые части стандартной библиотеки.

p.s. Вообще здесь недавно был опрос, в котором большинство высказалось за более сложные «технические» статьи. На мой взгляд — это как раз такая статья, так как затрагивает довольно тонкие моменты C++
Log4cpp выбрали несколько лет назад (на самом деле, просто первая попалась), с тех пор используем во многих проектах. Не могу сказать что это отличная библиотека логгирования, но нам пока хватает.
Гугл по запросу «C++ logging library» дает ссылку на stackoverflow, в принципе там есть полезная информация, может вам пригодится
Я бы поправил пример с мьютексами, потому-что в приведенном примере мьютексы не выполняют свою роль — они ничего не защищают.
Здесь важно понимать, что при таком подходе мы добавляем еще один уровень абстракции — прячем вызов функции в объект, это вносит свои накладные расходы.

Как я уже писал, можно произвести замеры времени и оценить падение производительности для различных реализаций методов (в том числе, и для inline методов).
Да, конечно же mutable, поправил. Спасибо!
Спасибо, я тоже надеюсь, что добавят)
Хотя меня больше интересует вот эта заявка. Без нее ценность этого плагина существенно снижается
Первый раз начали заниматься review мы где-то чуть больше года назад. Сначала смотрели изменения с помощью обычного diff в Tortoise SVN, затем поставили crucible + fisheye для проведения review. Но этот инструмент оказался неэффективным (это не значит, что он плохой), потому что мы не могли применить результаты review. Отчасти это было связано с тем, что исправления по результатам review не делались, либо их вносить было поздно (продукт уже проходил финальное тестирование и вносить изменения в код уже было нельзя). Так мы мучились в течении 2-3 месяцев, потом забросили это дело.

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

Сейчас мы решили внедрить code review как необходимый элемент итерации в нашем Scrum, и также прикрутить к CI серверу проверку на пройденный review перед сборкой дистрибутива. Так что я думаю, что у нас должно получиться во второй раз.

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

Information

Rating
Does not participate
Location
Новосибирск, Новосибирская обл., Россия
Date of birth
Registered
Activity