remove superfluous % and replace postfix ++ with prefix ++

[GoogleCodeExporter]

What steps will reproduce the problem?
1. view the source

What is the expected output? What do you see instead?
everything is ok

What version of the product are you using? On what operating system?
HEAD

Please provide any additional information below.

Paste this into the source code please:

// Master thread adds accepted socket to a queue
static void produce_socket(struct mg_context *ctx, const struct socket *sp) {
  (void) pthread_mutex_lock(&ctx->mutex);

  // If the queue is full, wait
  while (ctx->stop_flag == 0 &&
         ctx->sq_head - ctx->sq_tail >= (int) ARRAY_SIZE(ctx->queue)) {
    (void) pthread_cond_wait(&ctx->sq_empty, &ctx->mutex);
  }

  if (ctx->sq_head - ctx->sq_tail < (int) ARRAY_SIZE(ctx->queue)) {
    // Copy socket to the queue and increment head
    ctx->queue[ctx->sq_head] = *sp;
    ++ctx->sq_head;
    DEBUG_TRACE(("queued socket %d", sp->sock));
  }

  (void) pthread_cond_signal(&ctx->sq_full);
  (void) pthread_mutex_unlock(&ctx->mutex);
}

// Worker threads take accepted socket from the queue
static int consume_socket(struct mg_context *ctx, struct socket *sp) {
  (void) pthread_mutex_lock(&ctx->mutex);
  DEBUG_TRACE(("going idle"));

  // If the queue is empty, wait. We're idle at this point.
  while (ctx->sq_head == ctx->sq_tail && ctx->stop_flag == 0) {
    pthread_cond_wait(&ctx->sq_full, &ctx->mutex);
  }

  // If we're stopping, sq_head may be equal to sq_tail.
  if (ctx->sq_head > ctx->sq_tail) {
    // Copy socket from the queue and increment tail
    *sp = ctx->queue[ctx->sq_tail];
    ++ctx->sq_tail;
    DEBUG_TRACE(("grabbed socket %d, going busy", sp->sock));

    // Wrap pointers if needed
    while (ctx->sq_tail == (int) ARRAY_SIZE(ctx->queue)) {
      ctx->sq_tail -= ARRAY_SIZE(ctx->queue);
      ctx->sq_head -= ARRAY_SIZE(ctx->queue);
    }
  }

  (void) pthread_cond_signal(&ctx->sq_empty);
  (void) pthread_mutex_unlock(&ctx->mutex);

  return !ctx->stop_flag;
}

Rationale:
ARRAY_SIZE macro is expensive, as it contains a divide. Modding (%) with it is 
also expensive. There is no need to do either, as only one thread operates on 
the queue at a time, and the tail/head will reset if they grow beyond 
ARRAY_SIZE.

Prefix ++ is better than postfix on user types (C++), as it prevents generation 
of a temporary.

Original issue reported on code.google.com by janez...@gmail.com on 16 Feb 2012 at 1:02

[GoogleCodeExporter]

Mistake:
produce_socket() is ok, but consume_socket IMO should be:

// Worker threads take accepted socket from the queue
static int consume_socket(struct mg_context *ctx, struct socket *sp) {
  (void) pthread_mutex_lock(&ctx->mutex);
  DEBUG_TRACE(("going idle"));

  // If the queue is empty, wait. We're idle at this point.
  while (ctx->sq_head == ctx->sq_tail && ctx->stop_flag == 0) {
    pthread_cond_wait(&ctx->sq_full, &ctx->mutex);
  }

  // If we're stopping, sq_head may be equal to sq_tail.
  if (ctx->sq_head > ctx->sq_tail) {
    // Copy socket from the queue and increment tail
    *sp = ctx->queue[ctx->sq_tail];
    ++ctx->sq_tail;
    DEBUG_TRACE(("grabbed socket %d, going busy", sp->sock));

    // Wrap pointers if needed
    if (ctx->sq_tail == (int) ARRAY_SIZE(ctx->queue)) {
      ctx->sq_tail = 0;
      ctx->sq_head -= ARRAY_SIZE(ctx->queue);
    }
  }

  (void) pthread_cond_signal(&ctx->sq_empty);
  (void) pthread_mutex_unlock(&ctx->mutex);

  return !ctx->stop_flag;
}

Original comment by janez...@gmail.com on 16 Feb 2012 at 1:30

[GoogleCodeExporter]

You can also paste this into produce_socket():

  // If the queue is full, wait
  while (ctx->stop_flag == 0 &&
         ctx->sq_head - ctx->sq_tail == (int) ARRAY_SIZE(ctx->queue)) {
    (void) pthread_cond_wait(&ctx->sq_empty, &ctx->mutex);
  }

On some systems == is faster than >=, also >= is superfluous, as the function 
will block as soon as the queue is not freed.

Original comment by janez...@gmail.com on 16 Feb 2012 at 3:02

[GoogleCodeExporter]

Well, this is micro-optimization, I very much doubt it makes any difference at 
all.
And, by the way, compiler optimizes ARRAY_SIZE at compile time, even if no 
optimization options specified:
(notice   movl    $123, %eax in the assembly output)

#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
int foo() {
  int blah[123];
  return ARRAY_SIZE(blah);
}

$ cc -S -c -arch i386 a.c
$ cat a.S
        .text
.globl _foo
_foo:
        pushl   %ebp
        movl    %esp, %ebp
        subl    $504, %esp
        movl    $123, %eax
        leave
        ret
        .subsections_via_symbols

You're correct about infix vs postfix ++, but this is C, not C++.

Original comment by valenok on 16 Feb 2012 at 6:00

• Changed state: WontFix

[GoogleCodeExporter]

Please replace the inequalities with equalities and you'll make my
day. See my comments.

16. februar 2012 19:01 je oseba  <mongoose@googlecode.com> napisala:

Original comment by janez...@gmail.com on 16 Feb 2012 at 6:51