Saggi Mizrahi has posted comments on this change.

Change subject: oop: Use a single instance of IOProcess per SD
......................................................................


Patch Set 6: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/31501/6/vdsm/storage/outOfProcess.py
File vdsm/storage/outOfProcess.py:

Line 89: def _getIOProcessPool(clientName):
Line 90:     with _procPoolLock:
Line 91:         cleanIdleIOProcesses(clientName)
Line 92: 
Line 93:         try:
93-109 can be written as:
---
 proc = _refProcPool.get(clientName, lambda : None)()
 if proc is None:
     proc = _OopWrapper(IOProcess(max_threads=HELPERS_PER_DOMAIN,
                                  timeout=DEFAULT_TIMEOUT,
                                  max_queued_requests=MAX_QUEUED))
     _refProcPool[clientName] = weakref.ref(proc)


 _procPool[clientName] = (elapsed_time() + IOPROC_IDLE_TIME, proc)
 return proc
----
Which apart from being a bit shorter has a linear control flow which is simpler 
to understand.

 1. You get the proc in the refPool
 2. If it doesn't exist create one
 3. update the EOL
 4. return the proc
Line 94:             proc = _refProcPool[clientName]()
Line 95:             if proc is None:
Line 96:                 del _refProcPool[clientName]
Line 97:                 raise KeyError


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I383fe617ee4ce22de368ba54f980887d70ff37c5
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[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: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[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

Reply via email to