2
\$\begingroup\$

I have written a simple logger, AsyncFile, which asynchronously writes data to a file. Since data is going to be written very often (E.g. 10 writes per 30 milliseconds from different threads, my system has a 30ms sync time), I would like to know if:

1. The design makes sense att all? Am I missing something obvious?
2. If the logger is fairly optimal, e.g. that no unneccessary copying takes place.

Basically, my idea is to write data to a shared buffer(m_queue), then copy that data to a tempBuffer from which I then write to a file.

The method write can be called from multiple threads.

I have tested this logger and it seems to work fine, but like all problems involving shared resources, it is hard to know if it is well-designed until you have tried it for a good amount of time.

AsyncFile.h:

#pragma once

#include <cstdint>
#include <cstddef>
#include <array>
#include <mutex>
#include <condition_variable>

class AsyncFile
{
public:

   AsyncFile();

   void write(std::array<char,40>& buffer);

   void done();

private:

    std::array<char, 160> m_queue;

    std::array<char, 160> tempBuffer;

    std::mutex queue_mutex;

    std::condition_variable cv;

    size_t counter = 0;

    bool readyToCopy = false;

    bool finished = false;

    void logger_thread();

}; // class AsyncFile

AsyncFile.cpp:

#include <thread>
#include <fstream>
#include <string.h>
#include <iostream>
#include <chrono>

void AsyncFile::logger_thread()
{
    FILE* file = fopen("test_file.bin", "wb");

    while(!finished) // Pass "finished" as a parameter by ref? Protect by a mutex?
    {
        std::unique_lock<std::mutex> lk(queue_mutex);
        cv.wait(lk, [&]{return readyToCopy;});

        std::copy(m_queue.begin(), m_queue.end(), tempBuffer.begin());

        readyToCopy = false;
        lk.unlock();

        // Now write to file from the temporary buffer:
        fwrite(&tempBuffer, sizeof(char), sizeof(tempBuffer), file);
    }
    fclose(file);
}

AsyncFile::AsyncFile()
{
      std::thread m_worker(&AsyncFile::logger_thread, this);
      m_worker.detach(); // let the worker thread live on its own.
}

void AsyncFile::done()
{
    finished = true;
}

void AsyncFile::write(std::array<char, 40>& buffer) // 40 should probably be a
                                                    // configurable parameter.
{
   std::lock_guard<std::mutex> guard(queue_mutex);
   std::copy(buffer.begin(), buffer.end(), m_queue.begin() + counter);
   if (counter == 120) // fill the queue, 40 char each time, until we have 160 chars in total. Then
                       // notify the writing thread we are ready.
   {
      counter = 0;
      readyToCopy = true;
      cv.notify_one();
   }
   else
   {
      counter += 40;
   }
}
\$\endgroup\$
2
\$\begingroup\$

To make a fast logger you need to

  • minimise the use of mutexes.
  • detach the logging from the writing.
  • minimise copying
  • pre-alloc the buffers
    • no allocs during logging.
  • make a ring buffer
    • fixed number of logging
    • decide what should happen if its full
    • slightly slower using a dequeue with minimises allocs
  • use atomic indexes to the (ring) buffer
    • if your system if fully loaded, you have to log often use the cv or wait until the logger thread gets scheduled by the OS.
    • use the cv.notice_one where the index is updated if the front is sufficient in front of the back end.

So a few of the things that are issues

  • The logger_thread blocks further logging while it writes.
  • The write blocks other threads logging

Running into a mutex that is taken causes a task switch, typically taking from 1000ns to 22000ns on a x86, nearer the lower if its started again on the same hardware thread.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for your feedback! But I thought the writing to file is done asynchronously, since I unlock the lock before I write? The lock only protects the copying to tempBuffer? \$\endgroup\$ – Wballer3 May 20 '20 at 18:36
  • \$\begingroup\$ @Wballer3 you are right you do unlock before write. \$\endgroup\$ – Surt May 20 '20 at 20:14
1
\$\begingroup\$

There are some issues with this solution:

  1. You are saying that this class will be used from many threads under a heavy load. In this case a lock in AsyncFile::write is not going to scale well. Only one thread can write logs at a time, depending on the load this kind of thing can bring the whole system down. Though of course with your load pattern it might be just fine.
  2. I agree that the constants should be configurable
  3. fopen is most likely a buffered api. So, if your app crashes, the logs will be corrupted.
  4. There is no overflow mechanism. What would happen if the clients write faster than the writing thread can consume?
  5. If no one writes anything, how would your thread finish? You should check finished in the condition variable.
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for you feedback, it is really helpful! I just have a couple of questions. Regarding your 4th point, is there any strategy you would recommend, regarding the overflow? I hardly see any other strategy than just discarding the data if the buffer is full? My second question is about your point on multiple threads writing to the same buffer. Would it be more efficient to have some kind of atomic offset which is incremented after each write, instead of protecting the whole "m_queue" with a mutex? I am thinking of locking it ONLY when it is full, in order to copy m_queue to tempBuffer. \$\endgroup\$ – Wballer3 May 22 '20 at 19:14
  • \$\begingroup\$ I case of overflow, you basically have only two choices, either wait or discard. For minimizing locking there are more choices than I can describe in a comment: you can use a buffer for each thread and then ether join them inside the logging thread or use some kind of write-gather api. You can use a concurrent container, with finer-grained locking or lock-free instead of your queue and so on. \$\endgroup\$ – vvotan May 23 '20 at 20:33
1
\$\begingroup\$

An alternative to using (ring)buffers, mutexes and condition variables is to use a datagram UNIX socket pair. Writes to a datagram socket are atomic, so multiple threads can safely write to it without the messages getting mixed up. Also, you can choose whether or not to make the socket non-blocking. On Windows you might have to use something else, like a UDP socket.

The drawbacks are that it is not as portable (sockets are not part of the C++ standard library), and that each write to a socket will be a system call, so it has a higher overhead than just adding something to a ringbuffer. On the other hand, if you write so often that you have lock contention then the difference is likely minimal.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.