Асинхронный Входной Файл


Недавно я начал работать на моем собственном асинхронного файлового ввода "библиотека" на C++. Я сделал это сегодня и я решил поставить его на проверку кода, чтобы увидеть, насколько хорошо я на самом деле сделал, и где я еще могу улучшить код.

В log++ #include это просто какая-то регистрация заголовка я создал для записи цветной вывод в консоли.

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

AsyncTask.ч

#pragma once

#include <iostream>
#include <fstream>
#include <thread>
#include <string>
#include <sstream>
#include <mutex>
#include <functional>
#include <condition_variable>
#include "logpp/log++.h"



class AsyncTask
{
public:


    AsyncTask();

    template<typename Callable, typename ...Args>
    explicit AsyncTask(Callable task, Args&&... args);
    ~AsyncTask();

    /*Manually starts the task if it is not already running*/
    template<typename Callable, typename ...Args>
    void start(Callable func, Args&&... args);

    /*Interrupts the task until resume() is called*/
    void pause();

    /*Unpauses the task*/
    void resume();

    /*Blocks calling thread until the task is done*/
    void wait();

    bool is_complete();

    /*Stops the task and resets all it's data*/
    virtual void reset();

private:

    std::thread m_exec_thread;

    bool m_running;
    bool m_paused;

    std::condition_variable m_pause_cv;
    std::mutex m_pause_mutex;

protected:
    void handle_pause_update();
    void toggle_running();
};


template<typename Callable, typename... Args>
AsyncTask::AsyncTask(Callable task, Args&&... args) : m_running(false), m_paused(false)
{
    start(task, std::forward<Args>(args)...);
}


template<typename Callable, typename ...Args>
void AsyncTask::start(Callable func, Args&&... args)
{
    /*Only start the task if it is not already running*/
    if (!m_running)
    {
        m_exec_thread = std::thread(func, std::forward<Args>(args)...);
        m_running = true;
    }
}


class AsyncReadTask : public AsyncTask
{
public:
    AsyncReadTask();
    explicit AsyncReadTask(std::string const& filename);

    /*Obtain data that was read from the file. Will reset the task*/
    std::stringstream get_data();

    void start(std::string const& filename);

    virtual void reset() override;

private:
    std::stringstream m_data;
    std::string m_file;

    void read(std::string const& file_name);
};

AsyncTask.cpp

#include "AsyncTask.h"


#pragma region AsyncTaskImpl

AsyncTask::AsyncTask()
{

}

void AsyncTask::handle_pause_update()
{
    while (m_paused) //check if we have to pause
    {
        std::unique_lock<std::mutex> lk { m_pause_mutex };
        m_pause_cv.wait(lk);
        lk.unlock();
    }
}


void AsyncTask::pause()
{
    if (m_paused) //don't double pause
        return;
    std::lock_guard<std::mutex> lk { m_pause_mutex };
    m_paused = true;
}


void AsyncTask::resume()
{
    std::lock_guard<std::mutex> lk { m_pause_mutex };
    m_paused = false;
    m_pause_cv.notify_one();
}


void AsyncTask::wait()
{
    while (m_running)
    {
        //wait
    }
}


bool AsyncTask::is_complete()
{
    return !m_running;
}


void AsyncTask::reset()
{
    wait();
    m_running = false;

}

void AsyncTask::toggle_running()
{
    m_running = !m_running;
}

AsyncTask::~AsyncTask()
{
    m_exec_thread.join();
}

#pragma endregion AsyncTaskImpl

#pragma region AsyncReadTaskImpl


std::stringstream AsyncReadTask::get_data()
{
    wait();

    auto data = std::move(m_data);
    reset();
    return std::move(data);
}


void AsyncReadTask::read(std::string const& file_name)
{
    /*---Open File---*/

    std::ifstream in(file_name);


    logpp::Console::log_assert(in.good() && in.is_open(), "Could not open file: " + file_name);


    /*---Read data---*/

    std::string line;
    while (std::getline(in, line))
    {
        line += "\n"; //getline removes the newline character, but we want to give the stringstream the exact same data as the file

        m_data << line;

        handle_pause_update();
    }


    /*---Cleanup---*/

    in.close();
    toggle_running();
}

AsyncReadTask::AsyncReadTask()
{

}

AsyncReadTask::AsyncReadTask(std::string const& filename) : AsyncTask(&AsyncReadTask::read, this, filename), m_file(filename)
{

}

void AsyncReadTask::start(std::string const& file_name)
{
    AsyncTask::start(&AsyncReadTask::read, this, file_name);
}

void AsyncReadTask::reset() 
{
    AsyncTask::reset();

    m_data.clear();
    m_file = "";
}

#pragma endregion AsyncReadTaskImpl

Исходный код можно найти на Гитхабе тоже.



296
6
задан 31 марта 2018 в 08:03 Источник Поделиться
Комментарии
2 ответа

Просто несколько вещей, которые еще не сказал:


  • Вам не придется #include все в одном месте. Например, библиотека журналов не должны быть включены в основной заголовок. Рассмотреть возможность включения заголовочных файлов только там, где они необходимы - это позволит ускорить компиляцию большое , когда ваш проект растет.

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

  • Ваши строки потока могут не содержать те же данные, что файл - разных платформах используют разные разделители строки. Если вам нужен ваш stringstream, чтобы содержать ровно те же данные, что файл, попробуйте загрузить файл в двоичном виде.

  • при указании override вы не должны объявлять virtual.

  • вы можете использовать = default чтобы задать конструктор по умолчанию.

  • ваш AsyncTask() конструктор не инициализирует все члены, это должно быть обязательно исправлено.

  • в handle_pause_update есть петля - я не думаю, что это необходимо, просто if должен делать эту работу тоже.

  • некоторые функции могут быть константными (как правило, добытчики, например is_complete)

  • как указывалось ранее, никогда не код напряженного ожидания только в случае крайней необходимости. Почему не вы просто позвоните m_exec_thread.join() в wait()?

  • если вы никогда не начинайте свою задачу, то AsyncTask деструктор вызывает join в потоке, который никогда не был создан и что бросает исключение.

1
ответ дан 5 апреля 2018 в 05:04 Источник Поделиться

Так AsyncTask имеет виртуальную функцию, и используется как базовый класс, деструктор ~AsyncTask должны быть виртуальными.

m_running и m_paused должно быть std::atomic<bool>. Без этого, компилятор может предположить, что эти значения не изменятся, если он не видит изменений. Это может вызвать цикл в AsyncTask::wait чтобы никогда не завершить, так как оптимизатор может удалить повторные проверки, по всей видимости, неизменные переменные и уйти в бесконечный цикл. Кроме того, используя std::atomic позволит вам избавиться от использования замка, прежде чем получить доступ m_running или m_paused.

Кроме того, AsyncTask::wait не "дружественные", так как это очень активный заняты петли. Это должно быть что-то вроде sleep(1) чтобы избежать постоянно горит ядра процессора, в результате чего избыточное тепло и батареи, и, возможно, система нерасторопность. По крайней мере, использовать _mm_pause() внутренние позволить ЦП знаю, что в спин-петля.

В m_running = false; выражение в AsyncTask::reset эффективно ничего не делает, с момента вызова wait не вернется, пока эта переменная имеет значение false (и может причинить вред, если другой поток находится m_running обратно true после wait намерен вернуться).

В AsyncReadTask::get_dataвы не должны использовать return std::move(data), поскольку это подавляет РВО. Просто использовать return data;.

2
ответ дан 4 апреля 2018 в 08:04 Источник Поделиться