Change in vdsm[master]: utils: Optimize namedtuple definition in pidStat

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

utils: Optimize namedtuple definition in pidStat

Profiling idle vdsm reveal that 60% of cpu time is spent in creating
pointless namedtuples. In particular, 823 calls were made in 4.88
seconds of cpu time, invoking collections.py genexpr function 296805
times.

This patch creates the one and only namedtuple needed by pidStat when
the module is imported.

Change-Id: Ic5841b338cc0e9376c3bd7815206c27829c286d6
Signed-off-by: Nir Soffer 
---
M lib/vdsm/utils.py
1 file changed, 14 insertions(+), 12 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/26097/1

diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index b682dec..af0079e 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -318,19 +318,21 @@
             raise

+STAT = namedtuple('stat', ('pid', 'comm', 'state', 'ppid', 'pgrp', 'session',
+                           'tty_nr', 'tpgid', 'flags', 'minflt', 'cminflt',
+                           'majflt', 'cmajflt', 'utime', 'stime', 'cutime',
+                           'cstime', 'priority', 'nice', 'num_threads',
+                           'itrealvalue', 'starttime', 'vsize', 'rss',
+                           'rsslim', 'startcode', 'endcode', 'startstack',
+                           'kstkesp', 'kstkeip', 'signal', 'blocked',
+                           'sigignore', 'sigcatch', 'wchan', 'nswap',
+                           'cnswap', 'exit_signal', 'processor',
+                           'rt_priority', 'policy', 'delayacct_blkio_ticks',
+                           'guest_time', 'cguest_time'))
+
+
 def pidStat(pid):
     res = []
-    fields = ('pid', 'comm', 'state', 'ppid', 'pgrp', 'session',
-              'tty_nr', 'tpgid', 'flags', 'minflt', 'cminflt',
-              'majflt', 'cmajflt', 'utime', 'stime', 'cutime',
-              'cstime', 'priority', 'nice', 'num_threads',
-              'itrealvalue', 'starttime', 'vsize', 'rss', 'rsslim',
-              'startcode', 'endcode', 'startstack', 'kstkesp',
-              'kstkeip', 'signal', 'blocked', 'sigignore', 'sigcatch',
-              'wchan', 'nswap', 'cnswap', 'exit_signal', 'processor',
-              'rt_priority', 'policy', 'delayacct_blkio_ticks',
-              'guest_time', 'cguest_time')
-    stat = namedtuple('stat', fields)
     with open("/proc/%d/stat" % pid, "r") as f:
         statline = f.readline()
         procNameStart = statline.find("(")
@@ -343,7 +345,7 @@
         # Only 44 feilds are documented in man page while /proc/pid/stat has 52
         # The rest of the fields contain the process memory layout and
         # exit_code, which are not relevant for our use.
-        return stat._make(res[:len(fields)])
+        return STAT._make(res[:len(STAT._fields)])

def iteratePids():

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6852/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7642/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7752/ : SUCCESS

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/26097/1/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 344:         res.extend([int(item) for item in args[1:]])
Line 345:         # Only 44 feilds are documented in man page while /proc/pid/stat has 52
Line 346:         # The rest of the fields contain the process memory layout and
Line 347:         # exit_code, which are not relevant for our use.
Line 348:         return STAT._make(res[:len(STAT._fields)])
Note: _fields is not a private. Like _make, it uses an underscore prefix to prevent conflicts with field names. See http://docs.python.org/2.7/library/collections.html#namedtuple-factory-funct....
Line 349: 
Line 350: 
Line 351: def iteratePids():
Line 352:     for path in glob.iglob("/proc/[0-9]*"):

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

Patch Set 1: Verified+1

Verified by running with the patch as spm. The cpu usage of idle vdsm is lower.

Yaniv Bronhaim has posted comments on this change.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

Patch Set 1: Code-Review+1

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

Patch Set 1: Code-Review+2

Francesco Romani has posted comments on this change.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

Patch Set 1: Code-Review+1

good finding!

Antoni Segura Puimedon has posted comments on this change.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

Patch Set 1: Code-Review+1

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

Patch Set 1:

Waking up maintainer :-)

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: utils: Optimize namedtuple definition in pidStat
......................................................................

utils: Optimize namedtuple definition in pidStat

Profiling idle vdsm reveal that 60% of cpu time is spent in creating
pointless namedtuples. In particular, 823 calls were made in 4.88
seconds of cpu time, invoking collections.py genexpr function 296805
times.

This patch creates the one and only namedtuple needed by pidStat when
the module is imported.

Change-Id: Ic5841b338cc0e9376c3bd7815206c27829c286d6
Signed-off-by: Nir Soffer 
Reviewed-on: http://gerrit.ovirt.org/26097
Reviewed-by: Yaniv Bronhaim 
Reviewed-by: Dan Kenigsberg 
Reviewed-by: Francesco Romani 
Reviewed-by: Antoni Segura Puimedon 
---
M lib/vdsm/utils.py
1 file changed, 14 insertions(+), 12 deletions(-)

Approvals:
  Nir Soffer: Verified
  Yaniv Bronhaim: Looks good to me, but someone else must approve
  Antoni Segura Puimedon: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Looks good to me, but someone else must approve