Change in vdsm[master]: executor: avoid to notify() busy workers

fromani at redhat.com fromani at redhat.com
Tue Jan 27 16:47:42 UTC 2015


Francesco Romani has uploaded a new change for review.

Change subject: executor: avoid to notify() busy workers
......................................................................

executor: avoid to notify() busy workers

Optimize the usage of Condition variable in
the TaskQueue class.

Executor uses TaskQueue to dispatch tasks
to Workers. The TaskQueue class let the Worker
block if there are no tasks to do, and
dutifully notify()ies them after each new task
received.

Profiling suggests that to notify() already busy
Workers is wasteful and drags down performance,
due to the need to acquire the underlying Condition
variable.

Thus, this patch avoids notify()cation if there are
not Workers waiting for tasks, to achieve better
performances.

Credits for profiling and original optimization to
Nir Soffer during the review of the Executor patch
on http://gerrit.ovirt.org/#/c/29191

Change-Id: Ia197ee8bdeea3e1a353fc9fa5953788d95d53581
Signed-off-by: Francesco Romani <fromani at redhat.com>
---
M lib/vdsm/executor.py
1 file changed, 10 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/26/37326/1

diff --git a/lib/vdsm/executor.py b/lib/vdsm/executor.py
index 2cff983..374834d 100644
--- a/lib/vdsm/executor.py
+++ b/lib/vdsm/executor.py
@@ -224,6 +224,11 @@
         # additional locking. We need this condition only for waiting. See:
         # https://docs.python.org/2.6/library/queue.html#Queue.Full
         self._cond = threading.Condition(threading.Lock())
+        # we want to avoid taking the lock and invoking notify when nobody is
+        # waiting, which is common when all workers are busy.
+        # Profiling suggests significant improvements when testing with the
+        # sampling simulator.
+        self._waiters = 0
 
     def put(self, task):
         # there is a race here but we don't really mind. Worst case scenario,
@@ -232,8 +237,9 @@
         if len(self._tasks) == self._max_tasks:
             raise TooManyTasks()
         self._tasks.append(task)
-        with self._cond:
-            self._cond.notify()
+        if self._waiters > 0:
+            with self._cond:
+                self._cond.notify()
 
     def get(self):
         while True:
@@ -241,8 +247,10 @@
                 return self._tasks.popleft()
             except IndexError:
                 with self._cond:
+                    self._waiters += 1
                     if not self._tasks:
                         self._cond.wait()
+                    self._waiters -= 1
 
     def clear(self):
         self._tasks.clear()


-- 
To view, visit http://gerrit.ovirt.org/37326
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia197ee8bdeea3e1a353fc9fa5953788d95d53581
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list