[PATCH] perf tools: Separate progress bar update when processing events

[PATCH] perf tools: Separate progress bar update when processing events

[Jiri Olsa]

Currently when processing events in __perf_session__process_events
function we we update progress based on the file_size. During the
same processing we update progress bar from within flush_sample_queue
which is based on number of samples count.

Having 2 different based updates is causing the progress bar to
jump heavily back and forth giving not much useful info.

Fixing this by keeping only __perf_session__process_events
based progress bar update. And turning on flush_sample_queue
progress bar update only for final flushing.

This reduces the number of times the progress bar update
function is called and it significantly reduces the loading
time for TUI, where the progress bar update takes quite a lot
of time.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
---
 tools/perf/util/session.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1fc0c62..4df449c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -495,7 +495,7 @@ static int perf_session_deliver_event(struct perf_session *session,
 				      u64 file_offset);
 
 static int flush_sample_queue(struct perf_session *s,
-		       struct perf_tool *tool)
+		       struct perf_tool *tool, bool final)
 {
 	struct ordered_samples *os = &s->ordered_samples;
 	struct list_head *head = &os->samples;
@@ -526,7 +526,7 @@ static int flush_sample_queue(struct perf_session *s,
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
 		list_add(&iter->list, &os->sample_cache);
-		if (++idx >= progress_next) {
+		if (final && (++idx >= progress_next)) {
 			progress_next += os->nr_samples / 16;
 			ui_progress__update(idx, os->nr_samples,
 					    "Processing time ordered events...");
@@ -588,7 +588,7 @@ static int process_finished_round(struct perf_tool *tool,
 				  union perf_event *event __maybe_unused,
 				  struct perf_session *session)
 {
-	int ret = flush_sample_queue(session, tool);
+	int ret = flush_sample_queue(session, tool, false);
 	if (!ret)
 		session->ordered_samples.next_flush = session->ordered_samples.max_timestamp;
 
@@ -1354,7 +1354,7 @@ more:
 	err = 0;
 	/* do the final flush for ordered samples */
 	session->ordered_samples.next_flush = ULLONG_MAX;
-	err = flush_sample_queue(session, tool);
+	err = flush_sample_queue(session, tool, true);
 out_err:
 	ui_progress__finish();
 	perf_session__warn_about_errors(session, tool);
-- 
1.7.11.7

Re: [PATCH] perf tools: Separate progress bar update when processing events

[David Ahern]

On 9/4/13 10:27 AM, Jiri Olsa wrote:
> Currently when processing events in __perf_session__process_events
> function we we update progress based on the file_size. During the
> same processing we update progress bar from within flush_sample_queue
> which is based on number of samples count.
>
> Having 2 different based updates is causing the progress bar to
> jump heavily back and forth giving not much useful info.
>
> Fixing this by keeping only __perf_session__process_events
> based progress bar update. And turning on flush_sample_queue
> progress bar update only for final flushing.
>
> This reduces the number of times the progress bar update
> function is called and it significantly reduces the loading
> time for TUI, where the progress bar update takes quite a lot
> of time.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: David Ahern <dsahern@gmail.com>
> ---
>   tools/perf/util/session.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 1fc0c62..4df449c 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -495,7 +495,7 @@ static int perf_session_deliver_event(struct perf_session *session,
>   				      u64 file_offset);
>
>   static int flush_sample_queue(struct perf_session *s,
> -		       struct perf_tool *tool)
> +		       struct perf_tool *tool, bool final)

Why not base it on next_flush = ULLONG_MAX? Then you don't need the 
extra arg.

David

[PATCHv2] perf tools: Separate progress bar update when processing events

[Jiri Olsa]

On Wed, Sep 04, 2013 at 10:47:22AM -0600, David Ahern wrote:
> On 9/4/13 10:27 AM, Jiri Olsa wrote:

SNIP

> >index 1fc0c62..4df449c 100644
> >--- a/tools/perf/util/session.c
> >+++ b/tools/perf/util/session.c
> >@@ -495,7 +495,7 @@ static int perf_session_deliver_event(struct perf_session *session,
> >  				      u64 file_offset);
> >
> >  static int flush_sample_queue(struct perf_session *s,
> >-		       struct perf_tool *tool)
> >+		       struct perf_tool *tool, bool final)
> 
> Why not base it on next_flush = ULLONG_MAX? Then you don't need the
> extra arg.

ok, v2 attached

thanks,
jirka

---
Currently when processing events in __perf_session__process_events
function we we update progress based on the file_size. During the
same processing we update progress bar from within flush_sample_queue
which is based on number of samples count.

Having 2 different based updates is causing the progress bar to
jump heavily back and forth giving not much usefull info.

Fixing this byt keeping only __perf_session__process_events
based progress bar update. And turning on flush_sample_queue
progress bar update only for final flushing.

This reduces the number of time the progress bar update
function is called and it significantly reduces the loading
time for TUI, where the progress bar update takes quite a lot
of time.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
---
 tools/perf/util/session.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1fc0c62..476caa1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -504,6 +504,7 @@ static int flush_sample_queue(struct perf_session *s,
 	u64 limit = os->next_flush;
 	u64 last_ts = os->last_sample ? os->last_sample->timestamp : 0ULL;
 	unsigned idx = 0, progress_next = os->nr_samples / 16;
+	bool show_progress = limit == ULLONG_MAX;
 	int ret;
 
 	if (!tool->ordered_samples || !limit)
@@ -526,7 +527,7 @@ static int flush_sample_queue(struct perf_session *s,
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
 		list_add(&iter->list, &os->sample_cache);
-		if (++idx >= progress_next) {
+		if (show_progress && (++idx >= progress_next)) {
 			progress_next += os->nr_samples / 16;
 			ui_progress__update(idx, os->nr_samples,
 					    "Processing time ordered events...");
-- 
1.7.11.7

[tip:perf/urgent] perf session: Separate progress bar update when processing events

[tip-bot for Jiri Olsa]

Commit-ID:  526fd8d4f770d18e99680ff87965e16bb8f1d806
Gitweb:     http://git.kernel.org/tip/526fd8d4f770d18e99680ff87965e16bb8f1d806
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 5 Sep 2013 11:14:50 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 Sep 2013 16:19:02 -0300

perf session: Separate progress bar update when processing events

Currently when processing events in the __perf_session__process_events
function we update a progress bar based on the file_size. During the
same processing we update the progress bar from within
flush_sample_queue which is based on number of samples count.

Having 2 different based updates is causing the progress bar to jump
heavily back and forth giving not much usefull info.

Fixing this by keeping only __perf_session__process_events based
progress bar update. And turning on flush_sample_queue progress bar
update only for final flushing.

This reduces the number of time the progress bar update function is
called and it significantly reduces the loading time for TUI, where the
progress bar update takes quite a lot of time.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20130905091449.GC1100@krava.brq.redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1fc0c62..476caa1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -504,6 +504,7 @@ static int flush_sample_queue(struct perf_session *s,
 	u64 limit = os->next_flush;
 	u64 last_ts = os->last_sample ? os->last_sample->timestamp : 0ULL;
 	unsigned idx = 0, progress_next = os->nr_samples / 16;
+	bool show_progress = limit == ULLONG_MAX;
 	int ret;
 
 	if (!tool->ordered_samples || !limit)
@@ -526,7 +527,7 @@ static int flush_sample_queue(struct perf_session *s,
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
 		list_add(&iter->list, &os->sample_cache);
-		if (++idx >= progress_next) {
+		if (show_progress && (++idx >= progress_next)) {
 			progress_next += os->nr_samples / 16;
 			ui_progress__update(idx, os->nr_samples,
 					    "Processing time ordered events...");