Nir Soffer has posted comments on this change. Change subject: executor: introduce the executor library ......................................................................
Patch Set 31: (1 comment) http://gerrit.ovirt.org/#/c/29191/31/lib/vdsm/executor.py File lib/vdsm/executor.py: Line 231: except IndexError: Line 232: with self._cond: Line 233: self._waiters += 1 Line 234: if not self._tasks: Line 235: self._cond.wait() > I forgot that wait() reacquires the mutex once woken up. There's no race he Yes, there is a race as you describe if put skip the notify before self._waiters was increased. self._waiters was introduced to avoid taking the lock and invoking notify when nobody is waiting, which is common when all workers are busy. I profiled this and it was significant improvement when testing with the sampling simulator. On a busy executor, for example in the content of sampling, this can lead to small delay in one of the tasks, until the next task is scheduled. On an unused executor, this can lead to bigger delay. I think this can work: def get(self): while True: try: return self._tasks.popleft() except IndexError: with self._cond: self._waiters += 1 if not self._tasks: self._cond.wait() self._waiters -= 1 If put happened before increasing the self._waiters, self._tasks.empty() will return False, and the waiter will try to get the task from the queue. Line 236: self._waiters -= 1 Line 237: Line 238: def clear(self): -- To view, visit http://gerrit.ovirt.org/29191 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic06da1ba57868dc2c7db67a1868ad10087a1cff2 Gerrit-PatchSet: 31 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
