Created attachment 209497 [details] svn(1) diff against the ports tree As described in great detail elsewhere, Python subprocess will currently loop over every single invalid fd we can give it and attempt to close(2) it. This is very bad for performance, and is absolutely excruciating when trying to debug applications driven by Python. Python has already stated that they will adopt close_range after Linux merges it, and we will also have an implementation pending in https://reviews.freebsd.org/D21627 for when this happens. We no longer need to come up with an upstreamable solution, as they've chosen the ultimate one and now it's just a waiting game. In the meantime, everyone on FreeBSD will benefit by applying these patches. They will become naturally invalidated as close_range support trickles in and we can just remove them as it happens. Default limits will likely be lowered in base soon to help alleviate the problem, but we'll still be executing ~975+ syscalls that don't need to happen (and make truss(1) sad). Patches based on emaste/cem contributions in PR 221700. Q/A: - Looks good
CC'ing python@
Thanks for this Kyle
Created attachment 209518 [details] svn(1) diff against the ports tree Re-roll all patches, by request of koobs@, to use an #ifdef __FreeBSD__
A commit references this bug: Author: koobs Date: Fri Nov 29 10:55:03 UTC 2019 New revision: 518640 URL: https://svnweb.freebsd.org/changeset/ports/518640 Log: 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) Changes: head/lang/python27/Makefile head/lang/python27/files/patch-Modules_posixmodule.c head/lang/python35/Makefile head/lang/python35/files/patch-Modules___posixsubprocess.c head/lang/python35/files/patch-Modules_posixmodule.c head/lang/python36/Makefile head/lang/python36/files/patch-Modules___posixsubprocess.c head/lang/python36/files/patch-Modules_posixmodule.c head/lang/python37/Makefile head/lang/python37/files/patch-Modules___posixsubprocess.c head/lang/python37/files/patch-Modules_posixmodule.c head/lang/python38/Makefile head/lang/python38/files/patch-Modules___posixsubprocess.c head/lang/python38/files/patch-Modules_posixmodule.c
Committed, with minor changes (added patch comments), and forgot to mention "MFH: No" in the commit log. Thank you Kyle for the final patch I'll proceed to usher this upstream on https://bugs.python.org/issue38061
MARKED AS SPAM