Здравствуйте, so5team, Вы писали:

S>Ну и общее впечатление такое, что писал человек, у которого бэкграунда в Java или в C# больше, чем в C++.

Общее впечатление, что писал человек без понимания алго-сложности, типично для C++ бекграунда. Я даже не удивлён, что ты этого не заметил, а только докопался до копирования.
https://github.com/w5346c/hash_server/blob/master/lib/request_responder_impl.cpp

Здравствуйте, Тёмчик, Вы писали:

Тё>Я даже не удивлён, что ты этого не заметил, а только докопался до копирования.

Я отвечал на комментарий, в котором говорилось про копирование. И показал, что копирование там не только в том месте, на которое указал ArtDenis.

Тё>Общее впечатление, что писал человек без понимания алго-сложности, типично для C++ бекграунда.
Тё>https://github.com/w5346c/hash_server/blob/master/lib/request_responder_impl.cpp

Чтобы говорить про алго-сложность нужно знать точную постановку задачи. Я ее не знаю.

Судя по коду, автору нужно было разбить входной поток на отдельные строки и подсчитать хэш-код для каждой строки.
Если это так, то я бы сделал цикл разбора каждой прочитанной порции несколько иначе, используя find для '\n'. Что-то вроде (это набросок, который никак не проверялся и написан без оглядки на реальный API):

std::size_t current_pos = 0u;
while( auto nl_pos = request.find(current_pos, '\n'); nl_pos != std::string::npos ) {
  ... // Обработка очередного фрагмента строки.
  current_pos = nl_pos + 1u;
}
if( current_pos < request.size() ) {
  ... // Обработка остатка во входном буфере.
}

Но, по сути, это был бы тот же самый цикл от начала в конец входного буфера, как и у автора. Только с надеждой на то, что он будет несколько эффективнее, т.к. std::string::find может оптимизироваться до специфических инструкций CPU по обработке байтовых последовательностей. Хотя еще хрен знает во что современные оптимизирующие компиляторы развернут цикл, написанный в стиле:

for(std::size_t i = 0; i < len; ++i) {
  if(s[i] != '\n') continue;
  ...
}

Еще меня лично сильно смущает то, что автор накапливает "хвост" из входного буфера в отдельном std::string, который может расти неограниченно долго, в зависимости от входных данных. Следовало бы взять такую реализацию подсчета хэша, которой фрагменты строки можно было бы скармливать частями перед получением итогового результата. Но автор тестового задания явно такими вещами не заморачивался судя по коду.

---

UPD. Если нужно только хэши для строк считать и все, то достаточно всего одного прохода по входному буферу из начала в конец с посимвольным скармливанием очередного символа вычислителю хэша (если очередной символ не '\n') и специальной обработкой символов '\n'. Тут даже не нужно отдельных подстрок выделять в каком-либо виде.

---

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

Здравствуйте, so5team, Вы писали:

S>Я отвечал на комментарий, в котором говорилось про копирование. И показал, что копирование там не только в том месте, на которое указал ArtDenis.

Ну я особо внимательно не смотрел. Спросил про первое попавшееся на глаза место в коде. Если глянуть больше, то да, видно что человек писал сразу как пришло в голову. Из-за большого количества аллокаций/перелокаций, копирований и деалокаций в строках эффективностью тут и не пахнет.
Хотя, требования к эффективности надо обговаривать изначально в ТЗ. Возможно если бы они были, человек написал бы по-другому.

Здравствуйте, reversecode, Вы писали:

R>так что я улыбнулся когда очередной чел опять помянул ее
Сколько пролилось интересного Не взяли? Меня, кстати, в свое время не взяли, сейчас у меня пара их сотрудников работает. Отличные ребята (один как раз вроде меня и отшил на собеседовании по плюсам), только оверинжинирят слегка.

лол, я туда даже не могу подаваться, потому что не тот локейшин))
сказал же, видел их говнокод в реверс инжинириге

Здравствуйте, so5team, Вы писали:

S>Чтобы говорить про алго-сложность нужно знать точную постановку задачи. Я ее не знаю.


Вот постановка.

Клиентское приложение устанавливает TCP-соединение и передает
строки, разделенные символом перевода строки “\n”. Сервер должен считать их хеш-суммы (тип
суммы — на выбор кандидата) и отправлять их обратно в HEX-виде, также завершая каждую сумму
символом перевода строки.
Клиентские запросы должны обрабатываться параллельно, в случае достаточного количества
параллельных соединений должны быть загружены все ядра CPU. Сервер должен работать
эффективно — не потреблять лишней памяти и отправлять хэш-суммы по мере их готовности. Входные
строки могут быть неограниченной длины.
Для реализации сетевой части сервера можно использовать любую удобную вам библиотеку из числа
стандартных пакетов репозитория Ubuntu 16. Сервер также должен собираться и работать на Ubuntu
16.
На модули приложения должны быть написаны unit-тесты.

Да, я бOльшую часть времени убил на разбирательство с boost asio и осознанием как это все покрыть юнит-тестами.
Первая версия получилась неэффективной, но покрытой.
Вторая более эффективной, но не покрытой — https://github.com/w5346c/hash_server2
Потом получил устраивающий меня оффер из другой компании и продолжать не стал.

Здравствуйте, dwebster, Вы писали:

D>Входные строки могут быть неограниченной длины.

Вы специально это требование проигнорировали в обоих реализациях?

D>Для реализации сетевой части сервера можно использовать любую удобную вам библиотеку из числа
D>стандартных пакетов репозитория Ubuntu 16. Сервер также должен собираться и работать на Ubuntu
D>16.

Подразумевается Ubuntu 16.04?

D>Вторая более эффективной, но не покрытой — https://github.com/w5346c/hash_server2

Это как-то больше на C++ похоже. Хотя и к ней есть вопросы, но тут нужно в особенности поведения Asio погружаться.

А вот что забавляет, так это количество C++ников, которые пишут в таком духе:

    try
    {
        ...
        auto threadCount = std::thread::hardware_concurrency();
        std::vector<std::thread> threads;

        for(int i = 0; i < threadCount; i++)
            threads.emplace_back([&context]{ context.run(); });

        for(auto& thread : threads)
            thread.join();
    }
    catch (std::exception& e)
    {
        std::cout << "Error: " << e.what() << std::endl;
    }

и не задумываются о exception-safety подобного кода. Да и вообще непонятно, зачем вам здесь ожидать завершения порожденных вами тредов.

Здравствуйте, so5team, Вы писали:

D>>Входные строки могут быть неограниченной длины.

S>Вы специально это требование проигнорировали в обоих реализациях?

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

Здравствуйте, dwebster, Вы писали:

D>Да, я бOльшую часть времени убил на разбирательство с boost asio и осознанием как это все покрыть юнит-тестами.
D>Первая версия получилась неэффективной, но покрытой.
D>Вторая более эффективной, но не покрытой — https://github.com/w5346c/hash_server2
D>Потом получил устраивающий меня оффер из другой компании и продолжать не стал.

Вот непонятный фрагмент в OnDataReceived:

    std::iostream stream(&m_writeBuf);
    stream << std::hex << hash << "\n";

    if (!m_writeInProgress)
    {
        WriteAsync();
    }

Похоже, тут если `m_writeInProgress == true`, то подсчитанный hash просто не записывается. Я asio лет 5 уже не использовал, там вроде бы в случае, если идет асинхронная запись, принято добавлять данные в очередь исходящих, чтобы потом проверять ее в `OnWriteCompleted` и записывать оставшееся.

Ну и по традиции русского форума, добавлю что код написан в стиле C++98, что странно для 2021 года. Еще видно, что автор не имел опыта с asio, в чем он честно выше и признался.

Еще мне всегда было непонятно, как можно написать юнит тесты для сетевого кода. Это же взаимодействие с внешним миром. Все время получалось только end-to-end: со стороны клиента проверять ожидаемые ответы на определенные запросы.

Здравствуйте, dwebster, Вы писали:

S>>Вы специально это требование проигнорировали в обоих реализациях?

D>Строки такой длины, чтоб не влезали в память.. да ну, бред какой-то )

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

Здравствуйте, so5team, Вы писали:

S>Вы-то можете рассказать какое было хреновое тестовое и как вам вообще процесс общения с компанией не понравился.

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

Уверен, в NetworkOptix отлично работается и спецы там высококлассные. То что не добил тестовое — мое решение.
Я в это время параллельно проходил этапы собеседований в несколько компаний, и тратить много времени на одну не было ни возможности, ни желания.
Не так уж сильно хотелось работать именно там, чай не Гугл.

Здравствуйте, dwebster, Вы писали:

D>Клиентское приложение устанавливает TCP-соединение и передает
D>строки, разделенные символом перевода строки “\n”. Сервер должен считать их хеш-суммы (тип
D>суммы — на выбор кандидата) и отправлять их обратно в HEX-виде, также завершая каждую сумму
D>символом перевода строки.
D>Клиентские запросы должны обрабатываться параллельно, в случае достаточного количества
D>параллельных соединений должны быть загружены все ядра CPU. Сервер должен работать
D>эффективно — не потреблять лишней памяти и отправлять хэш-суммы по мере их готовности. Входные
D>строки могут быть неограниченной длины.

Смущает момент с "должны быть загружены все ядра CPU". Это самое сложное здесь
Вот смотри: вначале строки, хэш=0. Каждый последующий символ, хэш считает на основе предыдущего значения хэша, кумулятивно (т.е. алгоритм занимает фиксированно 32bit или сколько там у тебя хэш). По прибытию перевода строки, форматируем в строковое представление и асинхронно кидаем на отправку, а кумулятивный хэш обнуляем.
Надеюсь, что буст асио из коробки использует пул потоков (чтоб загружать ядра), иначе борьба с синхронизацией, гонками, дедлоками и т.п. прелестями приаедет тебя в целый новый мир боли и мазохизма.

Здравствуйте, so5team, Вы писали:

S>А вот что забавляет, так это количество C++ников, которые пишут в таком духе:
S>

S>    try
S>    {
S>        ...
S>        auto threadCount = std::thread::hardware_concurrency();
S>        std::vector<std::thread> threads;

S>        for(int i = 0; i < threadCount; i++)
S>            threads.emplace_back([&context]{ context.run(); });

S>        for(auto& thread : threads)
S>            thread.join();
S>    }
S>    catch (std::exception& e)
S>    {
S>        std::cout << "Error: " << e.what() << std::endl;
S>    }
S>

S>и не задумываются о exception-safety подобного кода. Да и вообще непонятно, зачем вам здесь ожидать завершения порожденных вами тредов.

вы уже отправили пуш реквест в репу asio examples по поводу фиксов таких ляпов ?

то всякие школьники написавшие asio, налепят тупых примеров
а другие потом учатся на них

тупое задание, как я и ожидал
но считаю что вы его +-выполнили, остальное фигня
а всякие ихние придирки это найти повод отказать

т.е. вердикт, компания Network Optix крутит вакансии just for fun

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

Здравствуйте, reversecode, Вы писали:

R>вы уже отправили пуш реквест в репу asio examples по поводу фиксов таких ляпов ?

Нет. И придерживаюсь мнения, что в примерах не нужно отвлекаться на такие тонкости.

R>то всякие школьники написавшие asio, налепят тупых примеров
R>а другие потом учатся на них

Скажите, пожалуйста, а ваш апломб подтвержден какими-то публично-доступными результатами, которые могут использоваться другими людьми?

Здравствуйте, reversecode, Вы писали:

R>но считаю что вы его +-выполнили, остальное фигня

Ахринеть, дайте два!

В условии задачи было три принципиальных момента:

1. Параллельная обработка запросов. Тут условная галочка, хотя есть вопрос к формулировке "Клиентские запросы должны обрабатываться параллельно, в случае достаточного количества параллельных соединений должны быть загружены все ядра CPU." Еще можно интерпретировать и так, что задействовать очередное ядро CPU следует после того, как количество параллельных соединений превышает некий порог. Т.е., до 1000 соединений на одном ядре, до 2000 -- на двух и т.д.

2. Отсылка хэшей по мере их готовности (т.е. запись одновременно с чтением). Тут, опять же, условная галочка для варианта #2, но минус для варианта #1. Кроме того, для варианта #2 нужно посмотреть, почему в коде не используются strand-ы для операций чтения/записи, и насколько безопасно писать что-то в asio::streambuf пока этот самый streambuf задействуется в незавершившейся еще операции записи (хотя, подозреваю, под Linux-ом это безопасно).

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

И это теперь называется +- выполнили?

Здравствуйте, dwebster, Вы писали:

D>Вообще-то в статье я написал ровно наоборот — "В целом, общение с представителями компании оставило самые приятные впечатления".

По факту вы оставили следующее впечатление о компании:

1. Они дают тестовое задание. Для многих, что видно даже по этому обсуждению, сие уже есть фу-фу-фу.

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

D>И внес компанию в тройку компаний, собеседоваться в которые мне понравилось больше всего за карьеру.

Из этого не следует, что эта компания хорошая. Следует лишь то, что почти все другие еще хуже.

Здравствуйте, so5team, Вы писали:

S>Это условие вообще самым тупым образом проигнорировано, а автор самым тупым образом прикидывается ветошью и пытается не отсвечивать.

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

Здравствуйте, so5team, Вы писали:

S>Ахринеть, дайте два!

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

По теме, разбить по пунктам:

алгоритмы 3
оформление 5
многопоточка 4.

Здравствуйте, Тёмчик, Вы писали:

Тё>Помнится, кое-кто хранит обьекты в хеш мапе, ибо в подсчет ссылок нишмогла.

Тёмчик, ты опять ничего не понял, но щеки надул и авторитетно пукнул в лужу.

Тё>алгоритмы 3
Тё>оформление 5
Тё>многопоточка 4.

На C++ с Asio в многопотоке давно программировал, экспертный ты наш?

PS. А что тема с алго-сложностью

Автор: Тёмчик
Дата: 12.02.22

так тихо стухла-то? Я требую продолжения банкета!