perf tools: Small sample parsing speedup

PATCH In review Jiri Olsa 4 participants 11 messages Oct 31, 2017 – Nov 1, 2017 perf events

lore

Share & tools

lore

Jiri Olsa

author reviewer

2017-10-31 09:29 UTC #0 lore raw

cover perf tools: Small sample parsing speedup

hi,
sending small speedup fix for sample parsing code
and few assorted fixes.

Also available in:
https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes

thanks,
jirka

---
Jiri Olsa (7):
perf tools: Reset cursor arg instead of callchain_cursor perf tools: Centralize perf_sample initialization perf tools: Add perf_evlist__parse_sample_timestamp function perf tools: Pass timestamp arg in perf_session__queue_event perf tools: Optimize sample parsing for ordered events perf tools: Remove perf_tool from event_op2
perf tools: Remove perf_tool from event_op3

 tools/perf/builtin-inject.c      |  32 ++++++++++++++------------------
 tools/perf/builtin-kvm.c         |   8 ++++----
 tools/perf/builtin-script.c      |  22 ++++++++++------------
 tools/perf/builtin-stat.c        |  23 +++++++++++------------
 tools/perf/util/auxtrace.c       |  17 +++++++----------
 tools/perf/util/auxtrace.h       |  15 ++++++---------
 tools/perf/util/evlist.c         |  11 +++++++++++
 tools/perf/util/evlist.h         |   4 ++++
 tools/perf/util/evsel.c          |  71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 tools/perf/util/evsel.h          |   4 ++++
 tools/perf/util/header.c         |  16 +++++++---------
 tools/perf/util/header.h         |  15 ++++++---------
 tools/perf/util/machine.c        |   2 +-
 tools/perf/util/ordered-events.c |   3 +--
 tools/perf/util/ordered-events.h |   2 +-
 tools/perf/util/session.c        | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------
 tools/perf/util/session.h        |   7 +++----
 tools/perf/util/stat.c           |   5 ++---
 tools/perf/util/stat.h           |   5 ++---
 tools/perf/util/tool.h           |   7 ++-----
 20 files changed, 208 insertions(+), 181 deletions(-)

Jiri Olsa

author reviewer

2017-10-31 09:29 UTC #1 lore raw

PATCH 1/7 perf tools: Reset cursor arg instead of callchain_cursor

We already pass cursor into thread__resolve_callchain function, so there's no point in resetting the global instance.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/util/machine.c +1 -1

index 94d8f1ccedd9..3f6512b0e6b3 100644
@@ -2200,7 +2200,7 @@ int thread__resolve_callchain(struct thread *thread,
 {
 	int ret = 0;
 
-	callchain_cursor_reset(&callchain_cursor);
+	callchain_cursor_reset(cursor);
 
 	if (callchain_param.order == ORDER_CALLEE) {
 		ret = thread__resolve_callchain_sample(thread, cursor,

Signature · 1 line

2.13.6

Jiri Olsa

author reviewer

2017-10-31 09:31 UTC #2 lore raw

PATCH 2/7 perf tools: Centralize perf_sample initialization

In reply to Jiri Olsa

Move the initialization bits into common place at the beginning of the function.

Also removing some superfluous zero initialization for addr and transaction, because we zero all the data at the top.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/evsel.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

tools/perf/util/evsel.c +2 -4

index f894893c203d..8bd013896b26 100644
@@ -1977,6 +1977,8 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	data->stream_id = data->id = data->time = -1ULL;
 	data->period = evsel->attr.sample_period;
 	data->cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
+	data->id = -1ULL;
+	data->data_src = PERF_MEM_DATA_SRC_NONE;
 
 	if (event->header.type != PERF_RECORD_SAMPLE) {
 		if (!evsel->attr.sample_id_all)

@@ -1994,7 +1996,6 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	if (evsel->sample_size + sizeof(event->header) > event->header.size)
 		return -EFAULT;
 
-	data->id = -1ULL;
 	if (type & PERF_SAMPLE_IDENTIFIER) {
 		data->id = *array;
 		array++;

@@ -2024,7 +2025,6 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 		array++;
 	}
 
-	data->addr = 0;
 	if (type & PERF_SAMPLE_ADDR) {
 		data->addr = *array;
 		array++;

@@ -2188,14 +2188,12 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 		array++;
 	}
 
-	data->data_src = PERF_MEM_DATA_SRC_NONE;
 	if (type & PERF_SAMPLE_DATA_SRC) {
 		OVERFLOW_CHECK_u64(array);
 		data->data_src = *array;
 		array++;
 	}
 
-	data->transaction = 0;
 	if (type & PERF_SAMPLE_TRANSACTION) {
 		OVERFLOW_CHECK_u64(array);
 		data->transaction = *array;

Signature · 1 line

2.13.6

Jiri Olsa

author reviewer

2017-10-31 09:30 UTC #3 lore raw

PATCH 3/7 perf tools: Add perf_evlist__parse_sample_timestamp function

In reply to Jiri Olsa

Adding perf_evlist__parse_sample_timestamp to retrieve the timestamp of the sample.

The idea is to use this function instead of the full sample parsing before we queue the sample. At that time only the timestamp is needed and we parse the sample once again later on delivery.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/evlist.c | 11 ++++++++
 tools/perf/util/evlist.h |  4 +++
 tools/perf/util/evsel.c  | 65 +++++++++++++++++++++++++++++++++++++++++++-----
 tools/perf/util/evsel.h  |  4 +++
 4 files changed, 78 insertions(+), 6 deletions(-)

tools/perf/util/evlist.c +11 -0

index c6c891e154a6..8e31729cca54 100644
@@ -1582,6 +1582,17 @@ int perf_evlist__parse_sample(struct perf_evlist *evlist, union perf_event *even
 	return perf_evsel__parse_sample(evsel, event, sample);
 }
 
+int perf_evlist__parse_sample_timestamp(struct perf_evlist *evlist,
+					union perf_event *event,
+					u64 *timestamp)
+{
+	struct perf_evsel *evsel = perf_evlist__event2evsel(evlist, event);
+
+	if (!evsel)
+		return -EFAULT;
+	return perf_evsel__parse_sample_timestamp(evsel, event, timestamp);
+}
+
 size_t perf_evlist__fprintf(struct perf_evlist *evlist, FILE *fp)
 {
 	struct perf_evsel *evsel;

tools/perf/util/evlist.h +4 -0

index 8c433e95bd9a..9acafbf6f7dd 100644
@@ -204,6 +204,10 @@ u16 perf_evlist__id_hdr_size(struct perf_evlist *evlist);
 int perf_evlist__parse_sample(struct perf_evlist *evlist, union perf_event *event,
 			      struct perf_sample *sample);
 
+int perf_evlist__parse_sample_timestamp(struct perf_evlist *evlist,
+					union perf_event *event,
+					u64 *timestamp);
+
 bool perf_evlist__valid_sample_type(struct perf_evlist *evlist);
 bool perf_evlist__valid_sample_id_all(struct perf_evlist *evlist);
 bool perf_evlist__valid_read_format(struct perf_evlist *evlist);

tools/perf/util/evsel.c +59 -6

83 lines not on this page · raw message

tools/perf/util/evsel.h +4 -0

index 64782b19089d..fb005ef62945 100644
@@ -337,6 +337,10 @@ static inline int perf_evsel__read_on_cpu_scaled(struct perf_evsel *evsel,
 int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 			     struct perf_sample *sample);
 
+int perf_evsel__parse_sample_timestamp(struct perf_evsel *evsel,
+				       union perf_event *event,
+				       u64 *timestamp);
+
 static inline struct perf_evsel *perf_evsel__next(struct perf_evsel *evsel)
 {
 	return list_entry(evsel->node.next, struct perf_evsel, node);

Signature · 1 line

2.13.6

Jiri Olsa

author reviewer

2017-10-31 09:31 UTC #4 lore raw

PATCH 4/7 perf tools: Pass timestamp arg in perf_session__queue_event

In reply to Jiri Olsa

There's no need to pass whole sample data, because it's only timestamp that is used.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/builtin-kvm.c         | 2 +-
 tools/perf/util/ordered-events.c | 3 +--
 tools/perf/util/ordered-events.h | 2 +-
 tools/perf/util/session.c        | 6 +++---
 tools/perf/util/session.h        | 2 +-
 5 files changed, 7 insertions(+), 8 deletions(-)

tools/perf/builtin-kvm.c +1 -1

index 0af4c092b471..293589a9adab 100644
@@ -753,7 +753,7 @@ static s64 perf_kvm__mmap_read_idx(struct perf_kvm_stat *kvm, int idx,
 			return -1;
 		}
 
-		err = perf_session__queue_event(kvm->session, event, &sample, 0);
+		err = perf_session__queue_event(kvm->session, event, sample.time, 0);
 		/*
 		 * FIXME: Here we can't consume the event, as perf_session__queue_event will
 		 *        point to it, and it'll get possibly overwritten by the kernel.

tools/perf/util/ordered-events.c +1 -2

index 4de398cfb577..7eb9115d8ad4 100644
@@ -156,9 +156,8 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
 }
 
 int ordered_events__queue(struct ordered_events *oe, union perf_event *event,
-			  struct perf_sample *sample, u64 file_offset)
+			  u64 timestamp, u64 file_offset)
 {
-	u64 timestamp = sample->time;
 	struct ordered_event *oevent;
 
 	if (!timestamp || timestamp == ~0ULL)

tools/perf/util/ordered-events.h +1 -1

index e11468a9a6e4..c2a5fd844b25 100644
@@ -44,7 +44,7 @@ struct ordered_events {
 };
 
 int ordered_events__queue(struct ordered_events *oe, union perf_event *event,
-			  struct perf_sample *sample, u64 file_offset);
+			  u64 timestamp, u64 file_offset);
 void ordered_events__delete(struct ordered_events *oe, struct ordered_event *event);
 int ordered_events__flush(struct ordered_events *oe, enum oe_flush how);
 void ordered_events__init(struct ordered_events *oe, ordered_events__deliver_t deliver);

tools/perf/util/session.c +3 -3

index c09b748ab599..e591006c0d56 100644
@@ -872,9 +872,9 @@ static int process_finished_round(struct perf_tool *tool __maybe_unused,
 }
 
 int perf_session__queue_event(struct perf_session *s, union perf_event *event,
-			      struct perf_sample *sample, u64 file_offset)
+			      u64 timestamp, u64 file_offset)
 {
-	return ordered_events__queue(&s->ordered_events, event, sample, file_offset);
+	return ordered_events__queue(&s->ordered_events, event, timestamp, file_offset);
 }
 
 static void callchain__lbr_callstack_printf(struct perf_sample *sample)

@@ -1516,7 +1516,7 @@ static s64 perf_session__process_event(struct perf_session *session,
 		return ret;
 
 	if (tool->ordered_events) {
-		ret = perf_session__queue_event(session, event, &sample, file_offset);
+		ret = perf_session__queue_event(session, event, sample.time, file_offset);
 		if (ret != -ETIME)
 			return ret;
 	}

tools/perf/util/session.h +1 -1

index cc1c5ea53c39..c0dd09aebb4b 100644
@@ -52,7 +52,7 @@ int perf_session__peek_event(struct perf_session *session, off_t file_offset,
 int perf_session__process_events(struct perf_session *session);
 
 int perf_session__queue_event(struct perf_session *s, union perf_event *event,
-			      struct perf_sample *sample, u64 file_offset);
+			      u64 timestamp, u64 file_offset);
 
 void perf_tool__fill_defaults(struct perf_tool *tool);

Signature · 1 line

2.13.6

Jiri Olsa

author reviewer

2017-10-31 09:31 UTC #5 lore raw

PATCH 5/7 perf tools: Optimize sample parsing for ordered events

In reply to Jiri Olsa

Currently when using ordered events we parse the sample twice (the perf_evlist__parse_sample function). Once before we queue the sample for sorting:

perf_session__process_event
perf_evlist__parse_sample(sample)
perf_session__queue_event(sample.time)

And then when we deliver the sorted sample:

ordered_events__deliver_event
perf_evlist__parse_sample
perf_session__deliver_event

We can skip the initial full sample parsing by using perf_evlist__parse_sample_timestamp function, which got introduced earlier. The new path looks like:

perf_session__process_event
perf_evlist__parse_sample_timestamp
perf_session__queue_event

ordered_events__deliver_event
perf_session__deliver_event
perf_evlist__parse_sample

It saves some instructions and is slightly faster:

Before:
Performance counter stats for './perf.old report --stdio' (5 runs):

    64,396,007,225      cycles:u                                                      ( +-  0.97% )
   105,882,112,735      instructions:u            #    1.64  insn per cycle           ( +-  0.00% )

21.618103465 seconds time elapsed ( +- 1.12% )

After:
Performance counter stats for './perf report --stdio' (5 runs):

    60,567,807,182      cycles:u                                                      ( +-  0.40% )
   104,853,333,514      instructions:u            #    1.73  insn per cycle           ( +-  0.00% )

20.168895243 seconds time elapsed ( +- 0.32% )

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/builtin-kvm.c  |  8 ++++----
 tools/perf/util/session.c | 41 ++++++++++++++++++-----------------------
 2 files changed, 22 insertions(+), 27 deletions(-)

tools/perf/builtin-kvm.c +4 -4

index 293589a9adab..24733aea25cb 100644
@@ -740,20 +740,20 @@ static s64 perf_kvm__mmap_read_idx(struct perf_kvm_stat *kvm, int idx,
 				   u64 *mmap_time)
 {
 	union perf_event *event;
-	struct perf_sample sample;
+	u64 timestamp;
 	s64 n = 0;
 	int err;
 
 	*mmap_time = ULLONG_MAX;
 	while ((event = perf_evlist__mmap_read(kvm->evlist, idx)) != NULL) {
-		err = perf_evlist__parse_sample(kvm->evlist, event, &sample);
+		err = perf_evlist__parse_sample_timestamp(kvm->evlist, event, &timestamp);
 		if (err) {
 			perf_evlist__mmap_consume(kvm->evlist, idx);
 			pr_err("Failed to parse sample\n");
 			return -1;
 		}
 
-		err = perf_session__queue_event(kvm->session, event, sample.time, 0);
+		err = perf_session__queue_event(kvm->session, event, timestamp, 0);
 		/*
 		 * FIXME: Here we can't consume the event, as perf_session__queue_event will
 		 *        point to it, and it'll get possibly overwritten by the kernel.

@@ -767,7 +767,7 @@ static s64 perf_kvm__mmap_read_idx(struct perf_kvm_stat *kvm, int idx,
 
 		/* save time stamp of our first sample for this mmap */
 		if (n == 0)
-			*mmap_time = sample.time;
+			*mmap_time = timestamp;
 
 		/* limit events per mmap handled all at once */
 		n++;

tools/perf/util/session.c +18 ~~-23~~

90 lines not on this page · raw message

Signature · 1 line

2.13.6

Ingo Molnar

2017-10-31 09:40 UTC #6 lore raw

Re: [PATCH 5/7] perf tools: Optimize sample parsing for ordered events

Quoted 36 lines · Jiri Olsa

Currently when using ordered events we parse the sample twice (the perf_evlist__parse_sample function). Once before we queue the sample for sorting:
perf_session__process_event
perf_evlist__parse_sample(sample)
perf_session__queue_event(sample.time)
And then when we deliver the sorted sample:
ordered_events__deliver_event
perf_evlist__parse_sample
perf_session__deliver_event
We can skip the initial full sample parsing by using perf_evlist__parse_sample_timestamp function, which got introduced earlier. The new path looks like:
perf_session__process_event
perf_evlist__parse_sample_timestamp
perf_session__queue_event
ordered_events__deliver_event
perf_session__deliver_event
perf_evlist__parse_sample
It saves some instructions and is slightly faster:
Before:
Performance counter stats for './perf.old report --stdio' (5 runs):
    64,396,007,225      cycles:u                                                      ( +-  0.97% )
   105,882,112,735      instructions:u            #    1.64  insn per cycle           ( +-  0.00% )
21.618103465 seconds time elapsed ( +- 1.12% )

Quoted 7 lines

After:
Performance counter stats for './perf report --stdio' (5 runs):
    60,567,807,182      cycles:u                                                      ( +-  0.40% )
   104,853,333,514      instructions:u            #    1.73  insn per cycle           ( +-  0.00% )
20.168895243 seconds time elapsed ( +- 0.32% )

That's a 7% speedup, not bad!

Thanks,

Ingo

Jiri Olsa

author reviewer

2017-10-31 09:30 UTC #7 lore raw

PATCH 6/7 perf tools: Remove perf_tool from event_op2

In reply to Jiri Olsa

Now when we keep perf_tool pointer inside perf_session, there's no need to have perf_tool argument in the event_op2 callback. Removing it.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/builtin-inject.c | 26 ++++++++----------
 tools/perf/builtin-script.c | 22 +++++++--------
 tools/perf/builtin-stat.c   | 23 ++++++++--------
 tools/perf/util/auxtrace.c  | 10 +++----
 tools/perf/util/auxtrace.h  | 10 +++----
 tools/perf/util/header.c    | 16 +++++------
 tools/perf/util/header.h    | 15 ++++------
 tools/perf/util/session.c   | 67 +++++++++++++++++++--------------------------
 tools/perf/util/session.h   |  5 ++--
 tools/perf/util/stat.c      |  5 ++--
 tools/perf/util/stat.h      |  5 ++--
 tools/perf/util/tool.h      |  3 +-
 12 files changed, 88 insertions(+), 119 deletions(-)

tools/perf/builtin-inject.c +11 ~~-15~~

index 91e65093d3c2..6f73e716d1c0 100644
@@ -85,12 +85,10 @@ static int perf_event__drop_oe(struct perf_tool *tool __maybe_unused,
 }
 #endif
 
-static int perf_event__repipe_op2_synth(struct perf_tool *tool,
-					union perf_event *event,
-					struct perf_session *session
-					__maybe_unused)
+static int perf_event__repipe_op2_synth(struct perf_session *session,
+					union perf_event *event)
 {
-	return perf_event__repipe_synth(tool, event);
+	return perf_event__repipe_synth(session->tool, event);
 }
 
 static int perf_event__repipe_attr(struct perf_tool *tool,

@@ -361,26 +359,24 @@ static int perf_event__repipe_exit(struct perf_tool *tool,
 	return err;
 }
 
-static int perf_event__repipe_tracing_data(struct perf_tool *tool,
-					   union perf_event *event,
-					   struct perf_session *session)
+static int perf_event__repipe_tracing_data(struct perf_session *session,
+					   union perf_event *event)
 {
 	int err;
 
-	perf_event__repipe_synth(tool, event);
-	err = perf_event__process_tracing_data(tool, event, session);
+	perf_event__repipe_synth(session->tool, event);
+	err = perf_event__process_tracing_data(session, event);
 
 	return err;
 }
 
-static int perf_event__repipe_id_index(struct perf_tool *tool,
-				       union perf_event *event,
-				       struct perf_session *session)
+static int perf_event__repipe_id_index(struct perf_session *session,
+				       union perf_event *event)
 {
 	int err;
 
-	perf_event__repipe_synth(tool, event);
-	err = perf_event__process_id_index(tool, event, session);
+	perf_event__repipe_synth(session->tool, event);
+	err = perf_event__process_id_index(session, event);
 
 	return err;
 }

tools/perf/builtin-script.c +10 ~~-12~~

index 89975e30c0ba..48080f2f2470 100644
@@ -2756,9 +2756,8 @@ static void script__setup_sample_type(struct perf_script *script)
 	}
 }
 
-static int process_stat_round_event(struct perf_tool *tool __maybe_unused,
-				    union perf_event *event,
-				    struct perf_session *session)
+static int process_stat_round_event(struct perf_session *session,
+				    union perf_event *event)
 {
 	struct stat_round_event *round = &event->stat_round;
 	struct perf_evsel *counter;

@@ -2772,9 +2771,8 @@ static int process_stat_round_event(struct perf_tool *tool __maybe_unused,
 	return 0;
 }
 
-static int process_stat_config_event(struct perf_tool *tool __maybe_unused,
-				     union perf_event *event,
-				     struct perf_session *session __maybe_unused)
+static int process_stat_config_event(struct perf_session *session __maybe_unused,
+				     union perf_event *event)
 {
 	perf_event__read_stat_config(&stat_config, &event->stat_config);
 	return 0;

@@ -2800,10 +2798,10 @@ static int set_maps(struct perf_script *script)
 }
 
 static
-int process_thread_map_event(struct perf_tool *tool,
-			     union perf_event *event,
-			     struct perf_session *session __maybe_unused)
+int process_thread_map_event(struct perf_session *session,
+			     union perf_event *event)
 {
+	struct perf_tool *tool = session->tool;
 	struct perf_script *script = container_of(tool, struct perf_script, tool);
 
 	if (script->threads) {

@@ -2819,10 +2817,10 @@ int process_thread_map_event(struct perf_tool *tool,
 }
 
 static
-int process_cpu_map_event(struct perf_tool *tool __maybe_unused,
-			  union perf_event *event,
-			  struct perf_session *session __maybe_unused)
+int process_cpu_map_event(struct perf_session *session,
+			  union perf_event *event)
 {
+	struct perf_tool *tool = session->tool;
 	struct perf_script *script = container_of(tool, struct perf_script, tool);
 
 	if (script->cpus) {

tools/perf/builtin-stat.c +11 ~~-12~~

index 59af5a8419e2..718a6a5b4ee0 100644
@@ -2432,9 +2432,8 @@ static int __cmd_record(int argc, const char **argv)
 	return argc;
 }
 
-static int process_stat_round_event(struct perf_tool *tool __maybe_unused,
-				    union perf_event *event,
-				    struct perf_session *session)
+static int process_stat_round_event(struct perf_session *session,
+				    union perf_event *event)
 {
 	struct stat_round_event *stat_round = &event->stat_round;
 	struct perf_evsel *counter;

@@ -2459,10 +2458,10 @@ static int process_stat_round_event(struct perf_tool *tool __maybe_unused,
 }
 
 static
-int process_stat_config_event(struct perf_tool *tool,
-			      union perf_event *event,
-			      struct perf_session *session __maybe_unused)
+int process_stat_config_event(struct perf_session *session,
+			      union perf_event *event)
 {
+	struct perf_tool *tool = session->tool;
 	struct perf_stat *st = container_of(tool, struct perf_stat, tool);
 
 	perf_event__read_stat_config(&stat_config, &event->stat_config);

@@ -2502,10 +2501,10 @@ static int set_maps(struct perf_stat *st)
 }
 
 static
-int process_thread_map_event(struct perf_tool *tool,
-			     union perf_event *event,
-			     struct perf_session *session __maybe_unused)
+int process_thread_map_event(struct perf_session *session,
+			     union perf_event *event)
 {
+	struct perf_tool *tool = session->tool;
 	struct perf_stat *st = container_of(tool, struct perf_stat, tool);
 
 	if (st->threads) {

@@ -2521,10 +2520,10 @@ int process_thread_map_event(struct perf_tool *tool,
 }
 
 static
-int process_cpu_map_event(struct perf_tool *tool,
-			  union perf_event *event,
-			  struct perf_session *session __maybe_unused)
+int process_cpu_map_event(struct perf_session *session,
+			  union perf_event *event)
 {
+	struct perf_tool *tool = session->tool;
 	struct perf_stat *st = container_of(tool, struct perf_stat, tool);
 	struct cpu_map *cpus;

tools/perf/util/auxtrace.c +4 -6

index a33491416400..cdc4a3187b5f 100644
@@ -899,9 +899,8 @@ static bool auxtrace__dont_decode(struct perf_session *session)
 	       session->itrace_synth_opts->dont_decode;
 }
 
-int perf_event__process_auxtrace_info(struct perf_tool *tool __maybe_unused,
-				      union perf_event *event,
-				      struct perf_session *session)
+int perf_event__process_auxtrace_info(struct perf_session *session,
+				      union perf_event *event)
 {
 	enum auxtrace_type type = event->auxtrace_info.type;

@@ -1173,9 +1172,8 @@ void events_stats__auxtrace_error_warn(const struct events_stats *stats)
 	}
 }
 
-int perf_event__process_auxtrace_error(struct perf_tool *tool __maybe_unused,
-				       union perf_event *event,
-				       struct perf_session *session)
+int perf_event__process_auxtrace_error(struct perf_session *session,
+				       union perf_event *event)
 {
 	if (auxtrace__dont_decode(session))
 		return 0;

tools/perf/util/auxtrace.h +4 -6

index 33b5e6cdf38c..e2329ac31acc 100644
@@ -513,15 +513,13 @@ int perf_event__synthesize_auxtrace_info(struct auxtrace_record *itr,
 					 struct perf_tool *tool,
 					 struct perf_session *session,
 					 perf_event__handler_t process);
-int perf_event__process_auxtrace_info(struct perf_tool *tool,
-				      union perf_event *event,
-				      struct perf_session *session);
+int perf_event__process_auxtrace_info(struct perf_session *session,
+				      union perf_event *event);
 s64 perf_event__process_auxtrace(struct perf_tool *tool,
 				 union perf_event *event,
 				 struct perf_session *session);
-int perf_event__process_auxtrace_error(struct perf_tool *tool,
-				       union perf_event *event,
-				       struct perf_session *session);
+int perf_event__process_auxtrace_error(struct perf_session *session,
+				       union perf_event *event);
 int itrace_parse_synth_opts(const struct option *opt, const char *str,
 			    int unset);
 void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts);

tools/perf/util/header.c +7 -9

index 6e59dcca9df2..8dbcd76c3269 100644
@@ -3050,10 +3050,10 @@ int perf_event__synthesize_features(struct perf_tool *tool,
 	return 0;
 }
 
-int perf_event__process_feature(struct perf_tool *tool,
-				union perf_event *event,
-				struct perf_session *session __maybe_unused)
+int perf_event__process_feature(struct perf_session *session,
+				union perf_event *event)
 {
+	struct perf_tool *tool = session->tool;
 	struct feat_fd ff = { .fd = 0 };
 	struct feature_event *fe = (struct feature_event *)event;
 	int type = fe->header.type;

@@ -3392,9 +3392,8 @@ int perf_event__synthesize_tracing_data(struct perf_tool *tool, int fd,
 	return aligned_size;
 }
 
-int perf_event__process_tracing_data(struct perf_tool *tool __maybe_unused,
-				     union perf_event *event,
-				     struct perf_session *session)
+int perf_event__process_tracing_data(struct perf_session *session,
+				     union perf_event *event)
 {
 	ssize_t size_read, padding, size = event->tracing_data.size;
 	int fd = perf_data__fd(session->data);

@@ -3460,9 +3459,8 @@ int perf_event__synthesize_build_id(struct perf_tool *tool,
 	return err;
 }
 
-int perf_event__process_build_id(struct perf_tool *tool __maybe_unused,
-				 union perf_event *event,
-				 struct perf_session *session)
+int perf_event__process_build_id(struct perf_session *session,
+				 union perf_event *event)
 {
 	__event_process_build_id(&event->build_id,
 				 event->build_id.filename,

tools/perf/util/header.h +6 -9

index f7a16ee527b8..170854a43ad5 100644
@@ -106,9 +106,8 @@ int perf_event__synthesize_features(struct perf_tool *tool,
 				    struct perf_evlist *evlist,
 				    perf_event__handler_t process);
 
-int perf_event__process_feature(struct perf_tool *tool,
-				union perf_event *event,
-				struct perf_session *session);
+int perf_event__process_feature(struct perf_session *session,
+				union perf_event *event);
 
 int perf_event__synthesize_attr(struct perf_tool *tool,
 				struct perf_event_attr *attr, u32 ids, u64 *id,

@@ -138,17 +137,15 @@ size_t perf_event__fprintf_event_update(union perf_event *event, FILE *fp);
 int perf_event__synthesize_tracing_data(struct perf_tool *tool,
 					int fd, struct perf_evlist *evlist,
 					perf_event__handler_t process);
-int perf_event__process_tracing_data(struct perf_tool *tool,
-				     union perf_event *event,
-				     struct perf_session *session);
+int perf_event__process_tracing_data(struct perf_session *session,
+				     union perf_event *event);
 
 int perf_event__synthesize_build_id(struct perf_tool *tool,
 				    struct dso *pos, u16 misc,
 				    perf_event__handler_t process,
 				    struct machine *machine);
-int perf_event__process_build_id(struct perf_tool *tool,
-				 union perf_event *event,
-				 struct perf_session *session);
+int perf_event__process_build_id(struct perf_session *session,
+				 union perf_event *event);
 bool is_perf_magic(u64 magic);
 
 #define NAME_ALIGN 64

tools/perf/util/session.c +28 ~~-39~~

142 lines not on this page · raw message

tools/perf/util/session.h +2 -3

index c0dd09aebb4b..c31dcb7e6fb2 100644
@@ -119,9 +119,8 @@ int perf_session__deliver_synth_event(struct perf_session *session,
 				      union perf_event *event,
 				      struct perf_sample *sample);
 
-int perf_event__process_id_index(struct perf_tool *tool,
-				 union perf_event *event,
-				 struct perf_session *session);
+int perf_event__process_id_index(struct perf_session *session,
+				 union perf_event *event);
 
 int perf_event__synthesize_id_index(struct perf_tool *tool,
 				    perf_event__handler_t process,

tools/perf/util/stat.c +2 -3

index 203f5d8d11d1..53567ddfe958 100644
@@ -366,9 +366,8 @@ int perf_stat_process_counter(struct perf_stat_config *config,
 	return 0;
 }
 
-int perf_event__process_stat_event(struct perf_tool *tool __maybe_unused,
-				   union perf_event *event,
-				   struct perf_session *session)
+int perf_event__process_stat_event(struct perf_session *session,
+				   union perf_event *event)
 {
 	struct perf_counts_values count;
 	struct stat_event *st = &event->stat;

tools/perf/util/stat.h +2 -3

index 490b78aa7230..a8a2b01ac124 100644
@@ -107,9 +107,8 @@ int perf_stat_process_counter(struct perf_stat_config *config,
 struct perf_tool;
 union perf_event;
 struct perf_session;
-int perf_event__process_stat_event(struct perf_tool *tool,
-				   union perf_event *event,
-				   struct perf_session *session);
+int perf_event__process_stat_event(struct perf_session *session,
+				   union perf_event *event);
 
 size_t perf_event__fprintf_stat(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf_stat_round(union perf_event *event, FILE *fp);

tools/perf/util/tool.h +1 -2

index d549e50db397..2eddeab74a6b 100644
@@ -25,8 +25,7 @@ typedef int (*event_attr_op)(struct perf_tool *tool,
 			     union perf_event *event,
 			     struct perf_evlist **pevlist);
 
-typedef int (*event_op2)(struct perf_tool *tool, union perf_event *event,
-			 struct perf_session *session);
+typedef int (*event_op2)(struct perf_session *session, union perf_event *event);
 
 typedef int (*event_oe)(struct perf_tool *tool, union perf_event *event,
 			struct ordered_events *oe);

Signature · 1 line

2.13.6

Jiri Olsa

author reviewer

2017-10-31 09:30 UTC #8 lore raw

PATCH 7/7 perf tools: Remove perf_tool from event_op3

In reply to Jiri Olsa

Now when we keep perf_tool pointer inside perf_session, there's no need to have perf_tool argument in the event_op3 callback. Removing it.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/builtin-inject.c | 6 +++---
 tools/perf/util/auxtrace.c  | 7 +++----
 tools/perf/util/auxtrace.h  | 5 ++---
 tools/perf/util/session.c   | 8 +++-----
 tools/perf/util/tool.h      | 4 +---
 5 files changed, 12 insertions(+), 18 deletions(-)

tools/perf/builtin-inject.c +3 -3

index 6f73e716d1c0..ef0b375b4091 100644
@@ -130,10 +130,10 @@ static int copy_bytes(struct perf_inject *inject, int fd, off_t size)
 	return 0;
 }
 
-static s64 perf_event__repipe_auxtrace(struct perf_tool *tool,
-				       union perf_event *event,
-				       struct perf_session *session)
+static s64 perf_event__repipe_auxtrace(struct perf_session *session,
+				       union perf_event *event)
 {
+	struct perf_tool *tool = session->tool;
 	struct perf_inject *inject = container_of(tool, struct perf_inject,
 						  tool);
 	int ret;

tools/perf/util/auxtrace.c +3 -4

index cdc4a3187b5f..775a6f960dd8 100644
@@ -919,9 +919,8 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
 	}
 }
 
-s64 perf_event__process_auxtrace(struct perf_tool *tool,
-				 union perf_event *event,
-				 struct perf_session *session)
+s64 perf_event__process_auxtrace(struct perf_session *session,
+				 union perf_event *event)
 {
 	s64 err;

@@ -937,7 +936,7 @@ s64 perf_event__process_auxtrace(struct perf_tool *tool,
 	if (!session->auxtrace || event->header.type != PERF_RECORD_AUXTRACE)
 		return -EINVAL;
 
-	err = session->auxtrace->process_auxtrace_event(session, event, tool);
+	err = session->auxtrace->process_auxtrace_event(session, event, session->tool);
 	if (err < 0)
 		return err;

tools/perf/util/auxtrace.h +2 -3

index e2329ac31acc..27842bf4dc1d 100644
@@ -515,9 +515,8 @@ int perf_event__synthesize_auxtrace_info(struct auxtrace_record *itr,
 					 perf_event__handler_t process);
 int perf_event__process_auxtrace_info(struct perf_session *session,
 				      union perf_event *event);
-s64 perf_event__process_auxtrace(struct perf_tool *tool,
-				 union perf_event *event,
-				 struct perf_session *session);
+s64 perf_event__process_auxtrace(struct perf_session *session,
+				 union perf_event *event);
 int perf_event__process_auxtrace_error(struct perf_session *session,
 				       union perf_event *event);
 int itrace_parse_synth_opts(const struct option *opt, const char *str,

tools/perf/util/session.c +3 -5

index e565a8332e11..2f3c4542570b 100644
@@ -274,10 +274,8 @@ static int skipn(int fd, off_t n)
 	return 0;
 }
 
-static s64 process_event_auxtrace_stub(struct perf_tool *tool __maybe_unused,
-				       union perf_event *event,
-				       struct perf_session *session
-				       __maybe_unused)
+static s64 process_event_auxtrace_stub(struct perf_session *session __maybe_unused,
+				       union perf_event *event)
 {
 	dump_printf(": unhandled!\n");
 	if (perf_data__is_pipe(session->data))

@@ -1374,7 +1372,7 @@ static s64 perf_session__process_user_event(struct perf_session *session,
 	case PERF_RECORD_AUXTRACE:
 		/* setup for reading amidst mmap */
 		lseek(fd, file_offset + event->header.size, SEEK_SET);
-		return tool->auxtrace(tool, event, session);
+		return tool->auxtrace(session, event);
 	case PERF_RECORD_AUXTRACE_ERROR:
 		perf_session__auxtrace_error_inc(session, event);
 		return tool->auxtrace_error(session, event);

tools/perf/util/tool.h +1 -3

index 2eddeab74a6b..b7d4fef9f683 100644
@@ -26,13 +26,11 @@ typedef int (*event_attr_op)(struct perf_tool *tool,
 			     struct perf_evlist **pevlist);
 
 typedef int (*event_op2)(struct perf_session *session, union perf_event *event);
+typedef s64 (*event_op3)(struct perf_session *session, union perf_event *event);
 
 typedef int (*event_oe)(struct perf_tool *tool, union perf_event *event,
 			struct ordered_events *oe);
 
-typedef s64 (*event_op3)(struct perf_tool *tool, union perf_event *event,
-			 struct perf_session *session);
-
 enum show_feature_header {
 	SHOW_FEAT_NO_HEADER = 0,
 	SHOW_FEAT_HEADER,

Signature · 1 line

2.13.6

Namhyung Kim

maintainer Acked-by on the v1 series

2017-11-01 08:42 UTC #9 lore raw

In reply to Jiri Olsa

Hi Jiri,

Quoted 3 lines · Jiri Olsa

hi,
sending small speedup fix for sample parsing code
and few assorted fixes.

Nice work, for patch 1-5:

Acked-by Namhyung Kim <[email protected]>

But I'm not sure for patch 6 and 7 as tool and event arguments are passed to others consistently.

Thanks,
Namhyung

Quoted 40 lines

Also available in:
https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes

thanks,
jirka

 tools/perf/builtin-inject.c      |  32 ++++++++++++++------------------
 tools/perf/builtin-kvm.c         |   8 ++++----
 tools/perf/builtin-script.c      |  22 ++++++++++------------
 tools/perf/builtin-stat.c        |  23 +++++++++++------------
 tools/perf/util/auxtrace.c       |  17 +++++++----------
 tools/perf/util/auxtrace.h       |  15 ++++++---------
 tools/perf/util/evlist.c         |  11 +++++++++++
 tools/perf/util/evlist.h         |   4 ++++
 tools/perf/util/evsel.c          |  71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 tools/perf/util/evsel.h          |   4 ++++
 tools/perf/util/header.c         |  16 +++++++---------
 tools/perf/util/header.h         |  15 ++++++---------
 tools/perf/util/machine.c        |   2 +-
 tools/perf/util/ordered-events.c |   3 +--
 tools/perf/util/ordered-events.h |   2 +-
 tools/perf/util/session.c        | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------
 tools/perf/util/session.h        |   7 +++----
 tools/perf/util/stat.c           |   5 ++---
 tools/perf/util/stat.h           |   5 ++---
 tools/perf/util/tool.h           |   7 ++-----
 20 files changed, 208 insertions(+), 181 deletions(-)

Jiri Olsa

2017-11-01 11:50 UTC #10 lore raw

Quoted 7 lines · Namhyung Kim

Hi Jiri,
Quoted 3 lines · Jiri Olsa
hi,
sending small speedup fix for sample parsing code
and few assorted fixes.

Quoted 6 lines

Nice work, for patch 1-5:
Acked-by Namhyung Kim <[email protected]>
But I'm not sure for patch 6 and 7 as tool and event arguments are passed to others consistently.

I have it as a base for my other patches in queue and they seemed standalone enough ;-) ... I removed it because I'm adding some more and it got a little crowded

jirka