0

I am learning mutex currently and the following is the test code. It works perfectly fine. It creates another instance after one is done. Yet, it also introduces overhead according to here. How efficient is locking an unlocked mutex? What is the cost of a mutex?. How can I modify my code to improve the efficiency?

pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;

typedef struct _context_t {
  uint64_t id;
  char *name;
  bool initialized;
} context_t;

context_t *ctx = NULL;

context_t *get_instance() {
  pthread_mutex_lock(&lock);
  if (ctx == NULL) {
    ctx = (context_t *)malloc(sizeof(context_t));
    assert(ctx != NULL);
    ctx->initialized = false;
  }
  pthread_mutex_unlock(&lock);
  return ctx;
}

int id = 0;

void *do_work(void *arg) {
  context_t *ctx = get_instance();
  if (!ctx->initialized) {
    ctx->name = (char *)arg;
    ctx->id = ++id;
    ctx->initialized = true;
  }
  printf("name=%s\tid=%ld\n", ctx->name, ctx->id);
  return NULL;
}

int main() {
  int rc;
  pthread_t p1, p2;

  rc = pthread_create(&p1, NULL, do_work, "A");
  assert(rc == 0);
  rc = pthread_create(&p2, NULL, do_work, "B");
  assert(rc == 0);

  rc = pthread_join(p1, NULL);
  assert(rc == 0);
  rc = pthread_join(p2, NULL);
  assert(rc == 0);

  if (ctx) {
    free(ctx);
  }
  return 0;
}
user13851309
  • 79
  • 1
  • 9
  • 1
    I'm not sure I understand the question. Yes, there is overhead involved in locking a mutex, but there is nothing your code can do to make that less on a per-call basis. The overhead is characteristic of the mutex operations, not of the calling code. You can reduce the *aggregate* overhead from mutex operations by reducing the number of mutex operations performed, but that may cause other inefficiencies. – John Bollinger Nov 14 '20 at 01:06
  • 4
    Note that there is a race condition in the `do_work()` function — any code that modifies the shared structure needs to use the mutex to ensure that the structure is not changed by other threads at the same time. The whole `if (!ctx->initialized) { … }` block needs to be protected by locking and unlocking the mutex. You are not using the mutex enough. You are not incurring enough overhead. – Jonathan Leffler Nov 14 '20 at 01:09
  • can you explain a little more about the race condition? – user13851309 Nov 14 '20 at 01:11
  • When you call `do_work()`, the mutex is locked inside `get_instance()` which is good, but is unlocked too early, at the end of `get_instance()` rather than the end of `do_work()`. – John Zwinck Nov 14 '20 at 01:12
  • Both thread `"A"` and thread `"B"` could be executing the test and the following code at the same time. The results are wholly unpredictable. Your code here may not be complex enough to show the problem, but the modifications must be protected for the code to be reliable. – Jonathan Leffler Nov 14 '20 at 01:12
  • 2
    I'm inclined to think your efficiency concerns are misplaced, or at least premature. Is your program too slow? If so, then first identify where it's spending its time, then have a look at improving those parts. But not before making it *correct*. – John Bollinger Nov 14 '20 at 01:15
  • @JohnBollinger How can I reduce the number of mutex operations? There is only one pair of a mutex. – user13851309 Nov 14 '20 at 01:16
  • That was a general observation about the nature of the problem. You might not be able to employ it in your particular code. But on the other hand, do have a look at @JohnZwinck's answer. – John Bollinger Nov 14 '20 at 01:19
  • 1
    You could avoid extra mutex operations by passing `arg` from `do_work()` to `get_instance()` and completing the initialization of the context in `get_instance()`. You'd have just four mutex operations — one lock and one unlock for each of two threads. That's about as minimal as you can get. – Jonathan Leffler Nov 14 '20 at 01:20

1 Answers1

2

Instead of having two threads racing to create the context_t, you should create it once before the threads start, and pass its address to the threads explicitly. Note that you can pass multiple arguments via pthread_create() by putting them in a struct and passing its address.

Then you won't need a mutex at all, because the threads will only read from ctx rather than potentially writing to it.

John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • If the code in `do_work()` should increment the `id` value, then you'd probably need a mutex — or you'd have to get involved with atomics (``, etc). If you need to do anything more than increment `id`, you'd need a mutex for reliable operation. – Jonathan Leffler Nov 14 '20 at 01:23