0

The Question What's a good way to increment a counter and signal once that counter reaches a given value (i.e., signaling a function waiting on blocks until full, below)? It's a lot like asking for a semaphore. The involved processes are communicating via shared memory (/dev/shm), and I'm currently trying to avoid using a library (like Boost).

Initial Solution

  • Declare a struct that contains a SignalingIncrementingCounter. This struct is allocated in shared memory, and a single process sets up the shared memory with this struct before the other processes begin. The SignalingIncrementingCounter contains the following three fields:

    1. A plain old int to represent the counter's value.
      Note: Due to the MESI caching protocol, we are guaranteed that if one cpu core modifies the value, that the updated value will be reflected in other caches once the value is read from those other caches.

    2. A pthread mutex to guard the reading and incrementing of the integer counter

    3. A pthread condition variable to signal when the integer has reached a desirable value

Other Solutions

  • Instead of using an int, I also tried using std::atomic<int>. I've tried just defining this field as a member of the SignalingIncrementingCounter class, and I've also tried allocating it into the struct at run time with placement new. It seems that neither worked better than the int.

The following should work.

The Implementation

I include most of the code, but I leave out parts of it for the sake of brevity.

signaling_incrementing_counter.h

#include <atomic>

struct SignalingIncrementingCounter {
public:
    void init(const int upper_limit_);
    void reset_to_empty();
    void increment(); // only valid when counting up
    void block_until_full(const char * comment = {""});
private:
    int upper_limit;
    volatile int value;
    pthread_mutex_t mutex;
    pthread_cond_t cv;

};

signaling_incrementing_counter.cpp

#include <pthread.h>
#include <stdexcept>

#include "signaling_incrementing_counter.h"


void SignalingIncrementingCounter::init(const int upper_limit_) {

    upper_limit = upper_limit_;
    {
        pthread_mutexattr_t attr;
        pthread_mutexattr_init(&attr);
        int retval = pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);
        if (retval) {
            throw std::runtime_error("Error while setting sharedp field for mutex");
        }
        pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);

        pthread_mutex_init(&mutex, &attr);
        pthread_mutexattr_destroy(&attr);
    }

    {
        pthread_condattr_t attr;
        pthread_condattr_init(&attr);
        pthread_condattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);

        pthread_cond_init(&cv, &attr);
        pthread_condattr_destroy(&attr);
    }

    value = 0;
}

void SignalingIncrementingCounter::reset_to_empty() {
    pthread_mutex_lock(&mutex);
    value = 0;
    // No need to signal, because in my use-case, there is no function that unblocks when the value changes to 0
    pthread_mutex_unlock(&mutex);
}

void SignalingIncrementingCounter::increment() {
    pthread_mutex_lock(&mutex);
    fprintf(stderr, "incrementing\n");
    ++value;
    if (value >= upper_limit) {
        pthread_cond_broadcast(&cv);
    }
    pthread_mutex_unlock(&mutex);
}

void SignalingIncrementingCounter::block_until_full(const char * comment) {
    struct timespec max_wait = {0, 0};
    pthread_mutex_lock(&mutex);
    while (value < upper_limit) {
        int val = value;
        printf("blocking until full, value is %i, for %s\n", val, comment);
        clock_gettime(CLOCK_REALTIME, &max_wait);
        max_wait.tv_sec += 5; // wait 5 seconds
        const int timed_wait_rv = pthread_cond_timedwait(&cv, &mutex, &max_wait);
        if (timed_wait_rv)
        {
            switch(timed_wait_rv) {
            case ETIMEDOUT:
                break;
            default:
                throw std::runtime_error("Unexpected error encountered.  Investigate.");
            }
        }
    }
    pthread_mutex_unlock(&mutex);
}
interestedparty333
  • 2,386
  • 1
  • 21
  • 35
  • 1
    Nitpick: const qualification plays no role on a function parameter as the declaration is not a definition. – curiousguy Sep 08 '19 at 20:37
  • It's c++, you have [condition variable](https://en.cppreference.com/w/cpp/thread/condition_variable). Don't use pthreads. – KamilCuk Sep 08 '19 at 23:46
  • Are pthreads and c++ libraries safe to use across processes? I've always understood any synchronization between threads of different processes to be unsafe using pthreads and to use a unix semaphore to implement locking between processes. – selbie Sep 09 '19 at 00:11
  • 2
    @selbie, pthreads primitives need to be configured properly for use across multiple processes, but this is one of their intended use cases, and they work fine for it when used correctly. The OP is correctly setting the "pshared" attribute of both the mutex and the CV, which is what is required. – John Bollinger Sep 09 '19 at 10:51
  • Here is one way: https://stackoverflow.com/questions/48614784/allocating-a-atomic-on-shared-memory/48620618#48620618 – Erik Alapää Sep 10 '19 at 08:26

1 Answers1

-1

Using either an int or std::atomic works.

One of the great things about the std::atomic interface is that it plays quite nicely with the int "interface". So, the code is almost exactly the same. One can switch between each implementation below by adding a #define USE_INT_IN_SHARED_MEMORY_FOR_SIGNALING_COUNTER true.

I'm not so sure about statically creating the std::atomic in shared memory, so I use placement new to allocate it. My guess is that relying on the static allocation would work, but it may technically be undefined behavior. Figuring that out is beyond the scope of my question, but a comment on that topic would be quite welcome.

signaling_incrementing_counter.h

#include <atomic>
#include "gpu_base_constants.h"

struct SignalingIncrementingCounter {
public:
    /**
     * We will either count up or count down to the given limit.  Once the limit is reached, whatever is waiting on this counter will be signaled and allowed to proceed.
     */
    void init(const int upper_limit_);
    void reset_to_empty();
    void increment(); // only valid when counting up
    void block_until_full(const char * comment = {""});
    // We don't have a use-case for the block_until_non_full

private:

    int upper_limit;

#if USE_INT_IN_SHARED_MEMORY_FOR_SIGNALING_COUNTER
    volatile int value;
#else // USE_INT_IN_SHARED_MEMORY_FOR_SIGNALING_COUNTER
    std::atomic<int> value;
    std::atomic<int> * value_ptr;
#endif // USE_INT_IN_SHARED_MEMORY_FOR_SIGNALING_COUNTER

    pthread_mutex_t mutex;
    pthread_cond_t cv;

};

signaling_incrementing_counter.cpp

#include <pthread.h>
#include <stdexcept>

#include "signaling_incrementing_counter.h"


void SignalingIncrementingCounter::init(const int upper_limit_) {

    upper_limit = upper_limit_;
#if !GPU_USE_INT_IN_SHARED_MEMORY_FOR_SIGNALING_COUNTER
    value_ptr = new(&value) std::atomic<int>(0);
#endif // GPU_USE_INT_IN_SHARED_MEMORY_FOR_SIGNALING_COUNTER
    {
        pthread_mutexattr_t attr;
        pthread_mutexattr_init(&attr);
        int retval = pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);
        if (retval) {
            throw std::runtime_error("Error while setting sharedp field for mutex");
        }
        pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);

        pthread_mutex_init(&mutex, &attr);
        pthread_mutexattr_destroy(&attr);
    }

    {
        pthread_condattr_t attr;
        pthread_condattr_init(&attr);
        pthread_condattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);

        pthread_cond_init(&cv, &attr);
        pthread_condattr_destroy(&attr);
    }

    reset_to_empty(); // should be done at end, since mutex functions are called
}

void SignalingIncrementingCounter::reset_to_empty() {
    int mutex_rv = pthread_mutex_lock(&mutex);
    if (mutex_rv) {
      throw std::runtime_error("Unexpected error encountered while grabbing lock.  Investigate.");
    }
    value = 0;
    // No need to signal, because there is no function that unblocks when the value changes to 0
    pthread_mutex_unlock(&mutex);
}

void SignalingIncrementingCounter::increment() {
    fprintf(stderr, "incrementing\n");
    int mutex_rv = pthread_mutex_lock(&mutex);
    if (mutex_rv) {
      throw std::runtime_error("Unexpected error encountered while grabbing lock.  Investigate.");
    }
    ++value;
    fprintf(stderr, "incremented\n");

    if (value >= upper_limit) {
        pthread_cond_broadcast(&cv);
    }
    pthread_mutex_unlock(&mutex);
}

void SignalingIncrementingCounter::block_until_full(const char * comment) {
    struct timespec max_wait = {0, 0};
    int mutex_rv = pthread_mutex_lock(&mutex);
    if (mutex_rv) {
      throw std::runtime_error("Unexpected error encountered while grabbing lock.  Investigate.");
    }
    while (value < upper_limit) {
        int val = value;
        printf("blocking during increment until full, value is %i, for %s\n", val, comment);
        /*const int gettime_rv =*/ clock_gettime(CLOCK_REALTIME, &max_wait);
        max_wait.tv_sec += 5;
        const int timed_wait_rv = pthread_cond_timedwait(&cv, &mutex, &max_wait);
        if (timed_wait_rv)
        {
            switch(timed_wait_rv) {
            case ETIMEDOUT:
                break;
            default:
              pthread_mutex_unlock(&mutex);
                throw std::runtime_error("Unexpected error encountered.  Investigate.");
            }
        }
    }
    pthread_mutex_unlock(&mutex);
}
interestedparty333
  • 2,386
  • 1
  • 21
  • 35
  • What's the point of storing `&value` inside the object? That's just a total waste of space. You only use it inside `init()` so it can at least be removed easily. – Peter Cordes Sep 17 '19 at 05:04
  • 1
    More importantly, taking the mutex around every increment defeats the purpose of using `atomic`. Just use `int newval = ++value` as shorthand for `.fetch_add(1) + 1` and only use the mutex + cv if necessary based on checking the local. Or if you need to use a mutex to avoid having multiple threads all notify at once, then don't use `atomic` or `volatile`; they're unnecessary if you're using a mutex anyway. But you can solve that by checking for `!= limit` instead of `>= limit` to make sure only one thread detects crossing the threshold. – Peter Cordes Sep 17 '19 at 05:10