* [PATCH 0/6] tools/kvm_stat improvements
@ 2018-02-14 21:45 Stefan Raspl
2018-02-14 21:45 ` [PATCH 1/6] tools/kvm_stat: mark private methods as such Stefan Raspl
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Stefan Raspl @ 2018-02-14 21:45 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Some bugfixes (patches 1&2), simplifications (patch 4), plus rework that
lays the basis for introducing a (hopefully) neater drilldown mode in
patch 7.
Note that in absence of any knowledge of the interdependencies of the
events in debugfs on other platforms like x86, this new 'functionality'
was only added for S390 for now. Adding other platforms would be easy
enough, though.
Furthermore note that the data passing between Providers and Stats is
pretty much at its limit now - if we'd ever wanted to put in more
attributes along with the event data passed from Providers to Stats, we
should probably switch to passing more involved data structures with
indicator flags, etc. Not sure if performance is a concern, but from a
coding/maintaining point of view that's something we should consider in
that case.
Changes in v2:
* Patch [1]:
- Switched to single underscore prefixing for "private"
methods.
* Patch [4]:
- Renamed methods to "tracepoint_is_child" and "debugfs_is_child"
from "trc_is_child" et al.
- Also, moved "child_events" and "pid" from DebugfsProvider and
TracepointProvider up to "Provider".
---
Stefan Raspl (6):
tools/kvm_stat: mark private methods as such
tools/kvm_stat: eliminate extra guest/pid selection dialog
tools/kvm_stat: cache compiled regular expression
tools/kvm_stat: separate drilldown and fields filtering
tools/kvm_stat: group child events indented after parent
tools/kvm_stat: print 'Total' line for multiple events only
tools/kvm/kvm_stat/kvm_stat | 435 ++++++++++++++++++++++------------------
tools/kvm/kvm_stat/kvm_stat.txt | 4 +-
2 files changed, 246 insertions(+), 193 deletions(-)
--
2.13.5
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/6] tools/kvm_stat: mark private methods as such
2018-02-14 21:45 [PATCH 0/6] tools/kvm_stat improvements Stefan Raspl
@ 2018-02-14 21:45 ` Stefan Raspl
2018-02-14 21:45 ` [PATCH 2/6] tools/kvm_stat: eliminate extra guest/pid selection dialog Stefan Raspl
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Raspl @ 2018-02-14 21:45 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
From: Stefan Raspl <stefan.raspl@de.ibm.com>
Helps quite a bit reading the code when it's obvious when a method is
intended for internal use only.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 132 ++++++++++++++++++++++----------------------
1 file changed, 66 insertions(+), 66 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 0d92c12f2b0f..0e69bbabc9db 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -376,8 +376,8 @@ class Event(object):
self.syscall = self.libc.syscall
self.name = name
self.fd = None
- self.setup_event(group, trace_cpu, trace_pid, trace_point,
- trace_filter, trace_set)
+ self._setup_event(group, trace_cpu, trace_pid, trace_point,
+ trace_filter, trace_set)
def __del__(self):
"""Closes the event's file descriptor.
@@ -390,7 +390,7 @@ class Event(object):
if self.fd:
os.close(self.fd)
- def perf_event_open(self, attr, pid, cpu, group_fd, flags):
+ def _perf_event_open(self, attr, pid, cpu, group_fd, flags):
"""Wrapper for the sys_perf_evt_open() syscall.
Used to set up performance events, returns a file descriptor or -1
@@ -409,7 +409,7 @@ class Event(object):
ctypes.c_int(pid), ctypes.c_int(cpu),
ctypes.c_int(group_fd), ctypes.c_long(flags))
- def setup_event_attribute(self, trace_set, trace_point):
+ def _setup_event_attribute(self, trace_set, trace_point):
"""Returns an initialized ctype perf_event_attr struct."""
id_path = os.path.join(PATH_DEBUGFS_TRACING, 'events', trace_set,
@@ -419,8 +419,8 @@ class Event(object):
event_attr.config = int(open(id_path).read())
return event_attr
- def setup_event(self, group, trace_cpu, trace_pid, trace_point,
- trace_filter, trace_set):
+ def _setup_event(self, group, trace_cpu, trace_pid, trace_point,
+ trace_filter, trace_set):
"""Sets up the perf event in Linux.
Issues the syscall to register the event in the kernel and
@@ -428,7 +428,7 @@ class Event(object):
"""
- event_attr = self.setup_event_attribute(trace_set, trace_point)
+ event_attr = self._setup_event_attribute(trace_set, trace_point)
# First event will be group leader.
group_leader = -1
@@ -437,8 +437,8 @@ class Event(object):
if group.events:
group_leader = group.events[0].fd
- fd = self.perf_event_open(event_attr, trace_pid,
- trace_cpu, group_leader, 0)
+ fd = self._perf_event_open(event_attr, trace_pid,
+ trace_cpu, group_leader, 0)
if fd == -1:
err = ctypes.get_errno()
raise OSError(err, os.strerror(err),
@@ -500,12 +500,12 @@ class TracepointProvider(Provider):
"""
def __init__(self, pid, fields_filter):
self.group_leaders = []
- self.filters = self.get_filters()
+ self.filters = self._get_filters()
self.update_fields(fields_filter)
self.pid = pid
@staticmethod
- def get_filters():
+ def _get_filters():
"""Returns a dict of trace events, their filter ids and
the values that can be filtered.
@@ -521,7 +521,7 @@ class TracepointProvider(Provider):
filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
return filters
- def get_available_fields(self):
+ def _get_available_fields(self):
"""Returns a list of available event's of format 'event name(filter
name)'.
@@ -549,11 +549,11 @@ class TracepointProvider(Provider):
def update_fields(self, fields_filter):
"""Refresh fields, applying fields_filter"""
- self.fields = [field for field in self.get_available_fields()
+ self.fields = [field for field in self._get_available_fields()
if self.is_field_wanted(fields_filter, field)]
@staticmethod
- def get_online_cpus():
+ def _get_online_cpus():
"""Returns a list of cpu id integers."""
def parse_int_list(list_string):
"""Returns an int list from a string of comma separated integers and
@@ -575,17 +575,17 @@ class TracepointProvider(Provider):
cpu_string = cpu_list.readline()
return parse_int_list(cpu_string)
- def setup_traces(self):
+ def _setup_traces(self):
"""Creates all event and group objects needed to be able to retrieve
data."""
- fields = self.get_available_fields()
+ fields = self._get_available_fields()
if self._pid > 0:
# Fetch list of all threads of the monitored pid, as qemu
# starts a thread for each vcpu.
path = os.path.join('/proc', str(self._pid), 'task')
groupids = self.walkdir(path)[1]
else:
- groupids = self.get_online_cpus()
+ groupids = self._get_online_cpus()
# The constant is needed as a buffer for python libs, std
# streams and other files that the script opens.
@@ -663,7 +663,7 @@ class TracepointProvider(Provider):
# The garbage collector will get rid of all Event/Group
# objects and open files after removing the references.
self.group_leaders = []
- self.setup_traces()
+ self._setup_traces()
self.fields = self._fields
def read(self, by_guest=0):
@@ -692,9 +692,9 @@ class DebugfsProvider(Provider):
self.paths = []
self.pid = pid
if include_past:
- self.restore()
+ self._restore()
- def get_available_fields(self):
+ def _get_available_fields(self):
""""Returns a list of available fields.
The fields are all available KVM debugfs files
@@ -704,7 +704,7 @@ class DebugfsProvider(Provider):
def update_fields(self, fields_filter):
"""Refresh fields, applying fields_filter"""
- self._fields = [field for field in self.get_available_fields()
+ self._fields = [field for field in self._get_available_fields()
if self.is_field_wanted(fields_filter, field)]
@property
@@ -758,7 +758,7 @@ class DebugfsProvider(Provider):
paths.append(dir)
for path in paths:
for field in self._fields:
- value = self.read_field(field, path)
+ value = self._read_field(field, path)
key = path + field
if reset == 1:
self._baseline[key] = value
@@ -779,7 +779,7 @@ class DebugfsProvider(Provider):
return results
- def read_field(self, field, path):
+ def _read_field(self, field, path):
"""Returns the value of a single field from a specific VM."""
try:
return int(open(os.path.join(PATH_DEBUGFS_KVM,
@@ -794,7 +794,7 @@ class DebugfsProvider(Provider):
self._baseline = {}
self.read(1)
- def restore(self):
+ def _restore(self):
"""Reset field counters"""
self._baseline = {}
self.read(2)
@@ -811,13 +811,12 @@ class Stats(object):
"""
def __init__(self, options):
- self.providers = self.get_providers(options)
+ self.providers = self._get_providers(options)
self._pid_filter = options.pid
self._fields_filter = options.fields
self.values = {}
- @staticmethod
- def get_providers(options):
+ def _get_providers(self, options):
"""Returns a list of data providers depending on the passed options."""
providers = []
@@ -829,7 +828,7 @@ class Stats(object):
return providers
- def update_provider_filters(self):
+ def _update_provider_filters(self):
"""Propagates fields filters to providers."""
# As we reset the counters when updating the fields we can
# also clear the cache of old values.
@@ -850,7 +849,7 @@ class Stats(object):
def fields_filter(self, fields_filter):
if fields_filter != self._fields_filter:
self._fields_filter = fields_filter
- self.update_provider_filters()
+ self._update_provider_filters()
@property
def pid_filter(self):
@@ -972,7 +971,7 @@ class Tui(object):
return res
- def print_all_gnames(self, row):
+ def _print_all_gnames(self, row):
"""Print a list of all running guests along with their pids."""
self.screen.addstr(row, 2, '%8s %-60s' %
('Pid', 'Guest Name (fuzzy list, might be '
@@ -1035,7 +1034,7 @@ class Tui(object):
return name
- def update_drilldown(self):
+ def _update_drilldown(self):
"""Sets or removes a filter that only allows fields without braces."""
if not self.stats.fields_filter:
self.stats.fields_filter = DEFAULT_REGEX
@@ -1043,11 +1042,11 @@ class Tui(object):
elif self.stats.fields_filter == DEFAULT_REGEX:
self.stats.fields_filter = None
- def update_pid(self, pid):
+ def _update_pid(self, pid):
"""Propagates pid selection to stats object."""
self.stats.pid_filter = pid
- def refresh_header(self, pid=None):
+ def _refresh_header(self, pid=None):
"""Refreshes the header."""
if pid is None:
pid = self.stats.pid_filter
@@ -1078,7 +1077,7 @@ class Tui(object):
self.screen.addstr(4, 1, 'Collecting data...')
self.screen.refresh()
- def refresh_body(self, sleeptime):
+ def _refresh_body(self, sleeptime):
row = 3
self.screen.move(row, 0)
self.screen.clrtobot()
@@ -1127,7 +1126,7 @@ class Tui(object):
curses.A_BOLD)
self.screen.refresh()
- def show_msg(self, text):
+ def _show_msg(self, text):
"""Display message centered text and exit on key press"""
hint = 'Press any key to continue'
curses.cbreak()
@@ -1142,7 +1141,7 @@ class Tui(object):
curses.A_STANDOUT)
self.screen.getkey()
- def show_help_interactive(self):
+ def _show_help_interactive(self):
"""Display help with list of interactive commands"""
msg = (' b toggle events by guests (debugfs only, honors'
' filters)',
@@ -1168,9 +1167,9 @@ class Tui(object):
self.screen.addstr(row, 0, line)
row += 1
self.screen.getkey()
- self.refresh_header()
+ self._refresh_header()
- def show_filter_selection(self):
+ def _show_filter_selection(self):
"""Draws filter selection mask.
Asks for a valid regex and sets the fields filter accordingly.
@@ -1192,18 +1191,18 @@ class Tui(object):
curses.noecho()
if len(regex) == 0:
self.stats.fields_filter = DEFAULT_REGEX
- self.refresh_header()
+ self._refresh_header()
return
try:
re.compile(regex)
self.stats.fields_filter = regex
- self.refresh_header()
+ self._refresh_header()
return
except re.error:
msg = '"' + regex + '": Not a valid regular expression'
continue
- def show_vm_selection_by_pid(self):
+ def _show_vm_selection_by_pid(self):
"""Draws PID selection mask.
Asks for a pid until a valid pid or 0 has been entered.
@@ -1219,7 +1218,7 @@ class Tui(object):
'This might limit the shown data to the trace '
'statistics.')
self.screen.addstr(5, 0, msg)
- self.print_all_gnames(7)
+ self._print_all_gnames(7)
curses.echo()
self.screen.addstr(3, 0, "Pid [0 or pid]: ")
@@ -1235,13 +1234,13 @@ class Tui(object):
continue
else:
pid = 0
- self.refresh_header(pid)
- self.update_pid(pid)
+ self._refresh_header(pid)
+ self._update_pid(pid)
break
except ValueError:
msg = '"' + str(pid) + '": Not a valid pid'
- def show_set_update_interval(self):
+ def _show_set_update_interval(self):
"""Draws update interval selection mask."""
msg = ''
while True:
@@ -1271,9 +1270,9 @@ class Tui(object):
except ValueError:
msg = '"' + str(val) + '": Invalid value'
- self.refresh_header()
+ self._refresh_header()
- def show_vm_selection_by_guest_name(self):
+ def _show_vm_selection_by_guest_name(self):
"""Draws guest selection mask.
Asks for a guest name until a valid guest name or '' is entered.
@@ -1289,15 +1288,15 @@ class Tui(object):
'This might limit the shown data to the trace '
'statistics.')
self.screen.addstr(5, 0, msg)
- self.print_all_gnames(7)
+ self._print_all_gnames(7)
curses.echo()
self.screen.addstr(3, 0, "Guest [ENTER or guest]: ")
gname = self.screen.getstr().decode(ENCODING)
curses.noecho()
if not gname:
- self.refresh_header(0)
- self.update_pid(0)
+ self._refresh_header(0)
+ self._update_pid(0)
break
else:
pids = []
@@ -1314,17 +1313,17 @@ class Tui(object):
msg = '"' + gname + '": Multiple matches found, use pid ' \
'filter instead'
continue
- self.refresh_header(pids[0])
- self.update_pid(pids[0])
+ self._refresh_header(pids[0])
+ self._update_pid(pids[0])
break
def show_stats(self):
"""Refreshes the screen and processes user input."""
sleeptime = self._delay_initial
- self.refresh_header()
+ self._refresh_header()
start = 0.0 # result based on init value never appears on screen
while True:
- self.refresh_body(time.time() - start)
+ self._refresh_body(time.time() - start)
curses.halfdelay(int(sleeptime * 10))
start = time.time()
sleeptime = self._delay_regular
@@ -1333,32 +1332,33 @@ class Tui(object):
if char == 'b':
self._display_guests = not self._display_guests
if self.stats.toggle_display_guests(self._display_guests):
- self.show_msg(['Command not available with tracepoints'
- ' enabled', 'Restart with debugfs only '
- '(see option \'-d\') and try again!'])
+ self._show_msg(['Command not available with '
+ 'tracepoints enabled', 'Restart with '
+ 'debugfs only (see option \'-d\') and '
+ 'try again!'])
self._display_guests = not self._display_guests
- self.refresh_header()
+ self._refresh_header()
if char == 'c':
self.stats.fields_filter = DEFAULT_REGEX
- self.refresh_header(0)
- self.update_pid(0)
+ self._refresh_header(0)
+ self._update_pid(0)
if char == 'f':
curses.curs_set(1)
- self.show_filter_selection()
+ self._show_filter_selection()
curses.curs_set(0)
sleeptime = self._delay_initial
if char == 'g':
curses.curs_set(1)
- self.show_vm_selection_by_guest_name()
+ self._show_vm_selection_by_guest_name()
curses.curs_set(0)
sleeptime = self._delay_initial
if char == 'h':
- self.show_help_interactive()
+ self._show_help_interactive()
if char == 'o':
self._sorting = not self._sorting
if char == 'p':
curses.curs_set(1)
- self.show_vm_selection_by_pid()
+ self._show_vm_selection_by_pid()
curses.curs_set(0)
sleeptime = self._delay_initial
if char == 'q':
@@ -1367,11 +1367,11 @@ class Tui(object):
self.stats.reset()
if char == 's':
curses.curs_set(1)
- self.show_set_update_interval()
+ self._show_set_update_interval()
curses.curs_set(0)
sleeptime = self._delay_initial
if char == 'x':
- self.update_drilldown()
+ self._update_drilldown()
# prevents display of current values on next refresh
self.stats.get(self._display_guests)
except KeyboardInterrupt:
--
2.13.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/6] tools/kvm_stat: eliminate extra guest/pid selection dialog
2018-02-14 21:45 [PATCH 0/6] tools/kvm_stat improvements Stefan Raspl
2018-02-14 21:45 ` [PATCH 1/6] tools/kvm_stat: mark private methods as such Stefan Raspl
@ 2018-02-14 21:45 ` Stefan Raspl
2018-02-14 21:45 ` [PATCH 3/6] tools/kvm_stat: cache compiled regular expression Stefan Raspl
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Raspl @ 2018-02-14 21:45 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
From: Stefan Raspl <stefan.raspl@de.ibm.com>
We can do with a single dialog that takes both, pids and guest names.
Note that we keep both interactive commands, 'p' and 'g' for now, to
avoid confusion among users used to a specific key.
While at it, we improve on some minor glitches regarding curses usage,
e.g. cursor still visible when not supposed to be.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 110 ++++++++++++++--------------------------
tools/kvm/kvm_stat/kvm_stat.txt | 4 +-
2 files changed, 39 insertions(+), 75 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 0e69bbabc9db..d7c80ac0374d 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -1044,6 +1044,8 @@ class Tui(object):
def _update_pid(self, pid):
"""Propagates pid selection to stats object."""
+ self.screen.addstr(4, 1, 'Updating pid filter...')
+ self.screen.refresh()
self.stats.pid_filter = pid
def _refresh_header(self, pid=None):
@@ -1147,10 +1149,10 @@ class Tui(object):
' filters)',
' c clear filter',
' f filter by regular expression',
- ' g filter by guest name',
+ ' g filter by guest name/PID',
' h display interactive commands reference',
' o toggle sorting order (Total vs CurAvg/s)',
- ' p filter by PID',
+ ' p filter by guest name/PID',
' q quit',
' r reset stats',
' s set update interval',
@@ -1202,44 +1204,6 @@ class Tui(object):
msg = '"' + regex + '": Not a valid regular expression'
continue
- def _show_vm_selection_by_pid(self):
- """Draws PID selection mask.
-
- Asks for a pid until a valid pid or 0 has been entered.
-
- """
- msg = ''
- while True:
- self.screen.erase()
- self.screen.addstr(0, 0,
- 'Show statistics for specific pid.',
- curses.A_BOLD)
- self.screen.addstr(1, 0,
- 'This might limit the shown data to the trace '
- 'statistics.')
- self.screen.addstr(5, 0, msg)
- self._print_all_gnames(7)
-
- curses.echo()
- self.screen.addstr(3, 0, "Pid [0 or pid]: ")
- pid = self.screen.getstr().decode(ENCODING)
- curses.noecho()
-
- try:
- if len(pid) > 0:
- pid = int(pid)
- if pid != 0 and not os.path.isdir(os.path.join('/proc/',
- str(pid))):
- msg = '"' + str(pid) + '": Not a running process'
- continue
- else:
- pid = 0
- self._refresh_header(pid)
- self._update_pid(pid)
- break
- except ValueError:
- msg = '"' + str(pid) + '": Not a valid pid'
-
def _show_set_update_interval(self):
"""Draws update interval selection mask."""
msg = ''
@@ -1272,17 +1236,17 @@ class Tui(object):
msg = '"' + str(val) + '": Invalid value'
self._refresh_header()
- def _show_vm_selection_by_guest_name(self):
+ def _show_vm_selection_by_guest(self):
"""Draws guest selection mask.
- Asks for a guest name until a valid guest name or '' is entered.
+ Asks for a guest name or pid until a valid guest name or '' is entered.
"""
msg = ''
while True:
self.screen.erase()
self.screen.addstr(0, 0,
- 'Show statistics for specific guest.',
+ 'Show statistics for specific guest or pid.',
curses.A_BOLD)
self.screen.addstr(1, 0,
'This might limit the shown data to the trace '
@@ -1290,32 +1254,39 @@ class Tui(object):
self.screen.addstr(5, 0, msg)
self._print_all_gnames(7)
curses.echo()
- self.screen.addstr(3, 0, "Guest [ENTER or guest]: ")
- gname = self.screen.getstr().decode(ENCODING)
+ curses.curs_set(1)
+ self.screen.addstr(3, 0, "Guest or pid [ENTER exits]: ")
+ guest = self.screen.getstr().decode(ENCODING)
curses.noecho()
- if not gname:
- self._refresh_header(0)
- self._update_pid(0)
+ pid = 0
+ if not guest or guest == '0':
break
- else:
- pids = []
- try:
- pids = self.get_pid_from_gname(gname)
- except:
- msg = '"' + gname + '": Internal error while searching, ' \
- 'use pid filter instead'
- continue
- if len(pids) == 0:
- msg = '"' + gname + '": Not an active guest'
+ if guest.isdigit():
+ if not os.path.isdir(os.path.join('/proc/', guest)):
+ msg = '"' + guest + '": Not a running process'
continue
- if len(pids) > 1:
- msg = '"' + gname + '": Multiple matches found, use pid ' \
- 'filter instead'
- continue
- self._refresh_header(pids[0])
- self._update_pid(pids[0])
+ pid = int(guest)
break
+ pids = []
+ try:
+ pids = self.get_pid_from_gname(guest)
+ except:
+ msg = '"' + guest + '": Internal error while searching, ' \
+ 'use pid filter instead'
+ continue
+ if len(pids) == 0:
+ msg = '"' + guest + '": Not an active guest'
+ continue
+ if len(pids) > 1:
+ msg = '"' + guest + '": Multiple matches found, use pid ' \
+ 'filter instead'
+ continue
+ pid = pids[0]
+ break
+ curses.curs_set(0)
+ self._refresh_header(pid)
+ self._update_pid(pid)
def show_stats(self):
"""Refreshes the screen and processes user input."""
@@ -1347,20 +1318,13 @@ class Tui(object):
self._show_filter_selection()
curses.curs_set(0)
sleeptime = self._delay_initial
- if char == 'g':
- curses.curs_set(1)
- self._show_vm_selection_by_guest_name()
- curses.curs_set(0)
+ if char == 'g' or char == 'p':
+ self._show_vm_selection_by_guest()
sleeptime = self._delay_initial
if char == 'h':
self._show_help_interactive()
if char == 'o':
self._sorting = not self._sorting
- if char == 'p':
- curses.curs_set(1)
- self._show_vm_selection_by_pid()
- curses.curs_set(0)
- sleeptime = self._delay_initial
if char == 'q':
break
if char == 'r':
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index b5b3810c9e94..0811d860fe75 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -35,13 +35,13 @@ INTERACTIVE COMMANDS
*f*:: filter by regular expression
-*g*:: filter by guest name
+*g*:: filter by guest name/PID
*h*:: display interactive commands reference
*o*:: toggle sorting order (Total vs CurAvg/s)
-*p*:: filter by PID
+*p*:: filter by guest name/PID
*q*:: quit
--
2.13.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/6] tools/kvm_stat: cache compiled regular expression
2018-02-14 21:45 [PATCH 0/6] tools/kvm_stat improvements Stefan Raspl
2018-02-14 21:45 ` [PATCH 1/6] tools/kvm_stat: mark private methods as such Stefan Raspl
2018-02-14 21:45 ` [PATCH 2/6] tools/kvm_stat: eliminate extra guest/pid selection dialog Stefan Raspl
@ 2018-02-14 21:45 ` Stefan Raspl
2018-02-15 7:32 ` Janosch Frank
2018-02-14 21:45 ` [PATCH 4/6] tools/kvm_stat: separate drilldown and fields filtering Stefan Raspl
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Stefan Raspl @ 2018-02-14 21:45 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
From: Stefan Raspl <stefan.raspl@de.ibm.com>
Should save us a couple of cycles.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index d7c80ac0374d..9f7e21cd5d19 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -480,7 +480,7 @@ class Provider(object):
"""Indicate whether field is valid according to fields_filter."""
if not fields_filter:
return True
- return re.match(fields_filter, field) is not None
+ return fields_filter.match(field) is not None
@staticmethod
def walkdir(path):
@@ -811,9 +811,10 @@ class Stats(object):
"""
def __init__(self, options):
- self.providers = self._get_providers(options)
self._pid_filter = options.pid
self._fields_filter = options.fields
+ self._fields_filter_comp = re.compile(options.fields)
+ self.providers = self._get_providers(options)
self.values = {}
def _get_providers(self, options):
@@ -821,10 +822,12 @@ class Stats(object):
providers = []
if options.debugfs:
- providers.append(DebugfsProvider(options.pid, options.fields,
+ providers.append(DebugfsProvider(self._pid_filter,
+ self._fields_filter_comp,
options.dbgfs_include_past))
if options.tracepoints or not providers:
- providers.append(TracepointProvider(options.pid, options.fields))
+ providers.append(TracepointProvider(self._pid_filter,
+ self._fields_filter_comp))
return providers
@@ -834,7 +837,7 @@ class Stats(object):
# also clear the cache of old values.
self.values = {}
for provider in self.providers:
- provider.update_fields(self._fields_filter)
+ provider.update_fields(self._fields_filter_comp)
def reset(self):
self.values = {}
@@ -849,6 +852,7 @@ class Stats(object):
def fields_filter(self, fields_filter):
if fields_filter != self._fields_filter:
self._fields_filter = fields_filter
+ self._fields_filter_comp = re.compile(fields_filter)
self._update_provider_filters()
@property
@@ -1040,7 +1044,7 @@ class Tui(object):
self.stats.fields_filter = DEFAULT_REGEX
elif self.stats.fields_filter == DEFAULT_REGEX:
- self.stats.fields_filter = None
+ self.stats.fields_filter = ''
def _update_pid(self, pid):
"""Propagates pid selection to stats object."""
@@ -1548,7 +1552,7 @@ def main():
stats = Stats(options)
if options.fields == 'help':
- stats.fields_filter = None
+ stats.fields_filter = ''
event_list = []
for key in stats.get().keys():
event_list.append(key.split('(', 1)[0])
--
2.13.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/6] tools/kvm_stat: separate drilldown and fields filtering
2018-02-14 21:45 [PATCH 0/6] tools/kvm_stat improvements Stefan Raspl
` (2 preceding siblings ...)
2018-02-14 21:45 ` [PATCH 3/6] tools/kvm_stat: cache compiled regular expression Stefan Raspl
@ 2018-02-14 21:45 ` Stefan Raspl
2018-02-14 21:45 ` [PATCH 5/6] tools/kvm_stat: group child events indented after parent Stefan Raspl
2018-02-14 21:45 ` [PATCH 6/6] tools/kvm_stat: print 'Total' line for multiple events only Stefan Raspl
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Raspl @ 2018-02-14 21:45 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
From: Stefan Raspl <stefan.raspl@de.ibm.com>
Drilldown (i.e. toggle display of child trace events) was implemented by
overriding the fields filter. This resulted in inconsistencies: E.g. when
drilldown was not active, adding a filter that also matches child trace
events would not only filter fields according to the filter, but also add
in the child trace events matching the filter. E.g. on x86, setting
'kvm_userspace_exit' as the fields filter after startup would result in
display of kvm_userspace_exit(DCR), although that wasn't previously
present - not exactly what one would expect from a filter.
This patch addresses the issue by keeping drilldown and fields filter
separate. While at it, we also fix a PEP8 issue by adding a blank line
at one place (since we're in the area...).
We implement this by adding a framework that also allows to define a
taxonomy among the debugfs events to identify child trace events. I.e.
drilldown using 'x' can now also work with debugfs. A respective parent-
child relationship is only known for S390 at the moment, but could be
added adjusting other platforms' ARCH.dbg_is_child() methods
accordingly.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 138 +++++++++++++++++++++++++++++++-------------
1 file changed, 97 insertions(+), 41 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 9f7e21cd5d19..bcd7327ad2d0 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -228,6 +228,7 @@ IOCTL_NUMBERS = {
}
ENCODING = locale.getpreferredencoding(False)
+TRACE_FILTER = re.compile(r'^[^\(]*$')
class Arch(object):
@@ -260,6 +261,11 @@ class Arch(object):
return ArchX86(SVM_EXIT_REASONS)
return
+ def tracepoint_is_child(self, field):
+ if (TRACE_FILTER.match(field)):
+ return None
+ return field.split('(', 1)[0]
+
class ArchX86(Arch):
def __init__(self, exit_reasons):
@@ -267,6 +273,10 @@ class ArchX86(Arch):
self.ioctl_numbers = IOCTL_NUMBERS
self.exit_reasons = exit_reasons
+ def debugfs_is_child(self, field):
+ """ Returns name of parent if 'field' is a child, None otherwise """
+ return None
+
class ArchPPC(Arch):
def __init__(self):
@@ -282,6 +292,10 @@ class ArchPPC(Arch):
self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16
self.exit_reasons = {}
+ def debugfs_is_child(self, field):
+ """ Returns name of parent if 'field' is a child, None otherwise """
+ return None
+
class ArchA64(Arch):
def __init__(self):
@@ -289,6 +303,10 @@ class ArchA64(Arch):
self.ioctl_numbers = IOCTL_NUMBERS
self.exit_reasons = AARCH64_EXIT_REASONS
+ def debugfs_is_child(self, field):
+ """ Returns name of parent if 'field' is a child, None otherwise """
+ return None
+
class ArchS390(Arch):
def __init__(self):
@@ -296,6 +314,12 @@ class ArchS390(Arch):
self.ioctl_numbers = IOCTL_NUMBERS
self.exit_reasons = None
+ def debugfs_is_child(self, field):
+ """ Returns name of parent if 'field' is a child, None otherwise """
+ if field.startswith('instruction_'):
+ return 'exit_instruction'
+
+
ARCH = Arch.get_arch()
@@ -475,6 +499,10 @@ class Event(object):
class Provider(object):
"""Encapsulates functionalities used by all providers."""
+ def __init__(self, pid):
+ self.child_events = False
+ self.pid = pid
+
@staticmethod
def is_field_wanted(fields_filter, field):
"""Indicate whether field is valid according to fields_filter."""
@@ -502,7 +530,7 @@ class TracepointProvider(Provider):
self.group_leaders = []
self.filters = self._get_filters()
self.update_fields(fields_filter)
- self.pid = pid
+ super(TracepointProvider, self).__init__(pid)
@staticmethod
def _get_filters():
@@ -522,7 +550,7 @@ class TracepointProvider(Provider):
return filters
def _get_available_fields(self):
- """Returns a list of available event's of format 'event name(filter
+ """Returns a list of available events of format 'event name(filter
name)'.
All available events have directories under
@@ -550,7 +578,8 @@ class TracepointProvider(Provider):
def update_fields(self, fields_filter):
"""Refresh fields, applying fields_filter"""
self.fields = [field for field in self._get_available_fields()
- if self.is_field_wanted(fields_filter, field)]
+ if self.is_field_wanted(fields_filter, field) or
+ ARCH.tracepoint_is_child(field)]
@staticmethod
def _get_online_cpus():
@@ -671,8 +700,12 @@ class TracepointProvider(Provider):
ret = defaultdict(int)
for group in self.group_leaders:
for name, val in group.read().items():
- if name in self._fields:
- ret[name] += val
+ if name not in self._fields:
+ continue
+ parent = ARCH.tracepoint_is_child(name)
+ if parent:
+ name += ' ' + parent
+ ret[name] += val
return ret
def reset(self):
@@ -690,7 +723,7 @@ class DebugfsProvider(Provider):
self._baseline = {}
self.do_read = True
self.paths = []
- self.pid = pid
+ super(DebugfsProvider, self).__init__(pid)
if include_past:
self._restore()
@@ -705,7 +738,8 @@ class DebugfsProvider(Provider):
def update_fields(self, fields_filter):
"""Refresh fields, applying fields_filter"""
self._fields = [field for field in self._get_available_fields()
- if self.is_field_wanted(fields_filter, field)]
+ if self.is_field_wanted(fields_filter, field) or
+ ARCH.debugfs_is_child(field)]
@property
def fields(self):
@@ -766,14 +800,15 @@ class DebugfsProvider(Provider):
self._baseline[key] = 0
if self._baseline.get(key, -1) == -1:
self._baseline[key] = value
- increment = (results.get(field, 0) + value -
- self._baseline.get(key, 0))
- if by_guest:
- pid = key.split('-')[0]
- if pid in results:
- results[pid] += increment
- else:
- results[pid] = increment
+ parent = ARCH.debugfs_is_child(field)
+ if parent:
+ field = field + ' ' + parent
+ else:
+ if by_guest:
+ field = key.split('-')[0] # set 'field' to 'pid'
+ increment = value - self._baseline.get(key, 0)
+ if field in results:
+ results[field] += increment
else:
results[field] = increment
@@ -816,6 +851,7 @@ class Stats(object):
self._fields_filter_comp = re.compile(options.fields)
self.providers = self._get_providers(options)
self.values = {}
+ self._child_events = False
def _get_providers(self, options):
"""Returns a list of data providers depending on the passed options."""
@@ -867,12 +903,29 @@ class Stats(object):
for provider in self.providers:
provider.pid = self._pid_filter
+ @property
+ def child_events(self):
+ return self._child_events
+
+ @child_events.setter
+ def child_events(self, val):
+ self._child_events = val
+ for provider in self.providers:
+ provider.child_events = val
+
def get(self, by_guest=0):
"""Returns a dict with field -> (value, delta to last value) of all
- provider data."""
+ provider data.
+ Key formats:
+ * plain: 'key' is event name
+ * child-parent: 'key' is in format '<child> <parent>'
+ * pid: 'key' is the pid of the guest, and the record contains the
+ aggregated event data
+ These formats are generated by the providers, and handled in class TUI.
+ """
for provider in self.providers:
new = provider.read(by_guest=by_guest)
- for key in new if by_guest else provider.fields:
+ for key in new:
oldval = self.values.get(key, EventStat(0, 0)).value
newval = new.get(key, 0)
newdelta = newval - oldval
@@ -905,10 +958,10 @@ class Stats(object):
self.get(to_pid)
return 0
+
DELAY_DEFAULT = 3.0
MAX_GUEST_NAME_LEN = 48
MAX_REGEX_LEN = 44
-DEFAULT_REGEX = r'^[^\(]*$'
SORT_DEFAULT = 0
@@ -1038,14 +1091,6 @@ class Tui(object):
return name
- def _update_drilldown(self):
- """Sets or removes a filter that only allows fields without braces."""
- if not self.stats.fields_filter:
- self.stats.fields_filter = DEFAULT_REGEX
-
- elif self.stats.fields_filter == DEFAULT_REGEX:
- self.stats.fields_filter = ''
-
def _update_pid(self, pid):
"""Propagates pid selection to stats object."""
self.screen.addstr(4, 1, 'Updating pid filter...')
@@ -1067,8 +1112,7 @@ class Tui(object):
.format(pid, gname), curses.A_BOLD)
else:
self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD)
- if self.stats.fields_filter and self.stats.fields_filter \
- != DEFAULT_REGEX:
+ if self.stats.fields_filter:
regex = self.stats.fields_filter
if len(regex) > MAX_REGEX_LEN:
regex = regex[:MAX_REGEX_LEN] + '...'
@@ -1084,6 +1128,9 @@ class Tui(object):
self.screen.refresh()
def _refresh_body(self, sleeptime):
+ def is_child_field(field):
+ return field.find('(') != -1
+
row = 3
self.screen.move(row, 0)
self.screen.clrtobot()
@@ -1091,7 +1138,11 @@ class Tui(object):
total = 0.
ctotal = 0.
for key, values in stats.items():
- if key.find('(') == -1:
+ if self._display_guests:
+ if self.get_gname_from_pid(key):
+ total += values.value
+ continue
+ if not key.find(' ') != -1:
total += values.value
else:
ctotal += values.value
@@ -1108,19 +1159,26 @@ class Tui(object):
# sort by overall value
return v.value
+ sorted_items = sorted(stats.items(), key=sortkey, reverse=True)
+
+ # print events
tavg = 0
- for key, values in sorted(stats.items(), key=sortkey, reverse=True):
+ for key, values in sorted_items:
if row >= self.screen.getmaxyx()[0] - 1:
break
- if not values.value and not values.delta:
- break
+ if values == (0, 0):
+ continue
+ if not self.stats.child_events and key.find(' ') != -1:
+ continue
if values.value is not None:
cur = int(round(values.delta / sleeptime)) if values.delta else ''
if self._display_guests:
key = self.get_gname_from_pid(key)
- self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' %
- (key, values.value, values.value * 100 / total,
- cur))
+ if not key:
+ continue
+ self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % (key
+ .split(' ')[0], values.value,
+ values.value * 100 / total, cur))
if cur != '' and key.find('(') == -1:
tavg += cur
row += 1
@@ -1196,7 +1254,7 @@ class Tui(object):
regex = self.screen.getstr().decode(ENCODING)
curses.noecho()
if len(regex) == 0:
- self.stats.fields_filter = DEFAULT_REGEX
+ self.stats.fields_filter = ''
self._refresh_header()
return
try:
@@ -1314,7 +1372,7 @@ class Tui(object):
self._display_guests = not self._display_guests
self._refresh_header()
if char == 'c':
- self.stats.fields_filter = DEFAULT_REGEX
+ self.stats.fields_filter = ''
self._refresh_header(0)
self._update_pid(0)
if char == 'f':
@@ -1339,9 +1397,7 @@ class Tui(object):
curses.curs_set(0)
sleeptime = self._delay_initial
if char == 'x':
- self._update_drilldown()
- # prevents display of current values on next refresh
- self.stats.get(self._display_guests)
+ self.stats.child_events = not self.stats.child_events
except KeyboardInterrupt:
break
except curses.error:
@@ -1476,7 +1532,7 @@ Press any other key to refresh statistics immediately.
)
optparser.add_option('-f', '--fields',
action='store',
- default=DEFAULT_REGEX,
+ default='',
dest='fields',
help='''fields to display (regex)
"-f help" for a list of available events''',
--
2.13.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/6] tools/kvm_stat: group child events indented after parent
2018-02-14 21:45 [PATCH 0/6] tools/kvm_stat improvements Stefan Raspl
` (3 preceding siblings ...)
2018-02-14 21:45 ` [PATCH 4/6] tools/kvm_stat: separate drilldown and fields filtering Stefan Raspl
@ 2018-02-14 21:45 ` Stefan Raspl
2018-02-14 21:45 ` [PATCH 6/6] tools/kvm_stat: print 'Total' line for multiple events only Stefan Raspl
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Raspl @ 2018-02-14 21:45 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
From: Stefan Raspl <stefan.raspl@de.ibm.com>
We keep the current logic that sorts all events (parent and child), but
re-shuffle the events afterwards, grouping the children after the
respective parent. Note that the percentage column for child events
gives the percentage of the parent's total.
Since we rework the logic anyway, we modify the total average
calculation to use the raw numbers instead of the (rounded) averages.
Note that this can result in differing numbers (between total average
and the sum of the individual averages) due to rounding errors.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 89 ++++++++++++++++++++++++++++++---------------
1 file changed, 59 insertions(+), 30 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index bcd7327ad2d0..1d1c2aa23fbc 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -1131,6 +1131,45 @@ class Tui(object):
def is_child_field(field):
return field.find('(') != -1
+ def insert_child(sorted_items, child, values, parent):
+ num = len(sorted_items)
+ for i in range(0, num):
+ # only add child if parent is present
+ if parent.startswith(sorted_items[i][0]):
+ sorted_items.insert(i + 1, (' ' + child, values))
+
+ def get_sorted_events(self, stats):
+ """ separate parent and child events """
+ if self._sorting == SORT_DEFAULT:
+ def sortkey((_k, v)):
+ # sort by (delta value, overall value)
+ return (v.delta, v.value)
+ else:
+ def sortkey((_k, v)):
+ # sort by overall value
+ return v.value
+
+ childs = []
+ sorted_items = []
+ # we can't rule out child events to appear prior to parents even
+ # when sorted - separate out all children first, and add in later
+ for key, values in sorted(stats.items(), key=sortkey,
+ reverse=True):
+ if values == (0, 0):
+ continue
+ if key.find(' ') != -1:
+ if not self.stats.child_events:
+ continue
+ childs.insert(0, (key, values))
+ else:
+ sorted_items.append((key, values))
+ if self.stats.child_events:
+ for key, values in childs:
+ (child, parent) = key.split(' ')
+ insert_child(sorted_items, child, values, parent)
+
+ return sorted_items
+
row = 3
self.screen.move(row, 0)
self.screen.clrtobot()
@@ -1150,44 +1189,34 @@ class Tui(object):
# we don't have any fields, or all non-child events are filtered
total = ctotal
- if self._sorting == SORT_DEFAULT:
- def sortkey((_k, v)):
- # sort by (delta value, overall value)
- return (v.delta, v.value)
- else:
- def sortkey((_k, v)):
- # sort by overall value
- return v.value
-
- sorted_items = sorted(stats.items(), key=sortkey, reverse=True)
-
# print events
tavg = 0
- for key, values in sorted_items:
- if row >= self.screen.getmaxyx()[0] - 1:
+ tcur = 0
+ for key, values in get_sorted_events(self, stats):
+ if row >= self.screen.getmaxyx()[0] - 1 or values == (0, 0):
break
- if values == (0, 0):
- continue
- if not self.stats.child_events and key.find(' ') != -1:
- continue
- if values.value is not None:
- cur = int(round(values.delta / sleeptime)) if values.delta else ''
- if self._display_guests:
- key = self.get_gname_from_pid(key)
- if not key:
- continue
- self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % (key
- .split(' ')[0], values.value,
- values.value * 100 / total, cur))
- if cur != '' and key.find('(') == -1:
- tavg += cur
+ if self._display_guests:
+ key = self.get_gname_from_pid(key)
+ if not key:
+ continue
+ cur = int(round(values.delta / sleeptime)) if values.delta else ''
+ if key[0] != ' ':
+ if values.delta:
+ tcur += values.delta
+ ptotal = values.value
+ ltotal = total
+ else:
+ ltotal = ptotal
+ self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % (key,
+ values.value,
+ values.value * 100 / float(ltotal), cur))
row += 1
if row == 3:
self.screen.addstr(4, 1, 'No matching events reported yet')
else:
+ tavg = int(round(tcur / sleeptime)) if tcur > 0 else ''
self.screen.addstr(row, 1, '%-40s %10d %8s' %
- ('Total', total, tavg if tavg else ''),
- curses.A_BOLD)
+ ('Total', total, tavg), curses.A_BOLD)
self.screen.refresh()
def _show_msg(self, text):
--
2.13.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 6/6] tools/kvm_stat: print 'Total' line for multiple events only
2018-02-14 21:45 [PATCH 0/6] tools/kvm_stat improvements Stefan Raspl
` (4 preceding siblings ...)
2018-02-14 21:45 ` [PATCH 5/6] tools/kvm_stat: group child events indented after parent Stefan Raspl
@ 2018-02-14 21:45 ` Stefan Raspl
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Raspl @ 2018-02-14 21:45 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
From: Stefan Raspl <stefan.raspl@de.ibm.com>
The 'Total' line looks a bit weird when we have a single event only. This
can happen e.g. due to filters. Therefore suppress when there's only a
single event in the output.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 1d1c2aa23fbc..d6aa2dfc6825 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -1213,7 +1213,7 @@ class Tui(object):
row += 1
if row == 3:
self.screen.addstr(4, 1, 'No matching events reported yet')
- else:
+ if row > 4:
tavg = int(round(tcur / sleeptime)) if tcur > 0 else ''
self.screen.addstr(row, 1, '%-40s %10d %8s' %
('Total', total, tavg), curses.A_BOLD)
--
2.13.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/6] tools/kvm_stat: cache compiled regular expression
2018-02-14 21:45 ` [PATCH 3/6] tools/kvm_stat: cache compiled regular expression Stefan Raspl
@ 2018-02-15 7:32 ` Janosch Frank
2018-02-15 8:14 ` Stefan Raspl
0 siblings, 1 reply; 9+ messages in thread
From: Janosch Frank @ 2018-02-15 7:32 UTC (permalink / raw)
To: Stefan Raspl, kvm; +Cc: pbonzini, rkrcmar
[-- Attachment #1.1: Type: text/plain, Size: 3693 bytes --]
On 14.02.2018 22:45, Stefan Raspl wrote:
> From: Stefan Raspl <stefan.raspl@de.ibm.com>
>
> Should save us a couple of cycles.
Are you sure, the wiki says otherwise?
Note
The compiled versions of the most recent patterns passed to re.match(),
re.search() or re.compile() are cached, so programs that use only a few
regular expressions at a time needn’t worry about compiling regular
expressions.
>
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
> ---
> tools/kvm/kvm_stat/kvm_stat | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index d7c80ac0374d..9f7e21cd5d19 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -480,7 +480,7 @@ class Provider(object):
> """Indicate whether field is valid according to fields_filter."""
> if not fields_filter:
> return True
> - return re.match(fields_filter, field) is not None
> + return fields_filter.match(field) is not None
>
> @staticmethod
> def walkdir(path):
> @@ -811,9 +811,10 @@ class Stats(object):
>
> """
> def __init__(self, options):
> - self.providers = self._get_providers(options)
> self._pid_filter = options.pid
> self._fields_filter = options.fields
> + self._fields_filter_comp = re.compile(options.fields)
> + self.providers = self._get_providers(options)
> self.values = {}
>
> def _get_providers(self, options):
> @@ -821,10 +822,12 @@ class Stats(object):
> providers = []
>
> if options.debugfs:
> - providers.append(DebugfsProvider(options.pid, options.fields,
> + providers.append(DebugfsProvider(self._pid_filter,
> + self._fields_filter_comp,
> options.dbgfs_include_past))
> if options.tracepoints or not providers:
> - providers.append(TracepointProvider(options.pid, options.fields))
> + providers.append(TracepointProvider(self._pid_filter,
> + self._fields_filter_comp))
>
> return providers
>
> @@ -834,7 +837,7 @@ class Stats(object):
> # also clear the cache of old values.
> self.values = {}
> for provider in self.providers:
> - provider.update_fields(self._fields_filter)
> + provider.update_fields(self._fields_filter_comp)
>
> def reset(self):
> self.values = {}
> @@ -849,6 +852,7 @@ class Stats(object):
> def fields_filter(self, fields_filter):
> if fields_filter != self._fields_filter:
> self._fields_filter = fields_filter
> + self._fields_filter_comp = re.compile(fields_filter)
> self._update_provider_filters()
>
> @property
> @@ -1040,7 +1044,7 @@ class Tui(object):
> self.stats.fields_filter = DEFAULT_REGEX
>
> elif self.stats.fields_filter == DEFAULT_REGEX:
> - self.stats.fields_filter = None
> + self.stats.fields_filter = ''
>
> def _update_pid(self, pid):
> """Propagates pid selection to stats object."""
> @@ -1548,7 +1552,7 @@ def main():
> stats = Stats(options)
>
> if options.fields == 'help':
> - stats.fields_filter = None
> + stats.fields_filter = ''
> event_list = []
> for key in stats.get().keys():
> event_list.append(key.split('(', 1)[0])
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/6] tools/kvm_stat: cache compiled regular expression
2018-02-15 7:32 ` Janosch Frank
@ 2018-02-15 8:14 ` Stefan Raspl
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Raspl @ 2018-02-15 8:14 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: pbonzini, rkrcmar
On 15.02.2018 08:32, Janosch Frank wrote:
> On 14.02.2018 22:45, Stefan Raspl wrote:
>> From: Stefan Raspl <stefan.raspl@de.ibm.com>
>>
>> Should save us a couple of cycles.
>
> Are you sure, the wiki says otherwise?
>
>
> Note
>
> The compiled versions of the most recent patterns passed to re.match(),
> re.search() or re.compile() are cached, so programs that use only a few
> regular expressions at a time needn’t worry about compiling regular
> expressions.
True - must have missed that note :O
In that case, we could probably do without this patch.
Ciao,
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-15 8:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-14 21:45 [PATCH 0/6] tools/kvm_stat improvements Stefan Raspl
2018-02-14 21:45 ` [PATCH 1/6] tools/kvm_stat: mark private methods as such Stefan Raspl
2018-02-14 21:45 ` [PATCH 2/6] tools/kvm_stat: eliminate extra guest/pid selection dialog Stefan Raspl
2018-02-14 21:45 ` [PATCH 3/6] tools/kvm_stat: cache compiled regular expression Stefan Raspl
2018-02-15 7:32 ` Janosch Frank
2018-02-15 8:14 ` Stefan Raspl
2018-02-14 21:45 ` [PATCH 4/6] tools/kvm_stat: separate drilldown and fields filtering Stefan Raspl
2018-02-14 21:45 ` [PATCH 5/6] tools/kvm_stat: group child events indented after parent Stefan Raspl
2018-02-14 21:45 ` [PATCH 6/6] tools/kvm_stat: print 'Total' line for multiple events only Stefan Raspl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox