perf script python: Provide perf_sample dict to all handlers

RFC In review Arun Kalyanasundaram 2 participants 7 messages Jul 17, 2017 – Jul 18, 2017 perf events

lore

Share & tools

lore

Arun Kalyanasundaram

author

2017-07-17 22:10 UTC #0 lore raw

cover perf script python: Provide perf_sample dict to all handlers

The process_event python hook receives a dict with all perf_sample entries.

Other handlers (e.g. trace_unhandled, python_process_tracepoint) predate the introduction of this dict and do not receive it. This patch series adds the dict to all handlers, aiming to unify the information passed to them.

This change adds an additional argument to the affected handlers. To keep backwards compatibility (and avoid unnecessary work), do not pass the aforementioned dict if the number of arguments signals that handler version predates this change.

Initial Discussion: https://lkml.org/lkml/2017/7/1/108

Arun Kalyanasundaram (4):
perf script python: Allocate memory only if handler exists perf script python: Refactor creation of perf sample dict perf script python: Add perf_sample dict to tracepoint handlers perf script python: Generate hooks with additional argument

 .../util/scripting-engines/trace-event-python.c    | 189 ++++++++++++++-------
 1 file changed, 128 insertions(+), 61 deletions(-)

Signature · 1 line

2.13.2.932.g7449e964c-goog

Arun Kalyanasundaram

author

2017-07-17 22:10 UTC #1 lore raw

RFC 1/4 perf script python: Allocate memory only if handler exists

Avoid allocating memory if hook handler is not available. This saves unused memory allocation and simplifies error path.

Let handler in python_process_tracepoint point to either tracepoint specific or trace_unhandled hook. Use dict to check if handler points to trace_unhandled.

Remove the exit label in python_process_general_event and return when no handler is available.

Signed-off-by Arun Kalyanasundaram <[email protected]>

---
 .../util/scripting-engines/trace-event-python.c    | 38 +++++++++++++---------
 1 file changed, 22 insertions(+), 16 deletions(-)

tools/perf/util/scripting-engines/trace-event-python.c +22 ~~-16~~

index 57b7a00e6f16..8a8f4829d3e2 100644
@@ -407,10 +407,7 @@ static void python_process_tracepoint(struct perf_sample *sample,
 	void *data = sample->raw_data;
 	unsigned long long nsecs = sample->time;
 	const char *comm = thread__comm_str(al->thread);
-
-	t = PyTuple_New(MAX_FIELDS);
-	if (!t)
-		Py_FatalError("couldn't create Python tuple");
+	const char *default_handler_name = "trace_unhandled";
 
 	if (!event) {
 		snprintf(handler_name, sizeof(handler_name),

@@ -427,10 +424,19 @@ static void python_process_tracepoint(struct perf_sample *sample,
 
 	handler = get_handler(handler_name);
 	if (!handler) {
+		handler = get_handler(default_handler_name);
+		if (!handler)
+			return;
 		dict = PyDict_New();
 		if (!dict)
 			Py_FatalError("couldn't create Python dict");
 	}
+
+	t = PyTuple_New(MAX_FIELDS);
+	if (!t)
+		Py_FatalError("couldn't create Python tuple");
+
+
 	s = nsecs / NSEC_PER_SEC;
 	ns = nsecs - s * NSEC_PER_SEC;

@@ -445,7 +451,7 @@ static void python_process_tracepoint(struct perf_sample *sample,
 	/* ip unwinding */
 	callchain = python_process_callchain(sample, evsel, al);
 
-	if (handler) {
+	if (!dict) {
 		PyTuple_SetItem(t, n++, PyInt_FromLong(cpu));
 		PyTuple_SetItem(t, n++, PyInt_FromLong(s));
 		PyTuple_SetItem(t, n++, PyInt_FromLong(ns));

@@ -484,23 +490,23 @@ static void python_process_tracepoint(struct perf_sample *sample,
 		} else { /* FIELD_IS_NUMERIC */
 			obj = get_field_numeric_entry(event, field, data);
 		}
-		if (handler)
+		if (!dict)
 			PyTuple_SetItem(t, n++, obj);
 		else
 			pydict_set_item_string_decref(dict, field->name, obj);
 
 	}
 
-	if (!handler)
+	if (dict)
 		PyTuple_SetItem(t, n++, dict);
 
 	if (_PyTuple_Resize(&t, n) == -1)
 		Py_FatalError("error resizing Python tuple");
 
-	if (handler) {
+	if (!dict) {
 		call_object(handler, t, handler_name);
 	} else {
-		try_call_object("trace_unhandled", t);
+		call_object(handler, t, default_handler_name);
 		Py_DECREF(dict);
 	}

@@ -799,6 +805,12 @@ static void python_process_general_event(struct perf_sample *sample,
 	static char handler_name[64];
 	unsigned n = 0;
 
+	snprintf(handler_name, sizeof(handler_name), "%s", "process_event");
+
+	handler = get_handler(handler_name);
+	if (!handler)
+		return;
+
 	/*
 	 * Use the MAX_FIELDS to make the function expandable, though
 	 * currently there is only one item for the tuple.

@@ -815,12 +827,6 @@ static void python_process_general_event(struct perf_sample *sample,
 	if (!dict_sample)
 		Py_FatalError("couldn't create Python dictionary");
 
-	snprintf(handler_name, sizeof(handler_name), "%s", "process_event");
-
-	handler = get_handler(handler_name);
-	if (!handler)
-		goto exit;
-
 	pydict_set_item_string_decref(dict, "ev_name", PyString_FromString(perf_evsel__name(evsel)));
 	pydict_set_item_string_decref(dict, "attr", PyString_FromStringAndSize(
 			(const char *)&evsel->attr, sizeof(evsel->attr)));

@@ -861,7 +867,7 @@ static void python_process_general_event(struct perf_sample *sample,
 		Py_FatalError("error resizing Python tuple");
 
 	call_object(handler, t, handler_name);
-exit:
+
 	Py_DECREF(dict);
 	Py_DECREF(t);
 }

Signature · 1 line

2.13.2.932.g7449e964c-goog

Arun Kalyanasundaram

author

2017-07-17 22:11 UTC #2 lore raw

RFC 2/4 perf script python: Refactor creation of perf sample dict

In reply to Arun Kalyanasundaram

Move the creation of the dict containing perf_sample entries into a helper function to enable its reuse in other sample processing routines.

Signed-off-by Arun Kalyanasundaram <[email protected]>

---
 .../util/scripting-engines/trace-event-python.c    | 94 ++++++++++++----------
 1 file changed, 53 insertions(+), 41 deletions(-)

tools/perf/util/scripting-engines/trace-event-python.c +53 ~~-41~~

index 8a8f4829d3e2..69d1b6db96f6 100644
@@ -391,6 +391,57 @@ static PyObject *python_process_callchain(struct perf_sample *sample,
 	return pylist;
 }
 
+static PyObject *get_perf_sample_dict(struct perf_sample *sample,
+					 struct perf_evsel *evsel,
+					 struct addr_location *al,
+					 PyObject *callchain)
+{
+	PyObject *dict, *dict_sample;
+
+	dict = PyDict_New();
+	if (!dict)
+		Py_FatalError("couldn't create Python dictionary");
+
+	dict_sample = PyDict_New();
+	if (!dict_sample)
+		Py_FatalError("couldn't create Python dictionary");
+
+	pydict_set_item_string_decref(dict, "ev_name", PyString_FromString(perf_evsel__name(evsel)));
+	pydict_set_item_string_decref(dict, "attr", PyString_FromStringAndSize(
+			(const char *)&evsel->attr, sizeof(evsel->attr)));
+
+	pydict_set_item_string_decref(dict_sample, "pid",
+			PyInt_FromLong(sample->pid));
+	pydict_set_item_string_decref(dict_sample, "tid",
+			PyInt_FromLong(sample->tid));
+	pydict_set_item_string_decref(dict_sample, "cpu",
+			PyInt_FromLong(sample->cpu));
+	pydict_set_item_string_decref(dict_sample, "ip",
+			PyLong_FromUnsignedLongLong(sample->ip));
+	pydict_set_item_string_decref(dict_sample, "time",
+			PyLong_FromUnsignedLongLong(sample->time));
+	pydict_set_item_string_decref(dict_sample, "period",
+			PyLong_FromUnsignedLongLong(sample->period));
+	pydict_set_item_string_decref(dict, "sample", dict_sample);
+
+	pydict_set_item_string_decref(dict, "raw_buf", PyString_FromStringAndSize(
+			(const char *)sample->raw_data, sample->raw_size));
+	pydict_set_item_string_decref(dict, "comm",
+			PyString_FromString(thread__comm_str(al->thread)));
+	if (al->map) {
+		pydict_set_item_string_decref(dict, "dso",
+			PyString_FromString(al->map->dso->name));
+	}
+	if (al->sym) {
+		pydict_set_item_string_decref(dict, "symbol",
+			PyString_FromString(al->sym->name));
+	}
+
+	pydict_set_item_string_decref(dict, "callchain", callchain);
+
+	return dict;
+}
+
 static void python_process_tracepoint(struct perf_sample *sample,
 				      struct perf_evsel *evsel,
 				      struct addr_location *al)

@@ -801,7 +852,7 @@ static void python_process_general_event(struct perf_sample *sample,
 					 struct perf_evsel *evsel,
 					 struct addr_location *al)
 {
-	PyObject *handler, *t, *dict, *callchain, *dict_sample;
+	PyObject *handler, *t, *dict, *callchain;
 	static char handler_name[64];
 	unsigned n = 0;

@@ -819,48 +870,9 @@ static void python_process_general_event(struct perf_sample *sample,
 	if (!t)
 		Py_FatalError("couldn't create Python tuple");
 
-	dict = PyDict_New();
-	if (!dict)
-		Py_FatalError("couldn't create Python dictionary");
-
-	dict_sample = PyDict_New();
-	if (!dict_sample)
-		Py_FatalError("couldn't create Python dictionary");
-
-	pydict_set_item_string_decref(dict, "ev_name", PyString_FromString(perf_evsel__name(evsel)));
-	pydict_set_item_string_decref(dict, "attr", PyString_FromStringAndSize(
-			(const char *)&evsel->attr, sizeof(evsel->attr)));
-
-	pydict_set_item_string_decref(dict_sample, "pid",
-			PyInt_FromLong(sample->pid));
-	pydict_set_item_string_decref(dict_sample, "tid",
-			PyInt_FromLong(sample->tid));
-	pydict_set_item_string_decref(dict_sample, "cpu",
-			PyInt_FromLong(sample->cpu));
-	pydict_set_item_string_decref(dict_sample, "ip",
-			PyLong_FromUnsignedLongLong(sample->ip));
-	pydict_set_item_string_decref(dict_sample, "time",
-			PyLong_FromUnsignedLongLong(sample->time));
-	pydict_set_item_string_decref(dict_sample, "period",
-			PyLong_FromUnsignedLongLong(sample->period));
-	pydict_set_item_string_decref(dict, "sample", dict_sample);
-
-	pydict_set_item_string_decref(dict, "raw_buf", PyString_FromStringAndSize(
-			(const char *)sample->raw_data, sample->raw_size));
-	pydict_set_item_string_decref(dict, "comm",
-			PyString_FromString(thread__comm_str(al->thread)));
-	if (al->map) {
-		pydict_set_item_string_decref(dict, "dso",
-			PyString_FromString(al->map->dso->name));
-	}
-	if (al->sym) {
-		pydict_set_item_string_decref(dict, "symbol",
-			PyString_FromString(al->sym->name));
-	}
-
 	/* ip unwinding */
 	callchain = python_process_callchain(sample, evsel, al);
-	pydict_set_item_string_decref(dict, "callchain", callchain);
+	dict = get_perf_sample_dict(sample, evsel, al, callchain);
 
 	PyTuple_SetItem(t, n++, dict);
 	if (_PyTuple_Resize(&t, n) == -1)

Signature · 1 line

2.13.2.932.g7449e964c-goog

Arun Kalyanasundaram

author

2017-07-17 22:11 UTC #3 lore raw

RFC 3/4 perf script python: Add perf_sample dict to tracepoint handlers

In reply to Arun Kalyanasundaram

The process_event python hook receives a dict with all perf_sample entries, but the tracepoint specific and trace_unhandled hooks predate the introduction of this dict, and do not receive it.

Add the aforementioned dict as an additional argument to the affected handlers. To keep backwards compatibility (and avoid unnecessary work), do not pass the dict if the number of arguments signals that handler version predates this change.

Signed-off-by Arun Kalyanasundaram <[email protected]>

---
 .../util/scripting-engines/trace-event-python.c    | 37 +++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

tools/perf/util/scripting-engines/trace-event-python.c +36 -1

index 69d1b6db96f6..a23f5940f826 100644
@@ -116,6 +116,34 @@ static PyObject *get_handler(const char *handler_name)
 	return handler;
 }
 
+static int get_argument_count(PyObject *handler)
+{
+	int arg_count = 0;
+
+	/*
+	 * The attribute for the code object is func_code in Python 2,
+	 * whereas it is __code__ in Python 3.0+.
+	 */
+	PyObject *code_obj = PyObject_GetAttrString(handler,
+		"func_code");
+	if (PyErr_Occurred()) {
+		PyErr_Clear();
+		code_obj = PyObject_GetAttrString(handler,
+			"__code__");
+	}
+	PyErr_Clear();
+	if (code_obj) {
+		PyObject *arg_count_obj = PyObject_GetAttrString(code_obj,
+			"co_argcount");
+		if (arg_count_obj) {
+			arg_count = (int) PyInt_AsLong(arg_count_obj);
+			Py_DECREF(arg_count_obj);
+		}
+		Py_DECREF(code_obj);
+	}
+	return arg_count;
+}
+
 static void call_object(PyObject *handler, PyObject *args, const char *die_msg)
 {
 	PyObject *retval;

@@ -448,7 +476,7 @@ static void python_process_tracepoint(struct perf_sample *sample,
 {
 	struct event_format *event = evsel->tp_format;
 	PyObject *handler, *context, *t, *obj = NULL, *callchain;
-	PyObject *dict = NULL;
+	PyObject *dict = NULL, *all_entries_dict = NULL;
 	static char handler_name[256];
 	struct format_field *field;
 	unsigned long s, ns;

@@ -551,6 +579,12 @@ static void python_process_tracepoint(struct perf_sample *sample,
 	if (dict)
 		PyTuple_SetItem(t, n++, dict);
 
+	if (get_argument_count(handler) == (int) n + 1) {
+		all_entries_dict = get_perf_sample_dict(sample, evsel, al,
+			callchain);
+		PyTuple_SetItem(t, n++,	all_entries_dict);
+	}
+
 	if (_PyTuple_Resize(&t, n) == -1)
 		Py_FatalError("error resizing Python tuple");

@@ -561,6 +595,7 @@ static void python_process_tracepoint(struct perf_sample *sample,
 		Py_DECREF(dict);
 	}
 
+	Py_XDECREF(all_entries_dict);
 	Py_DECREF(t);
 }

Signature · 1 line

2.13.2.932.g7449e964c-goog

Arun Kalyanasundaram

author

2017-07-17 22:11 UTC #4 lore raw

RFC 4/4 perf script python: Generate hooks with additional argument

In reply to Arun Kalyanasundaram

Modify the signature of tracepoint specific and trace_unhandled hooks to add the perf_sample dict as a new argument.
Create a python helper function to print a dictionary.

Signed-off-by Arun Kalyanasundaram <[email protected]>

---
 .../perf/util/scripting-engines/trace-event-python.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

tools/perf/util/scripting-engines/trace-event-python.c +17 -3

index a23f5940f826..2fef0cb0383c 100644
@@ -1312,6 +1312,12 @@ static int python_generate_script(struct pevent *pevent, const char *outfile)
 
 			fprintf(ofp, "%s", f->name);
 		}
+		if (not_first++)
+			fprintf(ofp, ", ");
+		if (++count % 5 == 0)
+			fprintf(ofp, "\n\t\t");
+		fprintf(ofp, "perf_sample_dict");
+
 		fprintf(ofp, "):\n");
 
 		fprintf(ofp, "\t\tprint_header(event_name, common_cpu, "

@@ -1381,6 +1387,9 @@ static int python_generate_script(struct pevent *pevent, const char *outfile)
 
 		fprintf(ofp, ")\n\n");
 
+		fprintf(ofp, "\t\tprint 'Sample: {'+"
+			"get_dict_as_string(perf_sample_dict['sample'], ', ')+'}'\n\n");
+
 		fprintf(ofp, "\t\tfor node in common_callchain:");
 		fprintf(ofp, "\n\t\t\tif 'sym' in node:");
 		fprintf(ofp, "\n\t\t\t\tprint \"\\t[%%x] %%s\" %% (node['ip'], node['sym']['name'])");

@@ -1391,16 +1400,21 @@ static int python_generate_script(struct pevent *pevent, const char *outfile)
 	}
 
 	fprintf(ofp, "def trace_unhandled(event_name, context, "
-		"event_fields_dict):\n");
+		"event_fields_dict, perf_sample_dict):\n");
 
-	fprintf(ofp, "\t\tprint ' '.join(['%%s=%%s'%%(k,str(v))"
-		"for k,v in sorted(event_fields_dict.items())])\n\n");
+	fprintf(ofp, "\t\tprint get_dict_as_string(event_fields_dict)\n");
+	fprintf(ofp, "\t\tprint 'Sample: {'+"
+		"get_dict_as_string(perf_sample_dict['sample'], ', ')+'}'\n\n");
 
 	fprintf(ofp, "def print_header("
 		"event_name, cpu, secs, nsecs, pid, comm):\n"
 		"\tprint \"%%-20s %%5u %%05u.%%09u %%8u %%-20s \" %% \\\n\t"
 		"(event_name, cpu, secs, nsecs, pid, comm),\n");
 
+	fprintf(ofp, "def get_dict_as_string(a_dict, delimiter=' '):\n"
+		"\treturn delimiter.join"
+		"(['%%s=%%s'%%(k,str(v))for k,v in sorted(a_dict.items())])");
+
 	fclose(ofp);
 
 	fprintf(stderr, "generated Python script: %s\n", fname);

Signature · 1 line

2.13.2.932.g7449e964c-goog

Jiri Olsa

2017-07-18 08:35 UTC #5 lore raw

In reply to Arun Kalyanasundaram

Quoted 8 lines · Arun Kalyanasundaram

The process_event python hook receives a dict with all perf_sample entries.
Other handlers (e.g. trace_unhandled, python_process_tracepoint) predate the introduction of this dict and do not receive it. This patch series adds the dict to all handlers, aiming to unify the information passed to them.
This change adds an additional argument to the affected handlers. To keep backwards compatibility (and avoid unnecessary work), do not pass the aforementioned dict if the number of arguments signals that handler version predates this change.
Initial Discussion: https://lkml.org/lkml/2017/7/1/108

Quoted 8 lines

Arun Kalyanasundaram (4):
perf script python: Allocate memory only if handler exists perf script python: Refactor creation of perf sample dict perf script python: Add perf_sample dict to tracepoint handlers perf script python: Generate hooks with additional argument
 .../util/scripting-engines/trace-event-python.c    | 189 ++++++++++++++-------
 1 file changed, 128 insertions(+), 61 deletions(-)

looks good, but any idea the perf-script.py output gives some trash at the end of 'comm' args? like:

sched__sched_stat_runtime 1 199971.010182869 21999 perf comm=perf^@^@-x86_64-l^@, pid=21999

thanks,
jirka

Arun Kalyanasundaram

author

2017-07-18 18:18 UTC #6 lore raw

Thank you for taking a look into it. I don't see the garbled output. Can you please send me the perf.data file or a way to reproduce it?

Quoted 26 lines · Jiri Olsa

Quoted 8 lines · Arun Kalyanasundaram
The process_event python hook receives a dict with all perf_sample entries.
Other handlers (e.g. trace_unhandled, python_process_tracepoint) predate the introduction of this dict and do not receive it. This patch series adds the dict to all handlers, aiming to unify the information passed to them.
This change adds an additional argument to the affected handlers. To keep backwards compatibility (and avoid unnecessary work), do not pass the aforementioned dict if the number of arguments signals that handler version predates this change.
Initial Discussion: https://lkml.org/lkml/2017/7/1/108
Quoted 8 lines
Arun Kalyanasundaram (4):
perf script python: Allocate memory only if handler exists perf script python: Refactor creation of perf sample dict perf script python: Add perf_sample dict to tracepoint handlers perf script python: Generate hooks with additional argument
 .../util/scripting-engines/trace-event-python.c    | 189 ++++++++++++++-------
 1 file changed, 128 insertions(+), 61 deletions(-)
looks good, but any idea the perf-script.py output gives some trash at the end of 'comm' args? like:
sched__sched_stat_runtime 1 199971.010182869 21999 perf comm=perf^@^@-x86_64-l^@, pid=21999
thanks,
jirka