[issue13788] os.closerange optimization

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Code needed in a modern patch:

1) Use the system call if compile time configure detected it may be available.  
(if we just use syscall() rather than a libc wrapper, a configure check may not 
be necessary, but various #ifdefs likely are)
2) If (1) produces an error saying it isn't available at runtime (ie: running 
on a kernel that doesn't support it, regardless of where we were built), fall 
back to another approach (3)
3) if iterating over /proc/$pid/fd works at runtime, use that; else (4)
4) the existing brute force code.

--
stage:  -> needs patch
versions: +Python 3.11 -Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[William Manley]

William Manley  added the comment:

Linux has a close_range syscall since v5.9 (Oct 2020): 
https://man7.org/linux/man-pages/man2/close_range.2.html

--
nosy: +wmanley

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Gregory P. Smith]

Change by Gregory P. Smith :


--
versions: +Python 3.8 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[STINNER Victor]

STINNER Victor  added the comment:

See also bpo-38061: "FreeBSD: Optimize subprocess.Popen(close_fds=True) using 
closefrom()".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Gregory P. Smith]

Changes by Gregory P. Smith g...@krypto.org:


--
assignee: gregory.p.smith - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Ronald Oussoren]

Ronald Oussoren added the comment:

Two small technical comments:

1) I'd add a configure or compile-time check to determine if the procfs
   interface might be available. I don't like probing for features that
   you know are not available.

2) MacOSX has simular functionality using /dev/fd instead of
   /proc/${PID}/fd  (and other BSD systems might have this as well)

--
nosy: +ronaldoussoren

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Christian Heimes]

Christian Heimes added the comment:

In case someone is wondering if the approach really reduces the amount of 
syscalls: yes, it does. readdir() doesn't do a syscall for each entry. On Linux 
it uses the internal syscall getdents() to fill a buffer of directory entry 
structs. http://man7.org/linux/man-pages/man2/getdents.2.html

On my system os.listdir() does four syscalls:

$ strace python -c import os; os.listdir('/home/heimes')

openat(AT_FDCWD, /home/heimes, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 381 entries */, 32768)   = 12880
getdents(3, /* 0 entries */, 32768) = 0
close(3)

On Linux you can also use /proc/self/fd instead of /proc/YOURPID/fd.

Other operating systems have different APIs to get a list of open FDs. AFAK 
/dev/fd is static on FreeBSD and Mac OS X:

FreeBSD:
  
http://www.manualpages.de/FreeBSD/FreeBSD-7.4-RELEASE/man3/kinfo_getfile.3.html

Darwin / Mac OS X:
  proc_pidinfo()

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Gregory P. Smith]

Gregory P. Smith added the comment:

_posixsubprocess already uses the Linux getdent64 syscall when available 
(though for different reasons: readdir is not safe in that context). 
http://hg.python.org/cpython/file/3f3cbfd52f94/Modules/_posixsubprocess.c#l227

Probing for procfs at configure time could be problematic. It is a virtual 
filesystem. It is entirely possible for a system to choose not to mount it. It 
might be reasonable to assume that it might be present only if the system had 
it mounted at compile time but a configure flag to override that might be 
desirable for some systems (not the Linux systems I usually deal with).

If we're going through all of these hoops for closerange: I'd love to see an 
API exposed in the os module to return a list of open fd's. It is an 
abstraction nobody should have to write for themselves.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[STINNER Victor]

STINNER Victor added the comment:

FreeBSD and other OSes provide closefrom(). Why not exposing this function 
which is probably implemented as a single syscall?

--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Christian Heimes]

Changes by Christian Heimes li...@cheimes.de:


--
nosy: +christian.heimes
stage: committed/rejected - 
versions: +Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Charles-François Natali]

Changes by Charles-François Natali neolo...@free.fr:


--
resolution:  - duplicate
stage:  - committed/rejected
status: open - closed
superseder:  - subprocess close_fds behavior should only close open fds

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Georg Brandl]

Georg Brandl ge...@python.org added the comment:

fwiw, s/MSDOS_WINDOWS/MS_WINDOWS/.

--
nosy: +georg.brandl

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Gregory P. Smith]

Gregory P. Smith g...@krypto.org added the comment:

Reopening.  Comments added to the code review.

This issue is independent of the subprocess module issue in #8052.  The 
_posixsubprocess.c has its own fd closing loop.

 http://hg.python.org/cpython/file/050c07b31192/Modules/_posixsubprocess.c#l118

--
assignee:  - gregory.p.smith
nosy: +gregory.p.smith
resolution: duplicate - 
status: closed - open
superseder: subprocess close_fds behavior should only close open fds - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Ferringb]

New submission from Ferringb ferri...@gmail.com:

The current implementation of closerange essentially is a bruteforce invocation 
of close for every integer in the range.

While this works, it's rather noisy for stracing, and for most invocations, is 
near a thousand close invocations more than needed.

As such it should be aware of /proc/${PID}/fd, and use that to isolate down 
just what is actually open, and close that.

--
components: Extension Modules
files: closerange-optimization.patch
keywords: patch
messages: 151273
nosy: ferringb
priority: normal
severity: normal
status: open
title: os.closerange optimization
type: performance
Added file: http://bugs.python.org/file24241/closerange-optimization.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Ferringb]

Ferringb ferri...@gmail.com added the comment:

Fixed tabs/spaces...

--
Added file: http://bugs.python.org/file24242/closerange-optimization.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Ferringb]

Changes by Ferringb ferri...@gmail.com:


Removed file: http://bugs.python.org/file24241/closerange-optimization.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13788] os.closerange optimization

[Ross Lagerwall]

Ross Lagerwall rosslagerw...@gmail.com added the comment:

Thanks for the patch.

However, this cannot as far as I understand be used for the subprocess 
implementation due to the limitation of what can be called after a fork() and 
before an exec().

Take a look at #8052 for some more discussion of this.

--
nosy: +rosslagerwall

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13788
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com