Многопоточный регистратор

От: Из: Общие

Я сделал многопоточный регистратор много лет назад, когда был новичком. «Сработало» (когда не зашло в тупик). Теперь, когда я немного меньше новичок, я хотел бы получить некоторую критику по поводу этой новой версии.

Регистратор порождает внутренний поток своим собственным методом writer(), целью которого является запись (в консоль и файл) информации журнала.

Есть две очереди, одна из которых записывает данные (queue_write), другой — где остальная часть программы может передавать данные (queue_log). При нажатии писатель пробуждается, он берет на себя управление мьютексом, чтобы убедиться, что никакой другой поток не обращается к нему. queue_log очередь, и он меняет местами содержимое с queue_write; затем снимает блокировку. Таким образом, потокам, использующим регистратор, в худшем случае придется ждать простой замены указателя между двумя очередями, что должно быть эффективным.

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

Пара замечаний:

  • «utils«namespace — это пространство имен утилит, которые я создал для себя и охотно отражает реализацию STL. Выбор начала класса в нижнем регистре сделан специально.
  • Регистратор выполнен с одной резьбой, производящей и потребляющей внутреннюю резьбу; он не предназначен для нескольких потребителей. У него может быть несколько производителей, но он не оптимизирован с учетом этого.
  • В Message класс, который я использую в примере использования, лишен всего нерелевантного контента (у меня есть другая часть utils заботясь о раскрашивании вывода на консоль).

logger.h

#pragma once
#include <fstream>
#include <iostream>
#include <queue>
#include <thread>
#include <mutex>

namespace utils
    {
    //TODO
    // With C++20 replace the time_points with actual readable time https://stackoverflow.com/questions/62954221/c-print-chronotime-pointchronohigh-resolution-clock-to-be-readable
    template <typename Message_type>
    class logger
        {
        private:
            std::ofstream file;
            std::queue<Message_type> queue_log;
            std::queue<Message_type> queue_write;
            std::thread thread;

            std::atomic_bool running = true;
            std::mutex queue_free;
            std::condition_variable work_available;


            void writer()
                {
                while (running)
                    {
                    if (true)
                        {
                        std::unique_lock lock{queue_free};
                        if (queue_log.empty()) { work_available.wait(lock); }

                        queue_write.swap(queue_log);
                        }

                    write_all();
                    }
                }

            void write_all()
                {
                while (!queue_write.empty())
                    {
                    Message_type& message = queue_write.front();

                    std::cout << message << std::endl;
                    file << message << std::endl;

                    queue_write.pop();
                    }
                }

        public:
            logger(const std::string& file_name = "log.txt")
                : thread(&logger::writer, this) { file.open(file_name); }

            logger(const logger& copy) = delete;            // don't copy threads please, thank you
            logger& operator=(const logger& copy) = delete;    // don't copy threads please, thank you
            logger(logger&& move) noexcept = default;    //could make those (running is already a flag for the instance being alive or not) but I'm lazy
            logger& operator=(logger&& move) = default;    //could make those (running is already a flag for the instance being alive or not) but I'm lazy

            ~logger() { if (running) { close(); } }

            //Push messages begin
            void operator<<(const Message_type& message) { push(message); }
            void operator()(const Message_type& message) { push(message); }

            template <typename ...Args>
            void emplace(Args&&... args)
                {
                Message_type message{args...};
                push(message);
                }

            void push(const Message_type& message)
                {
                if (true)
                    {
                    std::lock_guard lock(queue_free);
                    queue_log.push(message);
                    }
                work_available.notify_one();
                }
            //Push messages end

            void close()
                {
                running = false;
                work_available.notify_one();
                thread.join();

                queue_write.swap(queue_log);
                write_all();

                file.close();
                }
        };
    }

Пример использования

Message.h

#pragma once

#include <chrono>
#include <iomanip>
#include <string>
#include <string_view>

class Message
    {
    private:
        static std::string filter_last_newline(const std::string& string)
            {
            if (string.length() > 0 && string[string.length() - 1] == 'n') { return string.substr(0, string.length() - 1); }
            else { return string; }
            }

    public:
        enum class Type { log, dgn, inf, wrn, err };
        Message(Type type, const std::string& string, std::chrono::time_point<std::chrono::system_clock> time) noexcept
            : type(type), string(string), time(time)
            {}
        Message(Type type, const std::string& string) noexcept
            : type(type), string(string), time(std::chrono::system_clock::now())
            {}
        Message(const std::string& string) noexcept
            : type(Type::log), string(string), time(std::chrono::system_clock::now())
            {}

        Type type = Type::log;
        std::string string = "";
        std::chrono::time_point<std::chrono::system_clock> time;

        const char* out_type() const noexcept
            {
            switch (type)
                {
                case Message::Type::log: return "[LOG]"; // [LOG]
                case Message::Type::dgn: return "[DGN]"; // [DIAGNOSTIC]
                case Message::Type::inf: return "[INF]"; // [INFO]
                case Message::Type::wrn: return "[WRN]"; // [WARNING]
                case Message::Type::err: return "[ERR]"; // [ERROR]
                }
            }

        static Message log(const std::string& string) { return {Type::log, filter_last_newline(string), std::chrono::system_clock::now()}; }
        static Message dgn(const std::string& string) { return {Type::dgn, filter_last_newline(string), std::chrono::system_clock::now()}; }
        static Message inf(const std::string& string) { return {Type::inf, filter_last_newline(string), std::chrono::system_clock::now()}; }
        static Message wrn(const std::string& string) { return {Type::wrn, filter_last_newline(string), std::chrono::system_clock::now()}; }
        static Message err(const std::string& string) { return {Type::err, filter_last_newline(string), std::chrono::system_clock::now()}; }

        friend std::ostream& operator<<(std::ostream& os, const Message& m)
            {
            size_t beg = 0;
            size_t end = m.string.find_first_of('n', beg);
            if (end == std::string::npos) { end = m.string.length(); }

            //First line
            os << std::setw(18) << m.time.time_since_epoch().count() << ' ';

            os << m.out_type();

            os << ' ' << std::string_view(m.string).substr(beg, end - beg) << 'n';

            //Other lines
            while (true)
                {
                if (end == m.string.length()) { break; }
                else
                    {
                    beg = end + 1;
                    end = m.string.find_first_of('n', beg);
                    if (end == std::string::npos) { end = m.string.length(); }
                    }

                os << std::setw(24) << '|';
                os << ' ' << std::string_view(m.string).substr(beg, end - beg) << 'n';
                }

            return os;
            }
    };

Main.cpp

#include "utils/logger.h"
#include "Message.h"

int main()
    {
    utils::logger<Message> logger;

    logger.push(Message(Message::Type::err, "This is a verbose way to write an error."));
    logger.emplace(Message::Type::err, "This is a still quite verbose to write an error.");
    logger.push(Message::err("This is a less verbose way to write an error."));
    logger << Message::err("This is the slightly shorter way to write an error.");
    logger(Message::err("This is the shortest I could think of, although far from the "standard" way of doing things."));

    logger(Message::wrn("Multinlinenwarningnjustntonmakensurenitnworks."));

    return 0;
    }

1 ответ
1

Хотя форматирование является делом ваших предпочтений, вы не должны отходить слишком далеко от общих оснований. Подумайте о том, чтобы переместить открывающую и закрывающую скобки назад, чтобы:

int main()
{
    return 0;
}

вместо

int main()
    {
    return 0;
    }

Message_type

Сделайте стойку, CamelCase или snake_case, а не какое-нибудь диковинное детище.

            while (running)
                {
                if (true)

if (true) здесь избыточно, вы можете использовать { .. } Только.

if (queue_log.empty()) { work_available.wait(lock); }

вместо этого идите:

work_available.wait(lock, [&]() { return !running || !queue_log.empty(); });

std::condition_variable::wait() может ложно проснуться (без причины), и в этом случае ваш queue_log будет пусто, и вы позвонили write_all зря. wait call с предикатом позаботится об этом.

Message_type& message = queue_write.front();

использовать auto:

auto& message = queue_write.front();

это упростит рефакторинг кода.

            while (!queue_write.empty())
                {
                Message_type& message = queue_write.front();

                std::cout << message << std::endl;
                file << message << std::endl;

                queue_write.pop();
                }

Использовать std::vector вместо std::queue — вам не нужно выскакивать из queue_write, вы можете просто перебрать его и уничтожить все вместе. Так:

for(auto &message : queue_write) {
    std::cout << message << std::endl;
    file << message << std::endl;
}

и:

        std::vector<Message_type> queue_log;
        std::vector<Message_type> queue_write;

вместо:

        std::queue<Message_type> queue_log;
        std::queue<Message_type> queue_write;

Проходить queue_write аргументом. Не храните его как поле участника. Передача его в качестве аргумента показывает, что значение является «частным» для вызывающего и вызываемого. Когда вы помещаете его как поле-член, каждая функция-член может получить к нему доступ, поэтому кому-либо еще придется сканировать больше исходного кода, чтобы убедиться, что к нему обращаются только в двух функциях. Так:

        void write_all(const std::vector<Message_type> &queue_write) {
            for(auto &message: queue_write) {
                std::cout << message << std::endl;
                file << message << std::endl;
            }
        }

и

        void writer() {
            while (running) {
                std::vector<Message_type> queue_write;
                {
                    std::unique_lock lock{queue_free};
                    if (queue_log.empty()) { work_available.wait(lock); }

                    queue_write.swap(queue_write);
                }

                write_all(queue_write);
            }
        }

        logger(const std::string& file_name = "log.txt")
            : thread(&logger::writer, this) { file.open(file_name); }

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

        logger(const std::string& file_name = "log.txt")
            : file(file_name), thread(&logger::writer, this) {}

file переменная перед thread переменная, что означает, что она будет инициализирована первой. Таким образом вы непременно создадите поток после открытие вашего файла для записи.

        void operator<<(const Message_type& message) { push(message); }
        void operator()(const Message_type& message) { push(message); }

вместо этого напишите:

        void operator<<(Message_type message) { push(std::move(message)); }
        void operator()(Message_type message) { push(std::move(message)); }

Это может помешать копированию message.

void push(const Message_type& message)

вместо этого используйте:

        void push(Message_type message)
            {
            if (true)
                {
                std::lock_guard lock(queue_free);
                queue_log.push(std::move(message));
                }
            work_available.notify_one();
            }

Если вы беспокоитесь о Message_type будучи неподвижным, то предоставьте оба const Message_type & и Message_type && перегрузки.

    static std::string filter_last_newline(const std::string& string)
        {
        if (string.length() > 0 && string[string.length() - 1] == 'n') { return string.substr(0, string.length() - 1); }
        else { return string; }
        }

вместо этого напишите:

    static std::string filter_last_newline(std::string str) {
        if (!str.empty() && str.back() == 'n')
            str.pop_back();
        // c++17 rules about copy ellision are convoluted,
        // i'm not sure, if compiler will be forced to do std::move here
        // when returning value passed in as an argument, so just in case
        // we std::move explicitly
        return std::move(str);
    }

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

    Message(Type type, const std::string& string, std::chrono::time_point<std::chrono::system_clock> time) noexcept
        : type(type), string(string), time(time)
        {}

Как правило, аргументы в конструкторах принимают по значению, а не по ссылке const, поэтому:

    Message(Type type, std::string string, std::chrono::time_point<std::chrono::system_clock> time) noexcept
        : type(type), string(std::move(string)), time(time)
        {}

еще раз это может помешать копированию строки.

std::string string = "";

std::string уже есть конструктор по умолчанию, поэтому = "" избыточно.

        switch (type)
            {
            case Message::Type::log: return "[LOG]"; // [LOG]
            case Message::Type::dgn: return "[DGN]"; // [DIAGNOSTIC]
            case Message::Type::inf: return "[INF]"; // [INFO]
            case Message::Type::wrn: return "[WRN]"; // [WARNING]
            case Message::Type::err: return "[ERR]"; // [ERROR]
            }

в таком случае всегда добавляйте утверждение в конце функции, чтобы убедиться, что никто не доберется до нее. Этот одинарный лайнер может сэкономить вам много времени на отладку.

       while (true)
            {
            if (end == m.string.length()) { break; }

вместо этого используйте while условие:

       while (end < m.string.length()) {

  • Как правило, журналы должны отображаться в порядке получения. Используя должным образом потокобезопасную очередь (упаковывая std::queue интерфейс с std::scoped_lock) это обрабатывается автоматически и легко понять. Используя std::vector не интуитивно понятен и потенциально сбивает с толку.

    — Кейси

  • «Сделай стойку, CamelCase или snake_case, а не какое-нибудь странное детище». camelCase и snake_case больше говорят о том, что отделяет первое слово от второго. Первая заглавная буква или нет, обычно используется для отделения экземпляров от классов. CamelCase и Snake_case будут классами, camelCase и snake_case — экземплярами. Если вы не используете префиксный путь «class_xxx», «obj_xxx» etcc. Похожая вещь для позиционирования подтяжек, я использую отступы Whitesmiths, которые, по общему признанию, не очень популярны. en.wikipedia.org/wiki/Indentation_style

    — Барнак

  • Что меня больше всего волнует, так это то, чтобы интерфейс был максимально похож на stl. Тай для простой реализации filter_last_newline, я даже не знал, что есть метод pop для строк. Я обязательно внимательно рассмотрю все указанные вами оптимизации по предотвращению копирования, спасибо!

    — Барнак

Похожие записи:

  1. Реализация функции шаблона recursive_replace_copy_if на C ++
  2. Незавершенная ролевая игра
  3. Симуляция биржи, торгующая между валютами
  4. Программы C и C ++, позволяющие увеличить время доступа к файлам
  5. Простой профилировщик времени выполнения для исполняемых файлов

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *