I have constructed a script to make use of Python's internal benchmarking library timeit.

I think this is narrowing down the location of the problem. Does this script timeit_candidate_fds.sh demonstrate the same difference in behaviour when you run it on your changes?

Yes, your script does show the difference between the two implementations:

On the current master branch:

$ ./timeit_candidate_fds.sh
daemon.daemon.get_maximum_file_descriptors() returns 1048576
Timings for ‘daemon.daemon._get_candidate_file_descriptor_ranges(set())’
...
1 loop, best of 5: 1.35 sec per loop

On my branch:

$ ./timeit_candidate_fds.sh
daemon.daemon.get_maximum_file_descriptors() returns 1048576
Timings for ‘daemon.daemon._get_candidate_file_descriptor_ranges(set())’
...
50000 loops, best of 5: 8.69 usec per loop

I have done some extensive isolation of the _get_candidate_file_descriptor_ranges code, and I found that the FileDescriptorRange type (a collections.namedtuple subclass) is a bottleneck to the inner loop of that function.

The creation of a collection of all potential file descriptors is also being done every time; I think it's acceptable to do that one time on loading the module.

Based on those discoveries, I have made a number of optimisations in merge request 43. @apyrgio can you test those changes and see if the speed is now acceptable?

I've taken measurements both with my benchmark script (that measures the total daemonization time), and your timeit.sh script (which measures the time to construct the list of file descriptor ranges). You can see the comparison between the current state and the two PRs that tackle this problem below.

Time to daemonize:

Current: 1.9 sec
PR #43: 0.75 sec
PR #38: 0.5 sec

Time to construct fd ranges:

Current: 1.4 sec
PR #43: 300 msec
PR #38: 9 usec

So yes, the changes proposed by your PR have a big performance benefit. Before you merge it though, please take a look at my comments below:

1) Creating a set of 1 million items at import time has a non-negligible cost:

$ python -m timeit "set(range(1048576))"
5 loops, best of 5: 62.5 msec per loop

There are programs that may import the daemon library but not necessarily daemonize, e.g., due to a CLI argument. It's better if these programs didn't have to pay this cost up front, so I'd suggest moving the calculation back to the original function. After all, I think that it's called only once, but your comment above confuses me:

The creation of a collection of all potential file descriptors is also being done every time

2) Another reason why this calculation should not take place at import time is that there are programs that may call ulimit first and then daemonize. If they lower their number of file descriptors beforehand, they will crash. Note that in master, the maximum number of file descriptors is retrieved during the daemonization process, so that cannot happen.
3) The timeit.sh script should take into account that the creation of the file descriptor ranges happens at import time. As is, the fake_maxfd parameter is not used and produces wrong results.

Finally, I have to say that I just can't see the benefit of constructing and iterating a list of million items, when you can iterate just a handful of excluded file descriptors. Especially when in recent kernels the theoretical maximum is:

$ cat /proc/sys/fs/file-max 
9223372036854775807

Still, it's your call and the performance improvement of your PR is very helpful, so go ahead.

Thanks for examining the effect of merge request 43.

There are programs that may import the daemon library but not necessarily daemonize, e.g., due to a CLI argument. It's better if these programs didn't have to pay this cost up front, so I'd suggest moving the calculation back to the original function.

That's an excellent point, I agree. So I'll revert that part of the change and make a separate merge request for changing the initialisation of that collection.

As for the changes to migrate from FileDescriptorRange to a built-in tuple, that appears to be unobjectionable and provides significant speed improvement. I will merge just that change without mixing other changes in. (Update: done now as of commit 495aa9f17f6ee038.)

I have to say that I just can't see the benefit of constructing and iterating a list of million items, when you can iterate just a handful of excluded file descriptors.

This library is intended to conservatively implement canonical daemonisation procedure; that includes “close all open files”.

It is a valid concern that the Unix daemonisation procedure was standardised long ago, before we had kernels with such very large ceilings on potential open files.

Ideally, we would just ask the OS “what are all the file descriptors actually open now for this process?” and close only those ones. Unfortunately there appears to be no POSIX-compliant way to query the OS for that information. (Please show me where it's documented, if I am wrong about that!)

One possible way is to query the filesystem at /proc/$PID/fd/ and iterate all the entries there. I have not tried it, but that seems like it would be quite slow.

Even slower (I guess) would be to use the lsof utility program, which has the benefit of being cross-platform, but might not be installed and so we should not depend on it.

Another way that would be very nice is to ask “what is the highest file descriptor currently open?” of the kernel. https://stackoverflow.com/questions/899038/getting-the-highest-allocated-file-descriptor But, according to the accepted answer to that question, there's no portable way to do it:

Unfortunately, there is no reliable, portable alternative that does not involve iterating over every possible non-negative int file descriptor.

The most appealing prospect there is the F_MAXFD command to the fcntl system call. However, Linux does not have that; it's BSD-specific.

Python 3.4 and later tracks whether a file descriptor should be “inheritable”:

On UNIX, non-inheritable file descriptors are closed in child processes at the execution of a new program, other file descriptors are inherited.

I wonder whether we can make use of that facility for daemon.close_all_open_files.

This would mean support only for Python 3.4 and later. On the plus side, I have decided we will soon drop support for all earlier Python versions (see issue #44), so it's worth looking at the Python 3 mechanisms for closing open file descriptors.

Ideally, we would just ask the OS "what are all the file descriptors actually open now for this process?" and close only those ones. Unfortunately there appears to be no POSIX-compliant way to query the OS for that information. (Please show me where it's documented, if I am wrong about that!)

I totally agree with the above. Getting a list with just the opened file descriptors of a process would make things much faster. While, indeed, there is no POSIX way to achieve this, I'd like to zoom into the /proc/self/fd approach, as it's supported (in some form) by all OSes, except Windows.

The following measurements compare the current python-daemon version vs. a modified version that consults/proc/self/fd (see snippet at the end). We use a 1048576 file limit and we also measure what happens in a process both with no files open and with 1000 files open:

Time to daemonize:

Current: 750 msec
With /proc/self/fd (no files open): 13 msec
With /proc/self/fd (1000 files open): 15 msec

Time to construct fd ranges:

Current: 300 msec
With /proc/self/fd (no files open): 13 usec
With /proc/self/fd (1000 files open): 770 usec

The takeaway from these numbers is that checking /proc/self/fd is actually very fast, even for processes that have opened 1000 files.

The caveat of course is that not all OSes support this the same way, and Windows specifically do not have any support. I believe however that a try-fallback approach could work for every case. Roughly, we could do the following:

try:
    # list /proc/<pid>/fd, which is supported by Linux, Cygwin and NetBSD
except Exception:
    try:
        # list /dev/fd, which is supported by MacOS* and FreeBSD
    except Exception:
        # fallback to the current way of doing things, which works in every platform

Are you ok with this approach? Can I send you a PR to check it out?

A simple way to consult /proc/self/fd:

-    maxfd = get_maximum_file_descriptors()
-    candidates = set(range(0, maxfd)).difference(exclude)
-    return candidates
+
+    if sys.platform.startswith("linux"):
+        candidates = [int(fd) for fd in os.listdir("/proc/self/fd")]
+    else:
+        maxfd = get_maximum_file_descriptors()
+        candidates = range(0, maxfd)
+    return set(candidates).difference(exclude)

Ping for the previous comment.

The gist is that we can make the daemonization procedure take a few milliseconds in most platforms, and fallback to the slow way of doing things for other platforms. Are you OK with this approach?

@apyrgio:

The gist is that we can make the daemonization procedure take a few milliseconds in most platforms, and fallback to the slow way of doing things for other platforms. Are you OK with this approach?

Not yet, though I'm open to exploring it.

In the meantime, I will release the already-merged improvements discussed earlier.

In my case the daemonization takes about 70 seconds on a Raspberry Pi Zero W, which is pretty much unusable. For now I monkey-patched _get_candidate_file_descriptors to use /proc/self/fd and it's down to 12 milliseconds. My code for reference:

def monkey_patch_daemon(exclude):
    return {int(name) for name in os.listdir('/proc/self/fd') if name.isdigit()}.difference(exclude)
import daemon.daemon
daemon.daemon._get_candidate_file_descriptors = monkey_patch_daemon

The gist is that we can make the daemonization procedure take a few milliseconds in most platforms, and fallback to the slow way of doing things for other platforms. Are you OK with this approach?

I have created issue #60 to track this request.

In the meantime, I will release the already-merged improvements discussed earlier.

I have merged further enhancements to main (commit aac98d4f), that close this issue.

These improvements are now part of ‘python-daemon’ version 2.3.1, released today.