Re[3]: ревью multi-queue processor
От: VVV Россия  
Дата: 10.09.19 10:55
Оценка:
Здравствуйте, Hydrophobia, Вы писали:


H>спасибо за ревью, но Вы посмотрели код, который прислали вместе с тестовым заданием.


Ок.

пример предоставленного кода

понял как "пример предоставленного ТОБОЙ кода".

Смотрим твой код:
1. sleep в библиотеке в

эффективный, быстрый

использоваться не должен! К тому же, у тебя нет функции для изменения этого таймаута.

2. Вместо одного потока, жрущего процессорное время, ты сделал maxThreads потоков. Если посмотреть что делает потоковая функция Process без консумеров и без данных в очередях, то это просто пустой бесконечный цикл:

    void Process()
    {

        while (running)
        {
            continue;
        }
    }


скорее всего, 100% загрузки процессора, а на мобильных устройствах — ещё и бешеный разряд батарейки.

3. Самый главный недостаток реализации: ты вызываешь link.mConsumer->Consume(link.mKey, value); из своего потока, а не из потока консумера! Это значит, что Tls (thread local storage) консумера будет этому консумеру _недоступен_. И потоконебезопасные библиотеки невозможно будет использовать в этом коде.

4. возможный AV:
        while (running)
        {
            if (!links.get(iter, link)) {
                ++iter;
                continue;
            }
//
// в этом временном интервале может случиться Unsubscribe и удаление консумера из памяти
//

            try {
                auto value = Dequeue(link.mKey);
                if (value)
                    link.mConsumer->Consume(link.mKey, value); //<<<=== тут указатель mConsumer может быть уже "протухший"
            }
            catch (...) {
            }
        }


5. не очень понял идею links
6. не существенно, но в интерфейсы обычно не вставляют виртуальный деструктор:
struct IConsumer
{
    virtual ~IConsumer() noexcept {} // <<<=== не нужен здесь.
    virtual void Consume(Key const &key, std::shared_ptr<Value> value) = 0;
};
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.