Hello Piotr Kliczewski, Saggi Mizrahi, Dan Kenigsberg, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/36409
to review the following change.
Change subject: protocoldetector: Fix polling timeout
......................................................................
protocoldetector: Fix polling timeout
The event loop was calculating timeout in seconds, but poll timeout
should be milliseconds. This caused poll to wait up to 30 milliseconds
instead of 30 seconds where there are no events to handle.
Profiling a system running one vm for 5 mintues show _process_events is
dominating the profile, taking 55% of cpu time:
Mon Nov 17 00:22:02 2014 vdsmd-master.prof
14440725 function calls (14721061 primitive calls) in 16.907 CPU
seconds
Ordered by: internal time
List reduced from 3079 to 10 due to restriction <10>
ncalls tottime percall cumtime percall filename:lineno(function)
2241981 5.685 0.000 9.445 0.000
protocoldetector.py:108(MultiProtocolAcceptor._process_events)
4501127 1.442 0.000 1.442 0.000 time:0(time)
2246162 1.396 0.000 1.396 0.000 __builtin__:0(poll)
2242305 0.737 0.000 0.737 0.000 __builtin__:0(max)
6566 0.352 0.000 0.536 0.000 spark.py:211(Parser.buildState)
50705 0.170 0.001 0.373 0.000 copy.py:144(deepcopy)
127 0.158 0.001 0.158 0.001 cpopen.cpopen:0(createProcess)
10394 0.154 0.000 0.154 0.000 __builtin__:0(open)
154006 0.132 0.000 0.269 0.000
encoder.py:284(JSONEncoder._iterencode)
2533 0.124 0.000 0.141 0.000 pthread.py:133(Cond.timedwait)
With this patch we can see that number of functions calls decreased by 76% and
total cpu time decreased by 57%:
Mon Nov 17 00:33:44 2014 vdsmd-poll-timeout.prof
3344648 function calls (3624721 primitive calls) in 7.340 CPU seconds
Ordered by: internal time
List reduced from 3069 to 10 due to restriction <10>
ncalls tottime percall cumtime percall filename:lineno(function)
6435 0.346 0.000 0.518 0.000 spark.py:211(Parser.buildState)
122 0.166 0.001 0.166 0.001 cpopen.cpopen:0(createProcess)
10202 0.157 0.000 0.157 0.000 __builtin__:0(open)
50705 0.148 0.001 0.329 0.000 copy.py:144(deepcopy)
98 0.140 0.001 0.140 0.001 posix:0(fork)
154178 0.115 0.000 0.236 0.000
encoder.py:284(JSONEncoder._iterencode)
2496 0.113 0.000 0.131 0.000 pthread.py:133(Cond.timedwait)
5870 0.112 0.000 0.153 0.000 spark.py:103(Parser.addRule)
3325 0.109 0.000 0.194 0.000
__init__.py:226(LogRecord.__init__)
28264 0.103 0.000 0.384 0.000
protocoldetector.py:108(MultiProtocolAcceptor._process_events)
Now we calculate the the timeout correctly, and improved variable names
make this code unlikely to break again.
Change-Id: If676b543bff30d08067d2cff3e0639c7e415f61f
Signed-off-by: Nir Soffer <[email protected]>
Reviewed-on: http://gerrit.ovirt.org/35212
Reviewed-by: Francesco Romani <[email protected]>
Reviewed-by: Dan Kenigsberg <[email protected]>
Reviewed-by: Piotr Kliczewski <[email protected]>
Reviewed-by: Saggi Mizrahi <[email protected]>
---
M vdsm/protocoldetector.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/36409/1
diff --git a/vdsm/protocoldetector.py b/vdsm/protocoldetector.py
index c9feeb0..c53f41d 100644
--- a/vdsm/protocoldetector.py
+++ b/vdsm/protocoldetector.py
@@ -92,8 +92,8 @@
self._cleanup()
def _process_events(self):
- timeout = max(self._next_cleanup - time.time(), 0)
- events = self._poller.poll(timeout)
+ seconds = max(self._next_cleanup - time.time(), 0)
+ events = self._poller.poll(seconds * 1000)
for fd, event in events:
if event & (select.POLLIN | select.POLLPRI):
--
To view, visit http://gerrit.ovirt.org/36409
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If676b543bff30d08067d2cff3e0639c7e415f61f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Nir Soffer <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
