Messages in this thread | | | From | Ian Rogers <> | Date | Wed, 10 Jan 2024 08:11:19 -0800 | Subject | Re: [PATCH v2 1/2] perf tui: don't ignore job control |
| |
On Thu, Jan 4, 2024 at 1:23 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Tue, Dec 27, 2022 at 09:57:40PM +0100, Ahelenia Ziemiańska escreveu: > > In its infinite wisdom, by default, SLang sets susp undef, > > and this can only be un-done by calling SLtty_set_suspend_state(true). > > After every SLang_init_tty(). > > > > Additionally, no provisions are made for maintaining the teletype > > attributes across suspend/continue (outside of curses emulation mode(?!), > > which provides full support, naturally), so we need to save and restore > > the flags ourselves, as well as reset the text colours when going under. > > We need to also re-draw the screen, and raising SIGWINCH, shockingly, > > Just Works. > > > > The correct solution would be to Not Use SLang, but as a stop-gap, > > this makes TUI perf report usable. > > This was in the compost pile for a long time, but I got to it, indeed > 'perf report' works, the refresh after returning to it, etc. > > 'perf top' needs some care, but we can do it later. > > About not using slang, perf started with newt, I moved to slang, there > are people suggesting we move to some more modern thing, maybe > notcurses?
Fwiw, my feeling is that doing UI stuff in C is a particular pain. Maybe there will be a move to allow C++ in the kernel and so perf could follow [1]. For C++ the most maintained/best text UI looks like ftxui [2], but what I think looks best is a python library called textual [3] that already has support for things like flamegraphs [4] as well as having a HTML/JavaScript output/interaction option [5]. So my feeling is that if we're rebuilding the UI we should be moving more to python+textual than a C/C++ library that gets a fraction of the care.
Thanks, Ian
[1] https://www.phoronix.com/news/CPP-Linux-Kernel-2024-Discuss [2] https://github.com/ArthurSonzogni/FTXUI [3] https://github.com/Textualize/textual [4] https://github.com/Textualize/textual/discussions/3390 [5] https://github.com/Textualize/textual-web
> - Arnaldo > > > Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> > > --- > > Yeah, somehow I didn't hit that in my tests, but it was relatively easy > > to trigger when I tried. A full partial write retry loop is an overkill > > here, I think, and few other places in perf actually try to resubmit > > partial writes, so. > > > > tools/perf/ui/browsers/annotate.c | 1 + > > tools/perf/ui/browsers/hists.c | 2 ++ > > tools/perf/ui/browsers/scripts.c | 1 + > > tools/perf/ui/tui/setup.c | 22 ++++++++++++++++++++++ > > 4 files changed, 26 insertions(+) > > > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c > > index c03fa76c02ff..6a4ffbf66c7f 100644 > > --- a/tools/perf/ui/browsers/annotate.c > > +++ b/tools/perf/ui/browsers/annotate.c > > @@ -942,6 +942,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel, > > /* reset abort key so that it can get Ctrl-C as a key */ > > SLang_reset_tty(); > > SLang_init_tty(0, 0, 0); > > + SLtty_set_suspend_state(true); > > > > return map_symbol__tui_annotate(&he->ms, evsel, hbt, opts); > > } > > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c > > index b72ee6822222..2479e6d42e7c 100644 > > --- a/tools/perf/ui/browsers/hists.c > > +++ b/tools/perf/ui/browsers/hists.c > > @@ -3010,6 +3010,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h > > /* reset abort key so that it can get Ctrl-C as a key */ > > SLang_reset_tty(); > > SLang_init_tty(0, 0, 0); > > + SLtty_set_suspend_state(true); > > > > if (min_pcnt) > > browser->min_pcnt = min_pcnt; > > @@ -3682,6 +3683,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel, > > /* reset abort key so that it can get Ctrl-C as a key */ > > SLang_reset_tty(); > > SLang_init_tty(0, 0, 0); > > + SLtty_set_suspend_state(true); > > > > memset(&action, 0, sizeof(action)); > > > > diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c > > index 47d2c7a8cbe1..50d45054ed6c 100644 > > --- a/tools/perf/ui/browsers/scripts.c > > +++ b/tools/perf/ui/browsers/scripts.c > > @@ -166,6 +166,7 @@ void run_script(char *cmd) > > printf("\033[c\033[H\033[J"); > > fflush(stdout); > > SLang_init_tty(0, 0, 0); > > + SLtty_set_suspend_state(true); > > SLsmg_refresh(); > > } > > > > diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c > > index a3b8c397c24d..09524ba04975 100644 > > --- a/tools/perf/ui/tui/setup.c > > +++ b/tools/perf/ui/tui/setup.c > > @@ -2,12 +2,14 @@ > > #include <signal.h> > > #include <stdbool.h> > > #include <stdlib.h> > > +#include <termios.h> > > #include <unistd.h> > > #include <linux/kernel.h> > > #ifdef HAVE_BACKTRACE_SUPPORT > > #include <execinfo.h> > > #endif > > > > +#include "../../util/color.h" > > #include "../../util/debug.h" > > #include "../../perf.h" > > #include "../browser.h" > > @@ -122,6 +124,23 @@ static void ui__signal(int sig) > > exit(0); > > } > > > > +static void ui__sigcont(int sig) > > +{ > > + static struct termios tty; > > + > > + if (sig == SIGTSTP) { > > + while (tcgetattr(SLang_TT_Read_FD, &tty) == -1 && errno == EINTR) > > + ; > > + while (write(SLang_TT_Read_FD, PERF_COLOR_RESET, sizeof(PERF_COLOR_RESET) - 1) == -1 && errno == EINTR) > > + ; > > + raise(SIGSTOP); > > + } else { > > + while (tcsetattr(SLang_TT_Read_FD, TCSADRAIN, &tty) == -1 && errno == EINTR) > > + ; > > + raise(SIGWINCH); > > + } > > +} > > + > > int ui__init(void) > > { > > int err; > > @@ -136,6 +155,7 @@ int ui__init(void) > > err = SLang_init_tty(-1, 0, 0); > > if (err < 0) > > goto out; > > + SLtty_set_suspend_state(true); > > > > err = SLkp_init(); > > if (err < 0) { > > @@ -150,6 +170,8 @@ int ui__init(void) > > signal(SIGINT, ui__signal); > > signal(SIGQUIT, ui__signal); > > signal(SIGTERM, ui__signal); > > + signal(SIGTSTP, ui__sigcont); > > + signal(SIGCONT, ui__sigcont); > > > > perf_error__register(&perf_tui_eops); > > > > -- > > 2.30.2 > > > > > > -- > > - Arnaldo >
| |