When in perf mode (so not polling) previously the GUI would be refreshed at the arrival of any single perf event. This may mean that it would get refreshed all the time especially on quite loaded systems, causing very high CPU load and unuseable GUI. The patch limits the refresh to the minimum refresh_time (already used for polling and configurable via -R command line by the user) to prevent such cases. If no events arrive, the GUI will still not be refreshed at all as before. Signed-off-by: Federico Pellegrin <fede(a)evolware.org&gt; --- tuna/gui/procview.py | 14 ++++++++++---- tuna/tuna_gui.py | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tuna/gui/procview.py b/tuna/gui/procview.py index c224e4b..dc2e94e 100755 --- a/tuna/gui/procview.py +++ b/tuna/gui/procview.py @@ -219,6 +219,7 @@ class procview: self.treeview = treeview self.nr_cpus = procfs.cpuinfo().nr_cpus self.gladefile = gladefile + self.evlist_added = True self.evlist = None if not disable_perf: @@ -347,7 +348,7 @@ class procview: else: self.perf_counter[tid] = event.sample_period - self.show() + self.evlist_added = True # Mark that event arrived, so next periodic show() will refresh GUI return True def perf_init(self): @@ -426,9 +427,14 @@ class procview: # create the rows. if not self.refreshing and not force_refresh: return - row = self.tree_store.get_iter_first() - self.update_rows(self.ps, row, None) - self.treeview.show_all() + + # If using perf only refresh if we saw at least a new event since last refresh + if not self.evlist or self.evlist_added: + self.evlist_added = None + + row = self.tree_store.get_iter_first() + self.update_rows(self.ps, row, None) + self.treeview.show_all() def update_rows(self, threads, row, parent_row): new_tids = list(threads.keys()) diff --git a/tuna/tuna_gui.py b/tuna/tuna_gui.py index 83af063..f1f2caa 100755 --- a/tuna/tuna_gui.py +++ b/tuna/tuna_gui.py @@ -146,7 +146,7 @@ class main_gui: if not self.procview.evlist: # Poll, as we don't have perf self.ps.reload() self.ps.reload_threads() - self.procview.show() + self.procview.show() self.irqview.refresh() return True -- 2.26.3

Hi John, Thanks for the feedback! This patch should be applied on the top of the 9/9 of the previous bunch (this one: https://lists.fedorahosted.org/archives/list/tuna-devel@lists.fedorahoste... ) that is still pending, as I was assuming that could go in too ([PATCH 9/9] tuna_gui: add command line option to explicitly disable perf usage). I believe if you apply that too, then it should be fine. In my opinion that 9/9 patch should be still useful, as it covers still some use cases, depending what information one is after (since ie. when in perf mode you don't get context switches or so), which cannot be fulfilled if we force always the user to go (when automatically detected on the system) in "perf mode". Of course if you don't agree with that one (or if applying both still creates problems), please just let me know and I'll immediately submit a new version of this patch that doesn't rely it, so it can be merged cleanly. Thanks! Federico Il giorno mar 13 apr 2021 alle ore 22:38 John Kacur <jkacur(a)redhat.com&gt; ha scritto: > > > On Thu, 25 Mar 2021, Federico Pellegrin wrote: > > > When in perf mode (so not polling) previously the GUI would be > > refreshed at the arrival of any single perf event. This may mean > > that it would get refreshed all the time especially on quite loaded > > systems, causing very high CPU load and unuseable GUI. > > > > The patch limits the refresh to the minimum refresh_time (already > > used for polling and configurable via -R command line by the user) > > to prevent such cases. If no events arrive, the GUI will still not > > be refreshed at all as before. > > > > Signed-off-by: Federico Pellegrin <fede(a)evolware.org&gt; > > --- > > tuna/gui/procview.py | 14 ++++++++++---- > > tuna/tuna_gui.py | 2 +- > > 2 files changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/tuna/gui/procview.py b/tuna/gui/procview.py > > index c224e4b..dc2e94e 100755 > > --- a/tuna/gui/procview.py > > +++ b/tuna/gui/procview.py > > @@ -219,6 +219,7 @@ class procview: > > self.treeview = treeview > > self.nr_cpus = procfs.cpuinfo().nr_cpus > > self.gladefile = gladefile > > + self.evlist_added = True > > > > self.evlist = None > > if not disable_perf: > > @@ -347,7 +348,7 @@ class procview: > > else: > > self.perf_counter[tid] = > event.sample_period > > > > - self.show() > > + self.evlist_added = True # Mark that event arrived, so next > periodic show() will refresh GUI > > return True > > > > def perf_init(self): > > @@ -426,9 +427,14 @@ class procview: > > # create the rows. > > if not self.refreshing and not force_refresh: > > return > > - row = self.tree_store.get_iter_first() > > - self.update_rows(self.ps, row, None) > > - self.treeview.show_all() > > + > > + # If using perf only refresh if we saw at least a new event > since last refresh > > + if not self.evlist or self.evlist_added: > > + self.evlist_added = None > > + > > + row = self.tree_store.get_iter_first() > > + self.update_rows(self.ps, row, None) > > + self.treeview.show_all() > > > > def update_rows(self, threads, row, parent_row): > > new_tids = list(threads.keys()) > > diff --git a/tuna/tuna_gui.py b/tuna/tuna_gui.py > > index 83af063..f1f2caa 100755 > > --- a/tuna/tuna_gui.py > > +++ b/tuna/tuna_gui.py > > @@ -146,7 +146,7 @@ class main_gui: > > if not self.procview.evlist: # Poll, as we don't have perf > > self.ps.reload() > > self.ps.reload_threads() > > - self.procview.show() > > + self.procview.show() > > self.irqview.refresh() > > return True > > > > -- > > 2.26.3 > > > > > > I like the idea but the patch does not apply cleanly, do you want to take > a look? > > Thanks > > John >

On Mon, 10 May 2021, Federico Pellegrin wrote: > > Hi John, > Sorry just a "ping" on this still pending patch (or better two patches if you agree with what I mentioned in the previous email). Don't be afraid to ping me, I appreciate it. I think it would be easier for me if you would [re]send any pending patches so that I can either apply them, or send some feedback. This would be easier than slogging through old mails. Thanks John > > Many thanks! > Federico > > > Il giorno mer 14 apr 2021 alle ore 05:15 Federico Pellegrin < fede(a)evolware.org&gt; ha scritto: > > Hi John, > Thanks for the feedback! > > This patch should be applied on the top of the 9/9 of the previous bunch (this one: > https://lists.fedorahosted.org/archives/list/tuna-devel@lists.fedorahoste... ) that is still pending, as I was assuming that could go in > too ([PATCH 9/9] tuna_gui: add command line option to explicitly disable perf usage). I believe if you apply that too, then it should be fine. > > In my opinion that 9/9 patch should be still useful, as it covers still some use cases, depending what information one is after (since ie. when in perf mode you don't get context > switches or so), which cannot be fulfilled if we force always the user to go (when automatically detected on the system) in "perf mode". > > Of course if you don't agree with that one (or if applying both still creates problems), please just let me know and I'll immediately submit a new version of this patch that > doesn't rely it, so it can be merged cleanly. > > Thanks! > Federico > > > > Il giorno mar 13 apr 2021 alle ore 22:38 John Kacur <jkacur(a)redhat.com&gt; ha scritto: > > > On Thu, 25 Mar 2021, Federico Pellegrin wrote: > > > When in perf mode (so not polling) previously the GUI would be > > refreshed at the arrival of any single perf event. This may mean > > that it would get refreshed all the time especially on quite loaded > > systems, causing very high CPU load and unuseable GUI. > > > > The patch limits the refresh to the minimum refresh_time (already > > used for polling and configurable via -R command line by the user) > > to prevent such cases. If no events arrive, the GUI will still not > > be refreshed at all as before. > > > > Signed-off-by: Federico Pellegrin <fede(a)evolware.org&gt; > > --- > > tuna/gui/procview.py | 14 ++++++++++---- > > tuna/tuna_gui.py | 2 +- > > 2 files changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/tuna/gui/procview.py b/tuna/gui/procview.py > > index c224e4b..dc2e94e 100755 > > --- a/tuna/gui/procview.py > > +++ b/tuna/gui/procview.py > > @@ -219,6 +219,7 @@ class procview: > > self.treeview = treeview > > self.nr_cpus = procfs.cpuinfo().nr_cpus > > self.gladefile = gladefile > > + self.evlist_added = True > > > > self.evlist = None > > if not disable_perf: > > @@ -347,7 +348,7 @@ class procview: > > else: > > self.perf_counter[tid] = event.sample_period > > > > - self.show() > > + self.evlist_added = True # Mark that event arrived, so next periodic show() will refresh GUI > > return True > > > > def perf_init(self): > > @@ -426,9 +427,14 @@ class procview: > > # create the rows. > > if not self.refreshing and not force_refresh: > > return > > - row = self.tree_store.get_iter_first() > > - self.update_rows(self.ps, row, None) > > - self.treeview.show_all() > > + > > + # If using perf only refresh if we saw at least a new event since last refresh > > + if not self.evlist or self.evlist_added: > > + self.evlist_added = None > > + > > + row = self.tree_store.get_iter_first() > > + self.update_rows(self.ps, row, None) > > + self.treeview.show_all() > > > > def update_rows(self, threads, row, parent_row): > > new_tids = list(threads.keys()) > > diff --git a/tuna/tuna_gui.py b/tuna/tuna_gui.py > > index 83af063..f1f2caa 100755 > > --- a/tuna/tuna_gui.py > > +++ b/tuna/tuna_gui.py > > @@ -146,7 +146,7 @@ class main_gui: > > if not self.procview.evlist: # Poll, as we don't have perf > > self.ps.reload() > > self.ps.reload_threads() > > - self.procview.show() > > + self.procview.show() > > self.irqview.refresh() > > return True > > > > -- > > 2.26.3 > > > > > > I like the idea but the patch does not apply cleanly, do you want to take > a look? > > Thanks > > John > > >