diff mbox series

[1/3] tools/kvm_stat: add command line switch '-z' to skip zero records

Message ID 20200331200042.2026-2-raspl@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series tools/kvm_stat: add logfile support | expand

Commit Message

Stefan Raspl March 31, 2020, 8 p.m. UTC 
From: Stefan Raspl <raspl@de.ibm.com>

When running in logging mode, skip records with all zeros (=empty records)
to preserve space when logging to files.

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
---
 tools/kvm/kvm_stat/kvm_stat     | 47 +++++++++++++++++++++++++--------
 tools/kvm/kvm_stat/kvm_stat.txt |  4 +++
 2 files changed, 40 insertions(+), 11 deletions(-)

Comments

Paolo Bonzini March 31, 2020, 9:45 p.m. UTC | #1 
On 31/03/20 22:00, Stefan Raspl wrote:
> @@ -1523,14 +1535,20 @@ def log(stats, opts, frmt, keys):
>      """Prints statistics as reiterating key block, multiple value blocks."""
>      line = 0
>      banner_repeat = 20
> +    banner_printed = False
> +
>      while True:
>          try:
>              time.sleep(opts.set_delay)
> -            if line % banner_repeat == 0:
> +            if line % banner_repeat == 0 and not banner_printed:
>                  print(frmt.get_banner())
> -            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") +
> -                  frmt.get_statline(keys, stats.get()))
> +                banner_printed = True

Can't skip_zero_records be handled here instead?

    values = stats.get()
    if not opts.skip_zero_records or \
        any((values[k].delta != 0 for k in keys):
       statline = frmt.get_statline(keys, values)
       print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)

Paolo

> +            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)
>              line += 1
Stefan Raspl April 1, 2020, 8:19 a.m. UTC | #2 
On 2020-03-31 23:45, Paolo Bonzini wrote:
> On 31/03/20 22:00, Stefan Raspl wrote:
>> @@ -1523,14 +1535,20 @@ def log(stats, opts, frmt, keys):
>>      """Prints statistics as reiterating key block, multiple value blocks."""
>>      line = 0
>>      banner_repeat = 20
>> +    banner_printed = False
>> +
>>      while True:
>>          try:
>>              time.sleep(opts.set_delay)
>> -            if line % banner_repeat == 0:
>> +            if line % banner_repeat == 0 and not banner_printed:
>>                  print(frmt.get_banner())
>> -            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") +
>> -                  frmt.get_statline(keys, stats.get()))
>> +                banner_printed = True
> 
> Can't skip_zero_records be handled here instead?
> 
>     values = stats.get()
>     if not opts.skip_zero_records or \
>         any((values[k].delta != 0 for k in keys):
>        statline = frmt.get_statline(keys, values)
>        print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)

I wanted to avoid such an extra check for performance reasons. Granted, I have
to do something likewise (i.e. checking for non-zero values) in my patch for csv
records, but at least for the standard format things are a bit less costly
(avoiding an extra pass over the data).

Ciao,
Stefan
Paolo Bonzini April 1, 2020, 1:23 p.m. UTC | #3 
On 01/04/20 10:19, Stefan Raspl wrote:
> On 2020-03-31 23:45, Paolo Bonzini wrote:
>> On 31/03/20 22:00, Stefan Raspl wrote:
>>> @@ -1523,14 +1535,20 @@ def log(stats, opts, frmt, keys):
>>>      """Prints statistics as reiterating key block, multiple value blocks."""
>>>      line = 0
>>>      banner_repeat = 20
>>> +    banner_printed = False
>>> +
>>>      while True:
>>>          try:
>>>              time.sleep(opts.set_delay)
>>> -            if line % banner_repeat == 0:
>>> +            if line % banner_repeat == 0 and not banner_printed:
>>>                  print(frmt.get_banner())
>>> -            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") +
>>> -                  frmt.get_statline(keys, stats.get()))
>>> +                banner_printed = True
>>
>> Can't skip_zero_records be handled here instead?
>>
>>     values = stats.get()
>>     if not opts.skip_zero_records or \
>>         any((values[k].delta != 0 for k in keys):
>>        statline = frmt.get_statline(keys, values)
>>        print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)
> 
> I wanted to avoid such an extra check for performance reasons. Granted, I have
> to do something likewise (i.e. checking for non-zero values) in my patch for csv
> records, but at least for the standard format things are a bit less costly
> (avoiding an extra pass over the data).

I don't think it's a perceivable difference, and it simplifies the code
noticeably.

Paolo
diff mbox series

Patch

diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 7fe767bd2625..54000ac508f9 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -1488,7 +1488,8 @@  def batch(stats):
 
 
 class StdFormat(object):
-    def __init__(self, keys):
+    def __init__(self, keys, skip_zero_records):
+        self._skip_zero_records = skip_zero_records
         self._banner = ''
         for key in keys:
             self._banner += key.split(' ')[0] + ' '
@@ -1496,16 +1497,21 @@  class StdFormat(object):
     def get_banner(self):
         return self._banner
 
-    @staticmethod
-    def get_statline(keys, s):
+    def get_statline(self, keys, s):
         res = ''
+        non_zero = False
         for key in keys:
+            if s[key].delta != 0:
+                non_zero = True
             res += ' %9d' % s[key].delta
+        if self._skip_zero_records and not non_zero:
+            return ''
         return res
 
 
 class CSVFormat(object):
-    def __init__(self, keys):
+    def __init__(self, keys, skip_zero_records):
+        self._skip_zero_records = skip_zero_records
         self._banner = 'timestamp'
         self._banner += reduce(lambda res, key: "{},{!s}".format(res,
                                key.split(' ')[0]), keys, '')
@@ -1513,8 +1519,14 @@  class CSVFormat(object):
     def get_banner(self):
         return self._banner
 
-    @staticmethod
-    def get_statline(keys, s):
+    def get_statline(self, keys, s):
+        if self._skip_zero_records:
+            non_zero = False
+            for key in keys:
+                if s[key].delta != 0:
+                    non_zero = True
+            if self._skip_zero_records and not non_zero:
+                return ''
         return reduce(lambda res, key: "{},{!s}".format(res, s[key].delta),
                       keys, '')
 
@@ -1523,14 +1535,20 @@  def log(stats, opts, frmt, keys):
     """Prints statistics as reiterating key block, multiple value blocks."""
     line = 0
     banner_repeat = 20
+    banner_printed = False
+
     while True:
         try:
             time.sleep(opts.set_delay)
-            if line % banner_repeat == 0:
+            if line % banner_repeat == 0 and not banner_printed:
                 print(frmt.get_banner())
-            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") +
-                  frmt.get_statline(keys, stats.get()))
+                banner_printed = True
+            statline = frmt.get_statline(keys, stats.get())
+            if len(statline) == 0:
+                continue
+            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)
             line += 1
+            banner_printed = False
         except KeyboardInterrupt:
             break
 
@@ -1651,9 +1669,16 @@  Press any other key to refresh statistics immediately.
                            default=False,
                            help='retrieve statistics from tracepoints',
                            )
+    argparser.add_argument('-z', '--skip-zero-records',
+                           action='store_true',
+                           default=False,
+                           help='omit records with all zeros in logging mode',
+                           )
     options = argparser.parse_args()
     if options.csv and not options.log:
         sys.exit('Error: Option -c/--csv requires -l/--log')
+    if options.skip_zero_records and not options.log:
+        sys.exit('Error: Option -z/--skip-zero-records requires -l/--log')
     try:
         # verify that we were passed a valid regex up front
         re.compile(options.fields)
@@ -1736,9 +1761,9 @@  def main():
     if options.log:
         keys = sorted(stats.get().keys())
         if options.csv:
-            frmt = CSVFormat(keys)
+            frmt = CSVFormat(keys, options.skip_zero_records)
         else:
-            frmt = StdFormat(keys)
+            frmt = StdFormat(keys, options.skip_zero_records)
         log(stats, options, frmt, keys)
     elif not options.once:
         with Tui(stats, options) as tui:
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index a97ded2aedad..24296dccc00a 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -104,6 +104,10 @@  OPTIONS
 --tracepoints::
         retrieve statistics from tracepoints
 
+*z*::
+--skip-zero-records::
+        omit records with all zeros in logging mode
+
 SEE ALSO
 --------
 'perf'(1), 'trace-cmd'(1)