From e4c2b30ce8e600e2bf5f6d3e6b95c4dd27abcac4 Mon Sep 17 00:00:00 2001 From: Kubilay Kocak Date: Fri, 29 Nov 2019 10:55:00 +0000 Subject: [PATCH] lang/python{27,35,36,37,38}: Add closefrom(2) support A single close(fd) syscall is cheap, but when MAXFDS (maximum file descriptor number) is high, the loop calling close(fd) on each file descriptor can take several milliseconds. The default value of subprocess.Popen "close_fds" parameter changed to True in Python 3. Compared to Python 2, close_fds=True can make Popen 10x slower: see bpo-37790 [1] The present workaround on FreeBSD to improve performance is to load and mount the fdescfs kernel module, but this is not enabled by default. This change adds minimum viable (and upstreamable) closefrom(2) syscall support to Python's subprocess and posix modules, improving performance significantly for loads that involve working with many processes, such as diffoscope, ansible, and many others. For additional optimizations, upstream recently (3.8) landed posix_spawn(2) support [3] and has stated that they will adopt close_range(2) after Linux merges it [4]. Linux/FreeBSD developers are already collaborating on ensuring compatible implementations, with FreeBSD's implementation pending in D21627. [5] Thank you emaste, cem, kevans for providing analysis, input, clarifications, comms/upstream support and patches. [1] https://bugs.python.org/issue37790 [2] https://bugs.python.org/issue38061 [3] https://bugs.python.org/issue35537 [4] https://lwn.net/Articles/789023/ [5] https://reviews.freebsd.org/D21627 Additional References: https://bugs.python.org/issue8052 https://bugs.python.org/issue11284 https://bugs.python.org/issue13788 https://bugs.python.org/issue1663329 https://www.python.org/dev/peps/pep-0446/ PR: 242274, 221700 Submitted by: kevans (emaste, cem) Approved by: koobs (python (maintainer), santa) --- lang/python27/Makefile | 1 + .../files/patch-Modules_posixmodule.c | 27 +++++++++++++++++++ lang/python35/Makefile | 2 +- .../files/patch-Modules___posixsubprocess.c | 25 +++++++++++++++++ .../files/patch-Modules_posixmodule.c | 27 +++++++++++++++++++ lang/python36/Makefile | 1 + .../files/patch-Modules___posixsubprocess.c | 25 +++++++++++++++++ .../files/patch-Modules_posixmodule.c | 26 ++++++++++++++++++ lang/python37/Makefile | 1 + .../files/patch-Modules___posixsubprocess.c | 25 +++++++++++++++++ .../files/patch-Modules_posixmodule.c | 26 ++++++++++++++++++ lang/python38/Makefile | 2 +- .../files/patch-Modules___posixsubprocess.c | 25 +++++++++++++++++ .../files/patch-Modules_posixmodule.c | 26 ++++++++++++++++++ 14 files changed, 237 insertions(+), 2 deletions(-) create mode 100644 lang/python27/files/patch-Modules_posixmodule.c create mode 100644 lang/python35/files/patch-Modules___posixsubprocess.c create mode 100644 lang/python35/files/patch-Modules_posixmodule.c create mode 100644 lang/python36/files/patch-Modules___posixsubprocess.c create mode 100644 lang/python36/files/patch-Modules_posixmodule.c create mode 100644 lang/python37/files/patch-Modules___posixsubprocess.c create mode 100644 lang/python37/files/patch-Modules_posixmodule.c create mode 100644 lang/python38/files/patch-Modules___posixsubprocess.c create mode 100644 lang/python38/files/patch-Modules_posixmodule.c diff --git a/lang/python27/Makefile b/lang/python27/Makefile index 717f0296b46e..6ecb0882b96c 100644 --- a/lang/python27/Makefile +++ b/lang/python27/Makefile @@ -2,6 +2,7 @@ PORTNAME= python PORTVERSION= ${PYTHON_PORTVERSION} +PORTREVISION= 1 CATEGORIES= lang python MASTER_SITES= PYTHON/ftp/python/${PORTVERSION} PKGNAMESUFFIX= 27 diff --git a/lang/python27/files/patch-Modules_posixmodule.c b/lang/python27/files/patch-Modules_posixmodule.c new file mode 100644 index 000000000000..acc841e5c199 --- /dev/null +++ b/lang/python27/files/patch-Modules_posixmodule.c @@ -0,0 +1,27 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/posixmodule.c.orig 2019-10-19 18:38:44 UTC ++++ Modules/posixmodule.c +@@ -6676,9 +6676,16 @@ posix_closerange(PyObject *self, PyObject *args) + if (!PyArg_ParseTuple(args, "ii:closerange", &fd_from, &fd_to)) + return NULL; + Py_BEGIN_ALLOW_THREADS +- for (i = fd_from; i < fd_to; i++) +- if (_PyVerify_fd(i)) +- close(i); ++#ifdef __FreeBSD__ ++ if (fd_to >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(fd_from); ++ } else ++#endif ++ { ++ for (i = fd_from; i < fd_to; i++) ++ if (_PyVerify_fd(i)) ++ close(i); ++ } + Py_END_ALLOW_THREADS + Py_RETURN_NONE; + } diff --git a/lang/python35/Makefile b/lang/python35/Makefile index 981b9a5f8118..fe6fe01dfb61 100644 --- a/lang/python35/Makefile +++ b/lang/python35/Makefile @@ -3,7 +3,7 @@ PORTNAME= python PORTVERSION= ${PYTHON_PORTVERSION} -PORTREVISION= 2 +PORTREVISION= 3 CATEGORIES= lang python MASTER_SITES= PYTHON/ftp/python/${PORTVERSION} PKGNAMESUFFIX= ${PYTHON_SUFFIX} diff --git a/lang/python35/files/patch-Modules___posixsubprocess.c b/lang/python35/files/patch-Modules___posixsubprocess.c new file mode 100644 index 000000000000..54e31f476b3b --- /dev/null +++ b/lang/python35/files/patch-Modules___posixsubprocess.c @@ -0,0 +1,25 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/_posixsubprocess.c.orig 2019-11-01 23:02:34 UTC ++++ Modules/_posixsubprocess.c +@@ -233,8 +233,15 @@ _close_fds_by_brute_force(long start_fd, PyObject *py_ + start_fd = keep_fd + 1; + } + if (start_fd <= end_fd) { +- for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { +- close(fd_num); ++#ifdef __FreeBSD__ ++ if (end_fd >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(start_fd); ++ } else ++#endif ++ { ++ for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { ++ close(fd_num); ++ } + } + } + } diff --git a/lang/python35/files/patch-Modules_posixmodule.c b/lang/python35/files/patch-Modules_posixmodule.c new file mode 100644 index 000000000000..169075797556 --- /dev/null +++ b/lang/python35/files/patch-Modules_posixmodule.c @@ -0,0 +1,27 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/posixmodule.c.orig 2019-11-01 23:02:34 UTC ++++ Modules/posixmodule.c +@@ -7853,9 +7853,16 @@ os_closerange_impl(PyObject *module, int fd_low, int f + int i; + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH +- for (i = fd_low; i < fd_high; i++) +- if (_PyVerify_fd(i)) +- close(i); ++#ifdef __FreeBSD__ ++ if (fd_high >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(fd_low); ++ } else ++#endif ++ { ++ for (i = fd_low; i < fd_high; i++) ++ if (_PyVerify_fd(i)) ++ close(i); ++ } + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS + Py_RETURN_NONE; diff --git a/lang/python36/Makefile b/lang/python36/Makefile index 81c827a9994a..0ec8d034230e 100644 --- a/lang/python36/Makefile +++ b/lang/python36/Makefile @@ -3,6 +3,7 @@ PORTNAME= python PORTVERSION= ${PYTHON_PORTVERSION} +PORTREVISION= 1 CATEGORIES= lang python MASTER_SITES= PYTHON/ftp/python/${PORTVERSION} PKGNAMESUFFIX= ${PYTHON_SUFFIX} diff --git a/lang/python36/files/patch-Modules___posixsubprocess.c b/lang/python36/files/patch-Modules___posixsubprocess.c new file mode 100644 index 000000000000..01542ecb9e85 --- /dev/null +++ b/lang/python36/files/patch-Modules___posixsubprocess.c @@ -0,0 +1,25 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/_posixsubprocess.c.orig 2019-07-02 20:25:39 UTC ++++ Modules/_posixsubprocess.c +@@ -236,8 +236,15 @@ _close_fds_by_brute_force(long start_fd, PyObject *py_ + start_fd = keep_fd + 1; + } + if (start_fd <= end_fd) { +- for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { +- close(fd_num); ++#ifdef __FreeBSD__ ++ if (end_fd >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(start_fd); ++ } else ++#endif ++ { ++ for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { ++ close(fd_num); ++ } + } + } + } diff --git a/lang/python36/files/patch-Modules_posixmodule.c b/lang/python36/files/patch-Modules_posixmodule.c new file mode 100644 index 000000000000..a0247dcc628d --- /dev/null +++ b/lang/python36/files/patch-Modules_posixmodule.c @@ -0,0 +1,26 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/posixmodule.c.orig 2019-07-02 20:25:39 UTC ++++ Modules/posixmodule.c +@@ -7685,8 +7685,16 @@ os_closerange_impl(PyObject *module, int fd_low, int f + int i; + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH +- for (i = Py_MAX(fd_low, 0); i < fd_high; i++) +- close(i); ++ fd_low = Py_MAX(fd_low, 0); ++#ifdef __FreeBSD__ ++ if (fd_high >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(fd_low); ++ } else ++#endif ++ { ++ for (i = fd_low; i < fd_high; i++) ++ close(i); ++ } + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS + Py_RETURN_NONE; diff --git a/lang/python37/Makefile b/lang/python37/Makefile index 8d67478e446d..6d56e367d1fc 100644 --- a/lang/python37/Makefile +++ b/lang/python37/Makefile @@ -3,6 +3,7 @@ PORTNAME= python PORTVERSION= ${PYTHON_PORTVERSION} +PORTREVISION= 1 CATEGORIES= lang python MASTER_SITES= PYTHON/ftp/python/${PORTVERSION} PKGNAMESUFFIX= ${PYTHON_SUFFIX} diff --git a/lang/python37/files/patch-Modules___posixsubprocess.c b/lang/python37/files/patch-Modules___posixsubprocess.c new file mode 100644 index 000000000000..f46b682378bd --- /dev/null +++ b/lang/python37/files/patch-Modules___posixsubprocess.c @@ -0,0 +1,25 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/_posixsubprocess.c.orig 2019-10-14 22:32:36 UTC ++++ Modules/_posixsubprocess.c +@@ -236,8 +236,15 @@ _close_fds_by_brute_force(long start_fd, PyObject *py_ + start_fd = keep_fd + 1; + } + if (start_fd <= end_fd) { +- for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { +- close(fd_num); ++#ifdef __FreeBSD__ ++ if (end_fd >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(start_fd); ++ } else ++#endif ++ { ++ for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { ++ close(fd_num); ++ } + } + } + } diff --git a/lang/python37/files/patch-Modules_posixmodule.c b/lang/python37/files/patch-Modules_posixmodule.c new file mode 100644 index 000000000000..28411c3126dd --- /dev/null +++ b/lang/python37/files/patch-Modules_posixmodule.c @@ -0,0 +1,26 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/posixmodule.c.orig 2019-10-14 22:32:36 UTC ++++ Modules/posixmodule.c +@@ -7810,8 +7810,16 @@ os_closerange_impl(PyObject *module, int fd_low, int f + int i; + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH +- for (i = Py_MAX(fd_low, 0); i < fd_high; i++) +- close(i); ++ fd_low = Py_MAX(fd_low, 0); ++#ifdef __FreeBSD__ ++ if (fd_high >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(fd_low); ++ } else ++#endif ++ { ++ for (i = fd_low; i < fd_high; i++) ++ close(i); ++ } + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS + Py_RETURN_NONE; diff --git a/lang/python38/Makefile b/lang/python38/Makefile index 825d2876f164..20383781a1cc 100644 --- a/lang/python38/Makefile +++ b/lang/python38/Makefile @@ -3,7 +3,7 @@ PORTNAME= python PORTVERSION= ${PYTHON_PORTVERSION} -PORTREVISION= 1 +PORTREVISION= 2 CATEGORIES= lang python MASTER_SITES= PYTHON/ftp/python/${PORTVERSION} PKGNAMESUFFIX= ${PYTHON_SUFFIX} diff --git a/lang/python38/files/patch-Modules___posixsubprocess.c b/lang/python38/files/patch-Modules___posixsubprocess.c new file mode 100644 index 000000000000..a7c4e43e81cf --- /dev/null +++ b/lang/python38/files/patch-Modules___posixsubprocess.c @@ -0,0 +1,25 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/_posixsubprocess.c.orig 2019-10-14 13:34:47 UTC ++++ Modules/_posixsubprocess.c +@@ -236,8 +236,15 @@ _close_fds_by_brute_force(long start_fd, PyObject *py_ + start_fd = keep_fd + 1; + } + if (start_fd <= end_fd) { +- for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { +- close(fd_num); ++#if defined(__FreeBSD__) ++ if (end_fd >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(start_fd); ++ } else ++#endif ++ { ++ for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { ++ close(fd_num); ++ } + } + } + } diff --git a/lang/python38/files/patch-Modules_posixmodule.c b/lang/python38/files/patch-Modules_posixmodule.c new file mode 100644 index 000000000000..441c78f81016 --- /dev/null +++ b/lang/python38/files/patch-Modules_posixmodule.c @@ -0,0 +1,26 @@ +# Add closefrom(2) support +# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242274 +# https://bugs.python.org/issue38061 +# TODO: Upstream + +--- Modules/posixmodule.c.orig 2019-10-14 13:34:47 UTC ++++ Modules/posixmodule.c +@@ -8460,8 +8460,16 @@ os_closerange_impl(PyObject *module, int fd_low, int f + lohi[1] = fd_high; + fdwalk(_fdwalk_close_func, lohi); + #else +- for (i = Py_MAX(fd_low, 0); i < fd_high; i++) +- close(i); ++ fd_low = Py_MAX(fd_low, 0); ++#ifdef __FreeBSD__ ++ if (fd_high >= sysconf(_SC_OPEN_MAX)) { ++ closefrom(fd_low); ++ } else ++#endif ++ { ++ for (i = fd_low; i < fd_high; i++) ++ close(i); ++ } + #endif + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS