Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1728777 > unrolled thread

[RFC 1/4] perf ui progress: Add size info into progress bar

Back to article view | Back to linux.kernel

Contents

  [RFC 1/4] perf ui progress: Add size info into progress bar Jiri Olsa <jolsa@kernel.org> - 2017-09-08 14:10 +0200
    [PATCH 4/4] perf ui progress: Add size info into progress bar Jiri Olsa <jolsa@kernel.org> - 2017-09-08 14:10 +0200
    [PATCH 1/4] perf ui progress: Make sure we always define step value Jiri Olsa <jolsa@kernel.org> - 2017-09-08 14:10 +0200
      Re: [PATCH 1/4] perf ui progress: Make sure we always define step value Milian Wolff <milian.wolff@kdab.com> - 2017-09-08 15:20 +0200
        Re: [PATCH 1/4] perf ui progress: Make sure we always define step  value Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-09-08 15:30 +0200
          Re: [PATCH 1/4] perf ui progress: Make sure we always define step value Milian Wolff <milian.wolff@kdab.com> - 2017-09-09 12:20 +0200
    [PATCH 2/4] perf ui progress: Fix progress update Jiri Olsa <jolsa@kernel.org> - 2017-09-08 14:10 +0200
    [PATCH 3/4] perf ui progress: Add ui specific init function Jiri Olsa <jolsa@kernel.org> - 2017-09-08 14:10 +0200
    Re: [RFC 1/4] perf ui progress: Add size info into progress bar Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-09-08 15:50 +0200

#1728777 — [RFC 1/4] perf ui progress: Add size info into progress bar

hi,
sending some progress bar fixes.

thanks,
jirka

---
 tools/perf/ui/progress.c     | 15 ++++++++++++---
 tools/perf/ui/progress.h     | 12 +++++++++++-
 tools/perf/ui/tui/progress.c | 32 +++++++++++++++++++++++++++++---
 tools/perf/util/session.c    |  2 +-
 4 files changed, 53 insertions(+), 8 deletions(-)

[toc] | [next] | [standalone]

#1728778 — [PATCH 4/4] perf ui progress: Add size info into progress bar

Adding the size values '[current/total]' into progress bar,
to show more detailed progress of data reading.

Adding new ui_progress__init_size function to specify we
want to display the size.

Link: http://lkml.kernel.org/n/tip-39wn4d77phel3dgkzo3lyan0@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/progress.c     |  4 +++-
 tools/perf/ui/progress.h     | 11 ++++++++++-
 tools/perf/ui/tui/progress.c | 23 ++++++++++++++++++++++-
 tools/perf/util/session.c    |  2 +-
 4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/tools/perf/ui/progress.c b/tools/perf/ui/progress.c
index 3e2b5d64c55e..7ade387d511c 100644
--- a/tools/perf/ui/progress.c
+++ b/tools/perf/ui/progress.c
@@ -27,12 +27,14 @@ void ui_progress__update(struct ui_progress *p, u64 adv)
 	}
 }
 
-void ui_progress__init(struct ui_progress *p, u64 total, const char *title)
+void __ui_progress__init(struct ui_progress *p, u64 total,
+			 const char *title, bool size)
 {
 	p->curr = 0;
 	p->next = p->step = total / 16 ?: 1;
 	p->total = total;
 	p->title = title;
+	p->size  = size;
 
 	if (ui_progress__ops->init)
 		ui_progress__ops->init(p);
diff --git a/tools/perf/ui/progress.h b/tools/perf/ui/progress.h
index e5f434a2070b..fbaa1507ebfe 100644
--- a/tools/perf/ui/progress.h
+++ b/tools/perf/ui/progress.h
@@ -8,9 +8,18 @@ void ui_progress__finish(void);
 struct ui_progress {
 	const char *title;
 	u64 curr, next, step, total;
+	bool size;
 };
 
-void ui_progress__init(struct ui_progress *p, u64 total, const char *title);
+void __ui_progress__init(struct ui_progress *p, u64 total,
+			 const char *title, bool size);
+
+#define ui_progress__init(p, total, title) \
+	__ui_progress__init(p, total, title, false)
+
+#define ui_progress__init_size(p, total, title) \
+	__ui_progress__init(p, total, title, true)
+
 void ui_progress__update(struct ui_progress *p, u64 adv);
 
 struct ui_progress_ops {
diff --git a/tools/perf/ui/tui/progress.c b/tools/perf/ui/tui/progress.c
index f6b8f52aad7e..68f6144ea603 100644
--- a/tools/perf/ui/tui/progress.c
+++ b/tools/perf/ui/tui/progress.c
@@ -1,8 +1,10 @@
+#include <linux/kernel.h>
 #include "../cache.h"
 #include "../progress.h"
 #include "../libslang.h"
 #include "../ui.h"
 #include "tui.h"
+#include "units.h"
 #include "../browser.h"
 
 static void __tui_progress__init(struct ui_progress *p)
@@ -10,8 +12,22 @@ static void __tui_progress__init(struct ui_progress *p)
 	p->next = p->step = p->total / (SLtt_Screen_Cols - 2) ?: 1;
 }
 
+static int get_title(struct ui_progress *p, char *buf, size_t size)
+{
+	char buf_cur[20];
+	char buf_tot[20];
+	int ret;
+
+	ret  = unit_number__scnprintf(buf_cur, sizeof(buf_cur), p->curr);
+	ret += unit_number__scnprintf(buf_tot, sizeof(buf_tot), p->total);
+
+	return ret + scnprintf(buf, size, "%s [%s/%s]",
+			       p->title, buf_cur, buf_tot);
+}
+
 static void tui_progress__update(struct ui_progress *p)
 {
+	char buf[100], *title = (char *) p->title;
 	int bar, y;
 	/*
 	 * FIXME: We should have a per UI backend way of showing progress,
@@ -23,13 +39,18 @@ static void tui_progress__update(struct ui_progress *p)
 	if (p->total == 0)
 		return;
 
+	if (p->size) {
+		get_title(p, buf, sizeof(buf));
+		title = buf;
+	}
+
 	ui__refresh_dimensions(false);
 	pthread_mutex_lock(&ui__lock);
 	y = SLtt_Screen_Rows / 2 - 2;
 	SLsmg_set_color(0);
 	SLsmg_draw_box(y, 0, 3, SLtt_Screen_Cols);
 	SLsmg_gotorc(y++, 1);
-	SLsmg_write_string((char *)p->title);
+	SLsmg_write_string(title);
 	SLsmg_fill_region(y, 1, 1, SLtt_Screen_Cols - 2, ' ');
 	SLsmg_set_color(HE_COLORSET_SELECTED);
 	bar = ((SLtt_Screen_Cols - 2) * p->curr) / p->total;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index a7ebd9fe8e40..ceac0848469d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1847,7 +1847,7 @@ static int __perf_session__process_events(struct perf_session *session,
 	if (data_offset + data_size < file_size)
 		file_size = data_offset + data_size;
 
-	ui_progress__init(&prog, file_size, "Processing events...");
+	ui_progress__init_size(&prog, file_size, "Processing events...");
 
 	mmap_size = MMAP_SIZE;
 	if (mmap_size > file_size) {
-- 
2.9.5

[toc] | [prev] | [next] | [standalone]

#1728780 — [PATCH 1/4] perf ui progress: Make sure we always define step value

Unlikely, but we could have ui_progress__init being called
with total < 16, which would set the next and step variables
to 0. That would force unnecessary ui_progress__ops->update
calls because 'next' would never raise.

Forcing the next and step values to be always > 0.

Link: http://lkml.kernel.org/n/tip-0zcvhmsmbhkfgoi7c670rq2x@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/progress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/progress.c b/tools/perf/ui/progress.c
index a0f24c7115c5..a9c15804b1f6 100644
--- a/tools/perf/ui/progress.c
+++ b/tools/perf/ui/progress.c
@@ -25,7 +25,7 @@ void ui_progress__update(struct ui_progress *p, u64 adv)
 void ui_progress__init(struct ui_progress *p, u64 total, const char *title)
 {
 	p->curr = 0;
-	p->next = p->step = total / 16;
+	p->next = p->step = total / 16 ?: 1;
 	p->total = total;
 	p->title = title;
 
-- 
2.9.5

[toc] | [prev] | [next] | [standalone]

#1728810 — Re: [PATCH 1/4] perf ui progress: Make sure we always define step value

On Freitag, 8. September 2017 14:05:07 CEST Jiri Olsa wrote:
> Unlikely, but we could have ui_progress__init being called
> with total < 16, which would set the next and step variables
> to 0. That would force unnecessary ui_progress__ops->update
> calls because 'next' would never raise.
> 
> Forcing the next and step values to be always > 0.
> 
> Link: http://lkml.kernel.org/n/tip-0zcvhmsmbhkfgoi7c670rq2x@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/ui/progress.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/ui/progress.c b/tools/perf/ui/progress.c
> index a0f24c7115c5..a9c15804b1f6 100644
> --- a/tools/perf/ui/progress.c
> +++ b/tools/perf/ui/progress.c
> @@ -25,7 +25,7 @@ void ui_progress__update(struct ui_progress *p, u64 adv)
>  void ui_progress__init(struct ui_progress *p, u64 total, const char *title)
> {
>  	p->curr = 0;
> -	p->next = p->step = total / 16;
> +	p->next = p->step = total / 16 ?: 1;
>  	p->total = total;
>  	p->title = title;

This is a GNU extension, does this compile with clang?

Cheers

-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

[toc] | [prev] | [next] | [standalone]

#1728862 — Re: [PATCH 1/4] perf ui progress: Make sure we always define step value

Em Fri, Sep 08, 2017 at 03:16:37PM +0200, Milian Wolff escreveu:
> On Freitag, 8. September 2017 14:05:07 CEST Jiri Olsa wrote:
> > +++ b/tools/perf/ui/progress.c
> > @@ -25,7 +25,7 @@ void ui_progress__update(struct ui_progress *p, u64 adv)
> >  void ui_progress__init(struct ui_progress *p, u64 total, const char *title)
> > {
> >  	p->curr = 0;
> > -	p->next = p->step = total / 16;
> > +	p->next = p->step = total / 16 ?: 1;
> >  	p->total = total;
> >  	p->title = title;
> 
> This is a GNU extension, does this compile with clang?

Huh?

[acme@jouet linux]$ find tools/ -name "*.[ch]"| xargs grep ?: | wc -l
64
[acme@jouet linux]$ find . -name "*.[ch]"| xargs grep ?: | wc -l
725
[acme@jouet linux]$

And yes, tools/perf/ is built with clang regularly, I use containers to
test build the tools/{perf,lib} codebase with gcc and clang on almost
all distros, things like:

[acme@jouet linux]$ cat ~/git/linux-perf-tools-build/fedora/26/Dockerfile 
# docker.io/acmel/linux-perf-tools-build-fedora:26
FROM docker.io/fedora:26
MAINTAINER Arnaldo Carvalho de Melo <acme@kernel.org>
# The second dnf line is to be able to build a kernel, do a make header_install, etc,
# So that we can build samples/bpf/ and the kernel
RUN dnf -y install make gcc flex bison elfutils-libelf-devel \
		   bc findutils clang llvm openssl git \
		   elfutils-devel libunwind-devel audit-libs-devel \
		   openssl-devel slang-devel gtk2-devel perl-ExtUtils-Embed \
		   python-devel binutils-devel xz-devel numactl-devel systemtap-sdt-devel \
		   redhat-rpm-config && \
    dnf -y clean all && \
    rm -rf /usr/share/doc /usr/share/gtk-doc /usr/share/locale /usr/share/man && \
    mkdir -m 777 -p /tmp/build/perf /tmp/build/objtool /tmp/build/linux && \
    groupadd -r perfbuilder && \
    useradd -m -r -g perfbuilder perfbuilder && \
    chown -R perfbuilder.perfbuilder /tmp/build
USER perfbuilder
ENTRYPOINT make -C /git/linux/tools/perf O=/tmp/build/perf && \
           rm -rf /tmp/build/perf/{.[^.]*,*} && \
           make -C /git/linux/tools/perf O=/tmp/build/perf CC=clang && \
           rm -rf /tmp/build/perf/{.[^.]*,*} && \
           make NO_LIBELF=1 -C /git/linux/tools/perf O=/tmp/build/perf && \
           rm -rf /tmp/build/perf/{.[^.]*,*} && \
           make NO_LIBELF=1 -C /git/linux/tools/perf O=/tmp/build/perf CC=clang && \
           make -C /git/linux/tools/objtool O=/tmp/build/objtool && \
           make -C /git/linux O=/tmp/build/linux defconfig && \
           make -C /git/linux O=/tmp/build/linux headers_install && \
           make -C /git/linux O=/tmp/build/linux samples/bpf/
[acme@jouet linux]$

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1729421 — Re: [PATCH 1/4] perf ui progress: Make sure we always define step value

On Freitag, 8. September 2017 15:26:42 CEST Arnaldo Carvalho de Melo wrote:
> Em Fri, Sep 08, 2017 at 03:16:37PM +0200, Milian Wolff escreveu:
> > On Freitag, 8. September 2017 14:05:07 CEST Jiri Olsa wrote:
> > > +++ b/tools/perf/ui/progress.c
> > > @@ -25,7 +25,7 @@ void ui_progress__update(struct ui_progress *p, u64
> > > adv)
> > > 
> > >  void ui_progress__init(struct ui_progress *p, u64 total, const char
> > >  *title)
> > > 
> > > {
> > > 
> > >  	p->curr = 0;
> > > 
> > > -	p->next = p->step = total / 16;
> > > +	p->next = p->step = total / 16 ?: 1;
> > > 
> > >  	p->total = total;
> > >  	p->title = title;
> > 
> > This is a GNU extension, does this compile with clang?
> 
> Huh?
> 
> [acme@jouet linux]$ find tools/ -name "*.[ch]"| xargs grep ?: | wc -l
> 64
> [acme@jouet linux]$ find . -name "*.[ch]"| xargs grep ?: | wc -l
> 725
> [acme@jouet linux]$
> 
> And yes, tools/perf/ is built with clang regularly, I use containers to
> test build the tools/{perf,lib} codebase with gcc and clang on almost
> all distros, things like:

OK, thanks for the clarification. I was wondering because I didn't know about 
this syntax. Googling I found this: https://en.wikipedia.org/wiki/%3F:#C

Quote:

> A GNU extension to C allows omitting the second operand, and using 
implicitly the first operand as the second also:
>
>     a = x ? : y;

Cheers

-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

[toc] | [prev] | [next] | [standalone]

#1728782 — [PATCH 2/4] perf ui progress: Fix progress update

We currently update the 'next' variable only with a single
step value. But it's possible the 'adv' update is bigger
than single 'step' value. This would leave 'next' value
under counted and force unnecessary ui_progress__ops->update
calls.

Calculate the amount of steps we need for 'adv' update
and increase the 'next' with that amounts of steps.

Link: http://lkml.kernel.org/n/tip-4xtvcl0ykeg3ndg2gg46kvyn@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/progress.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/progress.c b/tools/perf/ui/progress.c
index a9c15804b1f6..ae91c8148edf 100644
--- a/tools/perf/ui/progress.c
+++ b/tools/perf/ui/progress.c
@@ -1,3 +1,4 @@
+#include <linux/kernel.h>
 #include "../cache.h"
 #include "progress.h"
 
@@ -14,10 +15,14 @@ struct ui_progress_ops *ui_progress__ops = &null_progress__ops;
 
 void ui_progress__update(struct ui_progress *p, u64 adv)
 {
+	u64 last = p->curr;
+
 	p->curr += adv;
 
 	if (p->curr >= p->next) {
-		p->next += p->step;
+		u64 nr = DIV_ROUND_UP(p->curr - last, p->step);
+
+		p->next += nr * p->step;
 		ui_progress__ops->update(p);
 	}
 }
-- 
2.9.5

[toc] | [prev] | [next] | [standalone]

#1728783 — [PATCH 3/4] perf ui progress: Add ui specific init function

Adding ui specific init function allowing to setup the
progress bar width based on current screen scales.

Adding TUI init function to get more grained update
of the progress bar.

Link: http://lkml.kernel.org/n/tip-tiuspugnjv3o3qbuugsvkgf3@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/progress.c     | 2 ++
 tools/perf/ui/progress.h     | 1 +
 tools/perf/ui/tui/progress.c | 9 +++++++--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/progress.c b/tools/perf/ui/progress.c
index ae91c8148edf..3e2b5d64c55e 100644
--- a/tools/perf/ui/progress.c
+++ b/tools/perf/ui/progress.c
@@ -34,6 +34,8 @@ void ui_progress__init(struct ui_progress *p, u64 total, const char *title)
 	p->total = total;
 	p->title = title;
 
+	if (ui_progress__ops->init)
+		ui_progress__ops->init(p);
 }
 
 void ui_progress__finish(void)
diff --git a/tools/perf/ui/progress.h b/tools/perf/ui/progress.h
index 717d39d3052b..e5f434a2070b 100644
--- a/tools/perf/ui/progress.h
+++ b/tools/perf/ui/progress.h
@@ -14,6 +14,7 @@ void ui_progress__init(struct ui_progress *p, u64 total, const char *title);
 void ui_progress__update(struct ui_progress *p, u64 adv);
 
 struct ui_progress_ops {
+	void (*init)(struct ui_progress *p);
 	void (*update)(struct ui_progress *p);
 	void (*finish)(void);
 };
diff --git a/tools/perf/ui/tui/progress.c b/tools/perf/ui/tui/progress.c
index c4b99008e2c9..f6b8f52aad7e 100644
--- a/tools/perf/ui/tui/progress.c
+++ b/tools/perf/ui/tui/progress.c
@@ -5,6 +5,11 @@
 #include "tui.h"
 #include "../browser.h"
 
+static void __tui_progress__init(struct ui_progress *p)
+{
+	p->next = p->step = p->total / (SLtt_Screen_Cols - 2) ?: 1;
+}
+
 static void tui_progress__update(struct ui_progress *p)
 {
 	int bar, y;
@@ -49,8 +54,8 @@ static void tui_progress__finish(void)
 	pthread_mutex_unlock(&ui__lock);
 }
 
-static struct ui_progress_ops tui_progress__ops =
-{
+static struct ui_progress_ops tui_progress__ops = {
+	.init   = __tui_progress__init,
 	.update = tui_progress__update,
 	.finish = tui_progress__finish,
 };
-- 
2.9.5

[toc] | [prev] | [next] | [standalone]

#1728910

Em Fri, Sep 08, 2017 at 02:05:06PM +0200, Jiri Olsa escreveu:
> hi,
> sending some progress bar fixes.

Thanks, tested and applied.

- Arnaldo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web