GNU bug report logs - #6729
[PATCH] sort: omit unnecessary mutex unlock+lock; simplify heap access

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6729 in the body.
You can then email your comments to 6729 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> CS.UCLA.EDU>
To: Bug Coreutils <bug-coreutils <at> gnu.org>
Subject: [PATCH] sort: omit unnecessary mutex unlock+lock; simplify heap access
Date: Sun, 25 Jul 2010 20:57:24 -0700

This removes an unnecessary mutex lock+unlock
and prepares for a refactoring patch to heap.c that I'll
install shortly.

From 7cc2429f13db9b0b70572ac1879506f4e444f4c6 Mon Sep 17 00:00:00 2001
From: Paul R. Eggert <eggert <at> cs.ucla.edu>
Date: Sun, 25 Jul 2010 20:54:55 -0700
Subject: [PATCH] sort: omit unnecessary mutex unlock+lock; simplify heap access

* src/sort.c (queue_pop): Omit unnecessary unlock+lock after
pthread_cond_wait returns.  Don't access "count" member of the
heap; any efficiency gains should be quite minor, the access
complicates this code, and "count" should be private anyway.
---
 src/sort.c |   19 +++++--------------
 1 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/src/sort.c b/src/sort.c
index ea2720f..577521d 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -3173,20 +3173,11 @@ queue_insert (struct merge_node_queue *queue, struct merge_node *node)
 static inline struct merge_node *
 queue_pop (struct merge_node_queue *queue)
 {
-  struct merge_node *node = NULL;
-
-  while (!node)
-    {
-      pthread_mutex_lock (&queue->mutex);
-      if (queue->priority_queue->count)
-        node = heap_remove_top (queue->priority_queue);
-      else
-        {
-          /* Go into conditional wait if no NODE is immediately available.  */
-          pthread_cond_wait (&queue->cond, &queue->mutex);
-        }
-      pthread_mutex_unlock (&queue->mutex);
-    }
+  struct merge_node *node;
+  pthread_mutex_lock (&queue->mutex);
+  while (! (node = heap_remove_top (queue->priority_queue)))
+    pthread_cond_wait (&queue->cond, &queue->mutex);
+  pthread_mutex_unlock (&queue->mutex);
   lock_node (node);
   node->queued = false;
   return node;
-- 
1.7.1

Message #10 received at 6729-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> CS.UCLA.EDU>
Cc: 6729-done <at> debbugs.gnu.org
Subject: Re: bug#6729: [PATCH] sort: omit unnecessary mutex unlock+lock;
	simplify heap access
Date: Mon, 26 Jul 2010 10:53:25 +0100

On 26/07/10 04:57, Paul Eggert wrote:
> This removes an unnecessary mutex lock+unlock
> and prepares for a refactoring patch to heap.c that I'll
> install shortly.

closing...

thanks,
Pádraig.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.