Fredrik Kuivinen schrieb:
> The patch has been rebased on top of next and I believe that it is now
> ready for inclusion. It is time to decide if the added code complexity
> is worth the increased performance.
I also have to add a few nits to make this play better on Windows.
> +/* This lock protects all the variables above. */
> +static pthread_mutex_t grep_lock = PTHREAD_MUTEX_INITIALIZER;
> +
> +/* Signalled when a new work_item is added to todo. */
> +static pthread_cond_t cond_add = PTHREAD_COND_INITIALIZER;
> +
> +/* Signalled when the result from one work_item is written to
> + stdout. */
> +static pthread_cond_t cond_write = PTHREAD_COND_INITIALIZER;
> +
> +/* Signalled when we are finished with everything. */
> +static pthread_cond_t cond_result = PTHREAD_COND_INITIALIZER;
Please do not use PTHREAD_MUTEX_INITIALIZER nor PTHREAD_COND_INITIALIZER;
call pthread_mutex_init and pthread_cond_init (and the corresponding
*_destroy!!) from the code.
> +static void add_work(enum work_type type, char *name, char *buf, size_t size)
> +{
> +...
> + pthread_mutex_unlock(&grep_lock);
> + pthread_cond_signal(&cond_add);
Please swap these two lines, so that pthread_cond_signal() is called while
the lock is held.
> + /* Wake up all the consumer threads so they can see that there
> + is no more work to do. */
> + pthread_cond_broadcast(&cond_add);
> + pthread_mutex_unlock(&grep_lock);
Ouch! We do not have pthread_cond_broadcast() on Windows, yet. This
shouldn't be a stopper for this patch, though. Perhaps Andrzej (cc:ed) can
implement it?
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html