[PATCH v1 1/2] perf python: Remove python 2 scripting support

Ian Rogers posted 2 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by Ian Rogers 2 months, 1 week ago
Python2 was deprecated 4 years ago, remove support and workarounds.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 .../scripts/python/Perf-Trace-Util/Context.c  | 18 -----
 tools/perf/util/python.c                      | 73 +++----------------
 .../scripting-engines/trace-event-python.c    | 63 +---------------
 3 files changed, 15 insertions(+), 139 deletions(-)

diff --git a/tools/perf/scripts/python/Perf-Trace-Util/Context.c b/tools/perf/scripts/python/Perf-Trace-Util/Context.c
index 3954bd1587ce..6d1c6be1d918 100644
--- a/tools/perf/scripts/python/Perf-Trace-Util/Context.c
+++ b/tools/perf/scripts/python/Perf-Trace-Util/Context.c
@@ -23,16 +23,6 @@
 #include "../../../util/srcline.h"
 #include "../../../util/srccode.h"
 
-#if PY_MAJOR_VERSION < 3
-#define _PyCapsule_GetPointer(arg1, arg2) \
-  PyCObject_AsVoidPtr(arg1)
-#define _PyBytes_FromStringAndSize(arg1, arg2) \
-  PyString_FromStringAndSize((arg1), (arg2))
-#define _PyUnicode_AsUTF8(arg) \
-  PyString_AsString(arg)
-
-PyMODINIT_FUNC initperf_trace_context(void);
-#else
 #define _PyCapsule_GetPointer(arg1, arg2) \
   PyCapsule_GetPointer((arg1), (arg2))
 #define _PyBytes_FromStringAndSize(arg1, arg2) \
@@ -41,7 +31,6 @@ PyMODINIT_FUNC initperf_trace_context(void);
   PyUnicode_AsUTF8(arg)
 
 PyMODINIT_FUNC PyInit_perf_trace_context(void);
-#endif
 
 static struct scripting_context *get_args(PyObject *args, const char *name, PyObject **arg2)
 {
@@ -202,12 +191,6 @@ static PyMethodDef ContextMethods[] = {
 	{ NULL, NULL, 0, NULL}
 };
 
-#if PY_MAJOR_VERSION < 3
-PyMODINIT_FUNC initperf_trace_context(void)
-{
-	(void) Py_InitModule("perf_trace_context", ContextMethods);
-}
-#else
 PyMODINIT_FUNC PyInit_perf_trace_context(void)
 {
 	static struct PyModuleDef moduledef = {
@@ -229,4 +212,3 @@ PyMODINIT_FUNC PyInit_perf_trace_context(void)
 
 	return mod;
 }
-#endif
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 31a223eaf8e6..02279ab4967c 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -25,40 +25,14 @@
 #include <internal/lib.h>
 #include "../builtin.h"
 
-#if PY_MAJOR_VERSION < 3
-#define _PyUnicode_FromString(arg) \
-  PyString_FromString(arg)
-#define _PyUnicode_AsString(arg) \
-  PyString_AsString(arg)
-#define _PyUnicode_FromFormat(...) \
-  PyString_FromFormat(__VA_ARGS__)
-#define _PyLong_FromLong(arg) \
-  PyInt_FromLong(arg)
-
-#else
-
 #define _PyUnicode_FromString(arg) \
   PyUnicode_FromString(arg)
 #define _PyUnicode_FromFormat(...) \
   PyUnicode_FromFormat(__VA_ARGS__)
 #define _PyLong_FromLong(arg) \
   PyLong_FromLong(arg)
-#endif
 
-#ifndef Py_TYPE
-#define Py_TYPE(ob) (((PyObject*)(ob))->ob_type)
-#endif
-
-/* Define PyVarObject_HEAD_INIT for python 2.5 */
-#ifndef PyVarObject_HEAD_INIT
-# define PyVarObject_HEAD_INIT(type, size) PyObject_HEAD_INIT(type) size,
-#endif
-
-#if PY_MAJOR_VERSION < 3
-PyMODINIT_FUNC initperf(void);
-#else
 PyMODINIT_FUNC PyInit_perf(void);
-#endif
 
 #define member_def(type, member, ptype, help) \
 	{ #member, ptype, \
@@ -116,7 +90,7 @@ static PyObject *pyrf_mmap_event__repr(struct pyrf_event *pevent)
 		     pevent->event.mmap.pgoff, pevent->event.mmap.filename) < 0) {
 		ret = PyErr_NoMemory();
 	} else {
-		ret = _PyUnicode_FromString(s);
+		ret = PyUnicode_FromString(s);
 		free(s);
 	}
 	return ret;
@@ -147,7 +121,7 @@ static PyMemberDef pyrf_task_event__members[] = {
 
 static PyObject *pyrf_task_event__repr(struct pyrf_event *pevent)
 {
-	return _PyUnicode_FromFormat("{ type: %s, pid: %u, ppid: %u, tid: %u, "
+	return PyUnicode_FromFormat("{ type: %s, pid: %u, ppid: %u, tid: %u, "
 				   "ptid: %u, time: %" PRI_lu64 "}",
 				   pevent->event.header.type == PERF_RECORD_FORK ? "fork" : "exit",
 				   pevent->event.fork.pid,
@@ -180,7 +154,7 @@ static PyMemberDef pyrf_comm_event__members[] = {
 
 static PyObject *pyrf_comm_event__repr(struct pyrf_event *pevent)
 {
-	return _PyUnicode_FromFormat("{ type: comm, pid: %u, tid: %u, comm: %s }",
+	return PyUnicode_FromFormat("{ type: comm, pid: %u, tid: %u, comm: %s }",
 				   pevent->event.comm.pid,
 				   pevent->event.comm.tid,
 				   pevent->event.comm.comm);
@@ -211,7 +185,7 @@ static PyObject *pyrf_throttle_event__repr(struct pyrf_event *pevent)
 {
 	struct perf_record_throttle *te = (struct perf_record_throttle *)(&pevent->event.header + 1);
 
-	return _PyUnicode_FromFormat("{ type: %sthrottle, time: %" PRI_lu64 ", id: %" PRI_lu64
+	return PyUnicode_FromFormat("{ type: %sthrottle, time: %" PRI_lu64 ", id: %" PRI_lu64
 				   ", stream_id: %" PRI_lu64 " }",
 				   pevent->event.header.type == PERF_RECORD_THROTTLE ? "" : "un",
 				   te->time, te->id, te->stream_id);
@@ -246,7 +220,7 @@ static PyObject *pyrf_lost_event__repr(struct pyrf_event *pevent)
 		     pevent->event.lost.id, pevent->event.lost.lost) < 0) {
 		ret = PyErr_NoMemory();
 	} else {
-		ret = _PyUnicode_FromString(s);
+		ret = PyUnicode_FromString(s);
 		free(s);
 	}
 	return ret;
@@ -273,7 +247,7 @@ static PyMemberDef pyrf_read_event__members[] = {
 
 static PyObject *pyrf_read_event__repr(struct pyrf_event *pevent)
 {
-	return _PyUnicode_FromFormat("{ type: read, pid: %u, tid: %u }",
+	return PyUnicode_FromFormat("{ type: read, pid: %u, tid: %u }",
 				   pevent->event.read.pid,
 				   pevent->event.read.tid);
 	/*
@@ -308,7 +282,7 @@ static PyObject *pyrf_sample_event__repr(struct pyrf_event *pevent)
 	if (asprintf(&s, "{ type: sample }") < 0) {
 		ret = PyErr_NoMemory();
 	} else {
-		ret = _PyUnicode_FromString(s);
+		ret = PyUnicode_FromString(s);
 		free(s);
 	}
 	return ret;
@@ -342,7 +316,7 @@ tracepoint_field(struct pyrf_event *pe, struct tep_format_field *field)
 		}
 		if (field->flags & TEP_FIELD_IS_STRING &&
 		    is_printable_array(data + offset, len)) {
-			ret = _PyUnicode_FromString((char *)data + offset);
+			ret = PyUnicode_FromString((char *)data + offset);
 		} else {
 			ret = PyByteArray_FromStringAndSize((const char *) data + offset, len);
 			field->flags &= ~TEP_FIELD_IS_STRING;
@@ -431,7 +405,7 @@ static PyObject *pyrf_context_switch_event__repr(struct pyrf_event *pevent)
 		     !!(pevent->event.header.misc & PERF_RECORD_MISC_SWITCH_OUT)) < 0) {
 		ret = PyErr_NoMemory();
 	} else {
-		ret = _PyUnicode_FromString(s);
+		ret = PyUnicode_FromString(s);
 		free(s);
 	}
 	return ret;
@@ -917,17 +891,8 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 
 	for (i = 0; i < evlist->core.pollfd.nr; ++i) {
 		PyObject *file;
-#if PY_MAJOR_VERSION < 3
-		FILE *fp = fdopen(evlist->core.pollfd.entries[i].fd, "r");
-
-		if (fp == NULL)
-			goto free_list;
-
-		file = PyFile_FromFile(fp, "perf", "r", NULL);
-#else
 		file = PyFile_FromFd(evlist->core.pollfd.entries[i].fd, "perf", "r", -1,
 				     NULL, NULL, NULL, 0);
-#endif
 		if (file == NULL)
 			goto free_list;
 
@@ -1233,9 +1198,9 @@ static PyObject *pyrf__tracepoint(struct pyrf_evsel *pevsel,
 
 	tp_format = trace_event__tp_format(sys, name);
 	if (IS_ERR(tp_format))
-		return _PyLong_FromLong(-1);
+		return PyLong_FromLong(-1);
 
-	return _PyLong_FromLong(tp_format->id);
+	return PyLong_FromLong(tp_format->id);
 #endif // HAVE_LIBTRACEEVENT
 }
 
@@ -1249,18 +1214,11 @@ static PyMethodDef perf__methods[] = {
 	{ .ml_name = NULL, }
 };
 
-#if PY_MAJOR_VERSION < 3
-PyMODINIT_FUNC initperf(void)
-#else
 PyMODINIT_FUNC PyInit_perf(void)
-#endif
 {
 	PyObject *obj;
 	int i;
 	PyObject *dict;
-#if PY_MAJOR_VERSION < 3
-	PyObject *module = Py_InitModule("perf", perf__methods);
-#else
 	static struct PyModuleDef moduledef = {
 		PyModuleDef_HEAD_INIT,
 		"perf",			/* m_name */
@@ -1273,7 +1231,6 @@ PyMODINIT_FUNC PyInit_perf(void)
 		NULL,			/* m_free */
 	};
 	PyObject *module = PyModule_Create(&moduledef);
-#endif
 
 	if (module == NULL ||
 	    pyrf_event__setup_types() < 0 ||
@@ -1281,11 +1238,7 @@ PyMODINIT_FUNC PyInit_perf(void)
 	    pyrf_evsel__setup_types() < 0 ||
 	    pyrf_thread_map__setup_types() < 0 ||
 	    pyrf_cpu_map__setup_types() < 0)
-#if PY_MAJOR_VERSION < 3
-		return;
-#else
 		return module;
-#endif
 
 	/* The page_size is placed in util object. */
 	page_size = sysconf(_SC_PAGE_SIZE);
@@ -1334,7 +1287,7 @@ PyMODINIT_FUNC PyInit_perf(void)
 		goto error;
 
 	for (i = 0; perf__constants[i].name != NULL; i++) {
-		obj = _PyLong_FromLong(perf__constants[i].value);
+		obj = PyLong_FromLong(perf__constants[i].value);
 		if (obj == NULL)
 			goto error;
 		PyDict_SetItemString(dict, perf__constants[i].name, obj);
@@ -1344,9 +1297,7 @@ PyMODINIT_FUNC PyInit_perf(void)
 error:
 	if (PyErr_Occurred())
 		PyErr_SetString(PyExc_ImportError, "perf: Init failed!");
-#if PY_MAJOR_VERSION >= 3
 	return module;
-#endif
 }
 
 
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index d7183134b669..f1d461d47d73 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -58,22 +58,6 @@
 #include "mem-events.h"
 #include "util/perf_regs.h"
 
-#if PY_MAJOR_VERSION < 3
-#define _PyUnicode_FromString(arg) \
-  PyString_FromString(arg)
-#define _PyUnicode_FromStringAndSize(arg1, arg2) \
-  PyString_FromStringAndSize((arg1), (arg2))
-#define _PyBytes_FromStringAndSize(arg1, arg2) \
-  PyString_FromStringAndSize((arg1), (arg2))
-#define _PyLong_FromLong(arg) \
-  PyInt_FromLong(arg)
-#define _PyLong_AsLong(arg) \
-  PyInt_AsLong(arg)
-#define _PyCapsule_New(arg1, arg2, arg3) \
-  PyCObject_FromVoidPtr((arg1), (arg2))
-
-PyMODINIT_FUNC initperf_trace_context(void);
-#else
 #define _PyUnicode_FromString(arg) \
   PyUnicode_FromString(arg)
 #define _PyUnicode_FromStringAndSize(arg1, arg2) \
@@ -88,7 +72,6 @@ PyMODINIT_FUNC initperf_trace_context(void);
   PyCapsule_New((arg1), (arg2), (arg3))
 
 PyMODINIT_FUNC PyInit_perf_trace_context(void);
-#endif
 
 #ifdef HAVE_LIBTRACEEVENT
 #define TRACE_EVENT_TYPE_MAX				\
@@ -181,17 +164,7 @@ 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__");
-	}
+	PyObject *code_obj = code_obj = PyObject_GetAttrString(handler, "__code__");
 	PyErr_Clear();
 	if (code_obj) {
 		PyObject *arg_count_obj = PyObject_GetAttrString(code_obj,
@@ -1899,12 +1872,6 @@ static void set_table_handlers(struct tables *tables)
 	tables->synth_handler = get_handler("synth_data");
 }
 
-#if PY_MAJOR_VERSION < 3
-static void _free_command_line(const char **command_line, int num)
-{
-	free(command_line);
-}
-#else
 static void _free_command_line(wchar_t **command_line, int num)
 {
 	int i;
@@ -1912,7 +1879,6 @@ static void _free_command_line(wchar_t **command_line, int num)
 		PyMem_RawFree(command_line[i]);
 	free(command_line);
 }
-#endif
 
 
 /*
@@ -1922,30 +1888,12 @@ static int python_start_script(const char *script, int argc, const char **argv,
 			       struct perf_session *session)
 {
 	struct tables *tables = &tables_global;
-#if PY_MAJOR_VERSION < 3
-	const char **command_line;
-#else
 	wchar_t **command_line;
-#endif
-	/*
-	 * Use a non-const name variable to cope with python 2.6's
-	 * PyImport_AppendInittab prototype
-	 */
-	char buf[PATH_MAX], name[19] = "perf_trace_context";
+	char buf[PATH_MAX];
 	int i, err = 0;
 	FILE *fp;
 
 	scripting_context->session = session;
-#if PY_MAJOR_VERSION < 3
-	command_line = malloc((argc + 1) * sizeof(const char *));
-	if (!command_line)
-		return -1;
-
-	command_line[0] = script;
-	for (i = 1; i < argc + 1; i++)
-		command_line[i] = argv[i - 1];
-	PyImport_AppendInittab(name, initperf_trace_context);
-#else
 	command_line = malloc((argc + 1) * sizeof(wchar_t *));
 	if (!command_line)
 		return -1;
@@ -1953,15 +1901,10 @@ static int python_start_script(const char *script, int argc, const char **argv,
 	command_line[0] = Py_DecodeLocale(script, NULL);
 	for (i = 1; i < argc + 1; i++)
 		command_line[i] = Py_DecodeLocale(argv[i - 1], NULL);
-	PyImport_AppendInittab(name, PyInit_perf_trace_context);
-#endif
+	PyImport_AppendInittab("perf_trace_context", PyInit_perf_trace_context);
 	Py_Initialize();
 
-#if PY_MAJOR_VERSION < 3
-	PySys_SetArgv(argc + 1, (char **)command_line);
-#else
 	PySys_SetArgv(argc + 1, command_line);
-#endif
 
 	fp = fopen(script, "r");
 	if (!fp) {
-- 
2.46.0.662.g92d0881bb0-goog
Re: [PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by James Clark 1 month, 4 weeks ago

On 18/09/2024 11:54 pm, Ian Rogers wrote:
> Python2 was deprecated 4 years ago, remove support and workarounds.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>   .../scripts/python/Perf-Trace-Util/Context.c  | 18 -----
>   tools/perf/util/python.c                      | 73 +++----------------
>   .../scripting-engines/trace-event-python.c    | 63 +---------------
>   3 files changed, 15 insertions(+), 139 deletions(-)
> 
> diff --git a/tools/perf/scripts/python/Perf-Trace-Util/Context.c b/tools/perf/scripts/python/Perf-Trace-Util/Context.c
> index 3954bd1587ce..6d1c6be1d918 100644
> --- a/tools/perf/scripts/python/Perf-Trace-Util/Context.c
> +++ b/tools/perf/scripts/python/Perf-Trace-Util/Context.c
> @@ -23,16 +23,6 @@
>   #include "../../../util/srcline.h"
>   #include "../../../util/srccode.h"
>   
> -#if PY_MAJOR_VERSION < 3
> -#define _PyCapsule_GetPointer(arg1, arg2) \
> -  PyCObject_AsVoidPtr(arg1)
> -#define _PyBytes_FromStringAndSize(arg1, arg2) \
> -  PyString_FromStringAndSize((arg1), (arg2))
> -#define _PyUnicode_AsUTF8(arg) \
> -  PyString_AsString(arg)
> -

If we know the workarounds were required should we add an error to 
prevent hard to debug build issues?

   #if PY_MAJOR_VERSION < 3
     #error "Python 2 not supported"
   #endif

Or maybe something in the top level makefile?
Re: [PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by Andi Kleen 2 months, 1 week ago
On Thu, Sep 19, 2024 at 12:54:17AM +0200, Ian Rogers wrote:
> Python2 was deprecated 4 years ago, remove support and workarounds.

Nacked-by: Andi Kleen

I don't see any advantages of breaking perfectly fine existing setups
for no benefits.

-Andi
Re: [PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by Ian Rogers 2 months, 1 week ago
On Thu, Sep 19, 2024 at 3:29 AM Andi Kleen <ak@linux.intel.com> wrote:
>
> On Thu, Sep 19, 2024 at 12:54:17AM +0200, Ian Rogers wrote:
> > Python2 was deprecated 4 years ago, remove support and workarounds.
>
> Nacked-by: Andi Kleen
>
> I don't see any advantages of breaking perfectly fine existing setups
> for no benefits.

The reason is that since moving to linking libraries rather than
building code we now have the ability to call functions like
parse_events - previously parse_events was stubbed out because there
was no way to build lex/yacc code. Calling parse_events is more
sensible than existing logic that does things like open a legacy
cycles event only on 1 PMU type. That is, the existing code doesn't
support hybrid. Practically there is no way to test python2 support
builds and works without having python2. We shouldn't be in the
business of installing python2 in 2024, my organization has removed
all support for over 3 years.

Thanks,
Ian
Re: [PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by Namhyung Kim 1 month, 4 weeks ago
On Thu, Sep 19, 2024 at 06:45:23AM +0200, Ian Rogers wrote:
> On Thu, Sep 19, 2024 at 3:29 AM Andi Kleen <ak@linux.intel.com> wrote:
> >
> > On Thu, Sep 19, 2024 at 12:54:17AM +0200, Ian Rogers wrote:
> > > Python2 was deprecated 4 years ago, remove support and workarounds.
> >
> > Nacked-by: Andi Kleen
> >
> > I don't see any advantages of breaking perfectly fine existing setups
> > for no benefits.

Well, I think the benefit is in the maintenance.  The EOL of Python 2
was 2020/1/1 [1] and we are targeting this release (v6.13) in 2025.  So
I think it's reasonable to consider removing Python 2 support now.

> 
> The reason is that since moving to linking libraries rather than
> building code we now have the ability to call functions like
> parse_events - previously parse_events was stubbed out because there
> was no way to build lex/yacc code. Calling parse_events is more
> sensible than existing logic that does things like open a legacy
> cycles event only on 1 PMU type. That is, the existing code doesn't
> support hybrid. Practically there is no way to test python2 support
> builds and works without having python2. We shouldn't be in the
> business of installing python2 in 2024, my organization has removed
> all support for over 3 years.

I think the problem nowdays is the container images for old distro.  I'm
not sure how long we need to support them but it should be cut out at
some point.

Thanks,
Namhyung


[1] https://www.python.org/doc/sunset-python-2/
Re: [PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by Andi Kleen 1 month, 4 weeks ago
On Mon, Sep 30, 2024 at 02:26:47PM -0700, Namhyung Kim wrote:
> On Thu, Sep 19, 2024 at 06:45:23AM +0200, Ian Rogers wrote:
> > On Thu, Sep 19, 2024 at 3:29 AM Andi Kleen <ak@linux.intel.com> wrote:
> > >
> > > On Thu, Sep 19, 2024 at 12:54:17AM +0200, Ian Rogers wrote:
> > > > Python2 was deprecated 4 years ago, remove support and workarounds.
> > >
> > > Nacked-by: Andi Kleen
> > >
> > > I don't see any advantages of breaking perfectly fine existing setups
> > > for no benefits.
> 
> Well, I think the benefit is in the maintenance.  The EOL of Python 2
> was 2020/1/1 [1] and we are targeting this release (v6.13) in 2025.  So

AFAIK it's still widely used, and supported by third parties. And even
if not it's not that the python usage in perf needs any external support.

In Linux deprecation is generally tied to nobody using something
anymore, and that is clearly not the case here.

> I think it's reasonable to consider removing Python 2 support now.

That's code that practically never changes, so I don't see any
maintenance benefit. I mean it needs to be compile tested / perf tested, 
but Arnaldo's container collection will do that practically
for free.

-Andi
Re: [PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by Ian Rogers 1 month, 4 weeks ago
On Tue, Oct 1, 2024 at 4:22 AM Andi Kleen <ak@linux.intel.com> wrote:
>
> On Mon, Sep 30, 2024 at 02:26:47PM -0700, Namhyung Kim wrote:
> > On Thu, Sep 19, 2024 at 06:45:23AM +0200, Ian Rogers wrote:
> > > On Thu, Sep 19, 2024 at 3:29 AM Andi Kleen <ak@linux.intel.com> wrote:
> > > >
> > > > On Thu, Sep 19, 2024 at 12:54:17AM +0200, Ian Rogers wrote:
> > > > > Python2 was deprecated 4 years ago, remove support and workarounds.
> > > >
> > > > Nacked-by: Andi Kleen
> > > >
> > > > I don't see any advantages of breaking perfectly fine existing setups
> > > > for no benefits.
> >
> > Well, I think the benefit is in the maintenance.  The EOL of Python 2
> > was 2020/1/1 [1] and we are targeting this release (v6.13) in 2025.  So
>
> AFAIK it's still widely used, and supported by third parties. And even
> if not it's not that the python usage in perf needs any external support.
>
> In Linux deprecation is generally tied to nobody using something
> anymore, and that is clearly not the case here.
>
> > I think it's reasonable to consider removing Python 2 support now.
>
> That's code that practically never changes, so I don't see any
> maintenance benefit. I mean it needs to be compile tested / perf tested,
> but Arnaldo's container collection will do that practically
> for free.

So of the Linux distributions that supported Python 2, the versions
that did stopped being supported in June this year.
The hypothetical person who cares about python 2 support needs to be
building their own kernel tree, they've had to upgrade GCC, etc. but
GCC isn't particularly relevant here.
The hypothetical person wants to build on their unsupported
distribution the latest and greatest perf tool. Now the perf build has
required python 3.6 for json events for 2.5 years. So this
hypothetical person who wants the latest perf tool on their
unsupported system must not care about json events and metrics but
does care about python 2 integration for scripting.
I really don't see such a person existing and if they did they can:
1) install python3
2) use an older perf tool

There is a constant cost in the code base trying to know whether
python means 2 or 3. Keeping dead code around at the very least
creates poor code coverage numbers. We want to add to the python entry
points into the code with parse_events. We want to make python a
dlopen dependency (as done in uftrace) rather than something that's
linked against. In other words python development is somewhat active
and trying to carry around python 2 support when no one we can name
wants it is just burdening us down, costing us in time and code
coverage for no gain.

Thanks,
Ian
Re: [PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by Namhyung Kim 1 month, 2 weeks ago
On Tue, Oct 01, 2024 at 06:47:47AM -0700, Ian Rogers wrote:
> On Tue, Oct 1, 2024 at 4:22 AM Andi Kleen <ak@linux.intel.com> wrote:
> >
> > On Mon, Sep 30, 2024 at 02:26:47PM -0700, Namhyung Kim wrote:
> > > On Thu, Sep 19, 2024 at 06:45:23AM +0200, Ian Rogers wrote:
> > > > On Thu, Sep 19, 2024 at 3:29 AM Andi Kleen <ak@linux.intel.com> wrote:
> > > > >
> > > > > On Thu, Sep 19, 2024 at 12:54:17AM +0200, Ian Rogers wrote:
> > > > > > Python2 was deprecated 4 years ago, remove support and workarounds.
> > > > >
> > > > > Nacked-by: Andi Kleen
> > > > >
> > > > > I don't see any advantages of breaking perfectly fine existing setups
> > > > > for no benefits.
> > >
> > > Well, I think the benefit is in the maintenance.  The EOL of Python 2
> > > was 2020/1/1 [1] and we are targeting this release (v6.13) in 2025.  So
> >
> > AFAIK it's still widely used, and supported by third parties. And even
> > if not it's not that the python usage in perf needs any external support.
> >
> > In Linux deprecation is generally tied to nobody using something
> > anymore, and that is clearly not the case here.
> >
> > > I think it's reasonable to consider removing Python 2 support now.
> >
> > That's code that practically never changes, so I don't see any
> > maintenance benefit. I mean it needs to be compile tested / perf tested,
> > but Arnaldo's container collection will do that practically
> > for free.
> 
> So of the Linux distributions that supported Python 2, the versions
> that did stopped being supported in June this year.
> The hypothetical person who cares about python 2 support needs to be
> building their own kernel tree, they've had to upgrade GCC, etc. but
> GCC isn't particularly relevant here.
> The hypothetical person wants to build on their unsupported
> distribution the latest and greatest perf tool. Now the perf build has
> required python 3.6 for json events for 2.5 years. So this
> hypothetical person who wants the latest perf tool on their
> unsupported system must not care about json events and metrics but
> does care about python 2 integration for scripting.
> I really don't see such a person existing and if they did they can:
> 1) install python3
> 2) use an older perf tool
> 
> There is a constant cost in the code base trying to know whether
> python means 2 or 3. Keeping dead code around at the very least
> creates poor code coverage numbers. We want to add to the python entry
> points into the code with parse_events. We want to make python a
> dlopen dependency (as done in uftrace) rather than something that's
> linked against. In other words python development is somewhat active
> and trying to carry around python 2 support when no one we can name
> wants it is just burdening us down, costing us in time and code
> coverage for no gain.

Yeah I think it's time to consider sunsetting Python 2.  As Ian and
James said, it was bothering some development.

Andi, do you still use it with Python 2?  Or do you know anybody who
might suffer from this change?  Is it hard to upgrade the system?

I'm inclined to pick up this change unless there's a specific reason.

Thanks,
Namhyung

Re: [PATCH v1 1/2] perf python: Remove python 2 scripting support
Posted by James Clark 1 month, 4 weeks ago

On 01/10/2024 12:22 pm, Andi Kleen wrote:
> On Mon, Sep 30, 2024 at 02:26:47PM -0700, Namhyung Kim wrote:
>> On Thu, Sep 19, 2024 at 06:45:23AM +0200, Ian Rogers wrote:
>>> On Thu, Sep 19, 2024 at 3:29 AM Andi Kleen <ak@linux.intel.com> wrote:
>>>>
>>>> On Thu, Sep 19, 2024 at 12:54:17AM +0200, Ian Rogers wrote:
>>>>> Python2 was deprecated 4 years ago, remove support and workarounds.
>>>>
>>>> Nacked-by: Andi Kleen
>>>>
>>>> I don't see any advantages of breaking perfectly fine existing setups
>>>> for no benefits.
>>
>> Well, I think the benefit is in the maintenance.  The EOL of Python 2
>> was 2020/1/1 [1] and we are targeting this release (v6.13) in 2025.  So
> 
> AFAIK it's still widely used, and supported by third parties. And even
> if not it's not that the python usage in perf needs any external support.
> 
> In Linux deprecation is generally tied to nobody using something
> anymore, and that is clearly not the case here.
> 
>> I think it's reasonable to consider removing Python 2 support now.
> 
> That's code that practically never changes, so I don't see any
> maintenance benefit. I mean it needs to be compile tested / perf tested,
> but Arnaldo's container collection will do that practically
> for free.
> 
> -Andi
> 

630af16 was quite painful to do and was related to supporting both 
python versions, so there is some ongoing cost. If we do this removal we 
could remove that one too. Plus Arnaldo could drop the python2 
containers and get a faster iteration time.

I think I'm neutral to in favor of removing python2 support. People on 
ancient systems would more likely be using an old branch anyway.

James