From a65465572773d6298523dffdc8d2b07f4221ab2d Mon Sep 17 00:00:00 2001 From: Ben Finney Date: Jul 07 2019 06:09:16 +0000 Subject: [PATCH 1/4] Construct the total range of file descriptors one time, when module loads. --- diff --git a/daemon/daemon.py b/daemon/daemon.py index 0357a00..38be93f 100644 --- a/daemon/daemon.py +++ b/daemon/daemon.py @@ -872,6 +872,10 @@ def get_maximum_file_descriptors(): return result +_total_file_descriptor_range = FileDescriptorRange( + low=0, high=get_maximum_file_descriptors()) + + def _get_candidate_file_descriptors(exclude): """ Get the collection of candidate file descriptors. @@ -888,8 +892,8 @@ def _get_candidate_file_descriptors(exclude): The `maxfd` value is determined from the standard library `resource` module. """ - maxfd = get_maximum_file_descriptors() - candidates = set(range(0, maxfd)).difference(exclude) + total_file_descriptor_set = set(range(*_total_file_descriptor_range)) + candidates = total_file_descriptor_set.difference(exclude) return candidates diff --git a/test/test_daemon.py b/test/test_daemon.py index 2008268..7b36fd5 100644 --- a/test/test_daemon.py +++ b/test/test_daemon.py @@ -1440,17 +1440,17 @@ def fake_get_maximum_file_descriptors(): return fake_default_maxfd -def make_get_maximum_file_descriptors_patch(self, fake_maxfd): - """ Make a `get_maximum_file_descriptors` patch for the `testcase`. +def make_total_file_descriptor_range_patch(testcase, fake_maxfd): + """ Make a `_total_file_descriptor_range` patch for the `testcase`. :param testcase: The `unittest.TestCase` instance to patch. :param fake_maxfd: The fake maximum file descriptor value. :return: The `unittest.mock.patch` object. """ - func_patcher = mock.patch.object( - daemon.daemon, "get_maximum_file_descriptors", - return_value=fake_maxfd) - return func_patcher + attr_patcher = mock.patch.object( + daemon.daemon, "_total_file_descriptor_range", + new=daemon.daemon.FileDescriptorRange(0, fake_maxfd)) + return attr_patcher class _get_candidate_file_descriptors_TestCase(scaffold.TestCaseWithScenarios): @@ -1482,7 +1482,7 @@ class _get_candidate_file_descriptors_TestCase(scaffold.TestCaseWithScenarios): def test_returns_expected_file_descriptors(self): """ Should return the expected set of file descriptors. """ - with make_get_maximum_file_descriptors_patch(self, self.fake_maxfd): + with make_total_file_descriptor_range_patch(self, self.fake_maxfd): result = daemon.daemon._get_candidate_file_descriptors( **self.test_kwargs) self.assertEqual(result, self.expected_result) @@ -1579,7 +1579,7 @@ class _get_candidate_file_descriptor_ranges_TestCase( def test_returns_expected_file_descriptors(self): """ Should return the expected set of file descriptors. """ - with make_get_maximum_file_descriptors_patch(self, self.fake_maxfd): + with make_total_file_descriptor_range_patch(self, self.fake_maxfd): result = daemon.daemon._get_candidate_file_descriptor_ranges( **self.test_kwargs) self.assertEqual(result, self.expected_result) @@ -1633,11 +1633,11 @@ class close_all_open_files_TestCase(scaffold.TestCase): """ Set up test fixtures. """ super(close_all_open_files_TestCase, self).setUp() - get_maximum_file_descriptors_patch = ( - make_get_maximum_file_descriptors_patch( + total_file_descriptor_range_patch = ( + make_total_file_descriptor_range_patch( self, fake_maxfd=self.fake_maxfd)) - get_maximum_file_descriptors_patch.start() - self.addCleanup(get_maximum_file_descriptors_patch.stop) + total_file_descriptor_range_patch.start() + self.addCleanup(total_file_descriptor_range_patch.stop) self.patch_os_closerange() From 0ed70fd5fe3f60553a5276cf78b9fd07413daaf5 Mon Sep 17 00:00:00 2001 From: Ben Finney Date: Oct 04 2019 04:22:00 +0000 Subject: [PATCH 2/4] Construct the total set of file descriptors one time, when module loads. --- diff --git a/daemon/daemon.py b/daemon/daemon.py index 38be93f..97ea34a 100644 --- a/daemon/daemon.py +++ b/daemon/daemon.py @@ -874,6 +874,7 @@ def get_maximum_file_descriptors(): _total_file_descriptor_range = FileDescriptorRange( low=0, high=get_maximum_file_descriptors()) +_total_file_descriptor_set = set(range(*_total_file_descriptor_range)) def _get_candidate_file_descriptors(exclude): @@ -892,8 +893,7 @@ def _get_candidate_file_descriptors(exclude): The `maxfd` value is determined from the standard library `resource` module. """ - total_file_descriptor_set = set(range(*_total_file_descriptor_range)) - candidates = total_file_descriptor_set.difference(exclude) + candidates = _total_file_descriptor_set.difference(exclude) return candidates diff --git a/test/test_daemon.py b/test/test_daemon.py index 7b36fd5..898ce78 100644 --- a/test/test_daemon.py +++ b/test/test_daemon.py @@ -13,6 +13,7 @@ from __future__ import (absolute_import, unicode_literals) import collections +import contextlib import errno import functools import io @@ -1453,6 +1454,20 @@ def make_total_file_descriptor_range_patch(testcase, fake_maxfd): return attr_patcher +def make_total_file_descriptor_set_patch(testcase, fake_maxfd): + """ Make a `_total_file_descriptor_set` patch for the `testcase`. + + :param testcase: The `unittest.TestCase` instance to patch. + :param fake_maxfd: The fake maximum file descriptor value. + :return: The `unittest.mock.patch` object. + """ + attr_patcher = mock.patch.object( + daemon.daemon, "_total_file_descriptor_set", + new=set( + range(*daemon.daemon.FileDescriptorRange(0, fake_maxfd)))) + return attr_patcher + + class _get_candidate_file_descriptors_TestCase(scaffold.TestCaseWithScenarios): """ Test cases for function `_get_candidate_file_descriptors`. """ @@ -1482,7 +1497,11 @@ class _get_candidate_file_descriptors_TestCase(scaffold.TestCaseWithScenarios): def test_returns_expected_file_descriptors(self): """ Should return the expected set of file descriptors. """ - with make_total_file_descriptor_range_patch(self, self.fake_maxfd): + with contextlib.ExitStack() as patch_stack: + patch_stack.enter_context( + make_total_file_descriptor_range_patch(self, self.fake_maxfd)) + patch_stack.enter_context( + make_total_file_descriptor_set_patch(self, self.fake_maxfd)) result = daemon.daemon._get_candidate_file_descriptors( **self.test_kwargs) self.assertEqual(result, self.expected_result) @@ -1579,7 +1598,11 @@ class _get_candidate_file_descriptor_ranges_TestCase( def test_returns_expected_file_descriptors(self): """ Should return the expected set of file descriptors. """ - with make_total_file_descriptor_range_patch(self, self.fake_maxfd): + with contextlib.ExitStack() as patch_stack: + patch_stack.enter_context( + make_total_file_descriptor_range_patch(self, self.fake_maxfd)) + patch_stack.enter_context( + make_total_file_descriptor_set_patch(self, self.fake_maxfd)) result = daemon.daemon._get_candidate_file_descriptor_ranges( **self.test_kwargs) self.assertEqual(result, self.expected_result) @@ -1638,6 +1661,11 @@ class close_all_open_files_TestCase(scaffold.TestCase): self, fake_maxfd=self.fake_maxfd)) total_file_descriptor_range_patch.start() self.addCleanup(total_file_descriptor_range_patch.stop) + total_file_descriptor_set_patch = ( + make_total_file_descriptor_set_patch( + self, fake_maxfd=self.fake_maxfd)) + total_file_descriptor_set_patch.start() + self.addCleanup(total_file_descriptor_set_patch.stop) self.patch_os_closerange() From 3fa37bab7564fcb4e7e6e9b66b005ecffad0eca4 Mon Sep 17 00:00:00 2001 From: Ben Finney Date: Oct 04 2019 04:22:00 +0000 Subject: [PATCH 3/4] Remove unused helper function. --- diff --git a/test/test_daemon.py b/test/test_daemon.py index 898ce78..ceeafdd 100644 --- a/test/test_daemon.py +++ b/test/test_daemon.py @@ -1437,10 +1437,6 @@ class get_maximum_file_descriptors_TestCase(scaffold.TestCase): self.assertEqual(expected_result, result) -def fake_get_maximum_file_descriptors(): - return fake_default_maxfd - - def make_total_file_descriptor_range_patch(testcase, fake_maxfd): """ Make a `_total_file_descriptor_range` patch for the `testcase`. From c11a370c3ba8906f225e7a78c485b61f4ffeedf3 Mon Sep 17 00:00:00 2001 From: Ben Finney Date: Oct 04 2019 06:10:31 +0000 Subject: [PATCH 4/4] Use a builtin `tuple` to represent a range of file descriptors. Compared to the custom `namedtuple`, this speeds the inner loop of `_get_candidate_file_descriptor_ranges` more than 5×. --- diff --git a/daemon/daemon.py b/daemon/daemon.py index 97ea34a..6210e52 100644 --- a/daemon/daemon.py +++ b/daemon/daemon.py @@ -846,9 +846,6 @@ def close_file_descriptor_if_open(fd): raise error -FileDescriptorRange = collections.namedtuple( - 'FileDescriptorRange', ['low', 'high']) - MAXFD = 2048 @@ -872,8 +869,7 @@ def get_maximum_file_descriptors(): return result -_total_file_descriptor_range = FileDescriptorRange( - low=0, high=get_maximum_file_descriptors()) +_total_file_descriptor_range = (0, get_maximum_file_descriptors()) _total_file_descriptor_set = set(range(*_total_file_descriptor_range)) @@ -917,25 +913,24 @@ def _get_candidate_file_descriptor_ranges(exclude): ranges = [] def append_range_if_needed(candidate_range): - if (candidate_range.low < candidate_range.high): + (low, high) = candidate_range + if (low < high): # The range is not empty. ranges.append(candidate_range) this_range = ( - FileDescriptorRange( - low=min(candidates_list), - high=(min(candidates_list) + 1)) - if candidates_list else FileDescriptorRange(low=0, high=0)) + (min(candidates_list), (min(candidates_list) + 1)) + if candidates_list else (0, 0)) for fd in candidates_list[1:]: high = fd + 1 - if this_range.high == fd: + if this_range[1] == fd: # This file descriptor extends the current range. - this_range = this_range._replace(high=high) + this_range = (this_range[0], high) else: # The previous range has ended at a gap. append_range_if_needed(this_range) # This file descriptor begins a new range. - this_range = FileDescriptorRange(low=fd, high=high) + this_range = (fd, high) append_range_if_needed(this_range) return ranges @@ -951,7 +946,7 @@ def _close_file_descriptor_ranges(ranges): `low` and ending before `high` – from each range in `ranges`. """ for range in ranges: - os.closerange(range.low, range.high) + os.closerange(range[0], range[1]) def close_all_open_files(exclude=None): diff --git a/test/test_daemon.py b/test/test_daemon.py index ceeafdd..08d431c 100644 --- a/test/test_daemon.py +++ b/test/test_daemon.py @@ -1446,7 +1446,7 @@ def make_total_file_descriptor_range_patch(testcase, fake_maxfd): """ attr_patcher = mock.patch.object( daemon.daemon, "_total_file_descriptor_range", - new=daemon.daemon.FileDescriptorRange(0, fake_maxfd)) + new=(0, fake_maxfd)) return attr_patcher @@ -1459,8 +1459,7 @@ def make_total_file_descriptor_set_patch(testcase, fake_maxfd): """ attr_patcher = mock.patch.object( daemon.daemon, "_total_file_descriptor_set", - new=set( - range(*daemon.daemon.FileDescriptorRange(0, fake_maxfd)))) + new=set(range(0, fake_maxfd))) return attr_patcher @@ -1612,7 +1611,7 @@ class _close_file_descriptor_ranges_TestCase(scaffold.TestCaseWithScenarios): ('ranges-one', { 'test_kwargs': { 'ranges': [ - daemon.daemon.FileDescriptorRange(0, 10), + (0, 10), ], }, 'expected_os_closerange_calls': [ @@ -1622,9 +1621,9 @@ class _close_file_descriptor_ranges_TestCase(scaffold.TestCaseWithScenarios): ('ranges-three', { 'test_kwargs': { 'ranges': [ - daemon.daemon.FileDescriptorRange(5, 10), - daemon.daemon.FileDescriptorRange(0, 3), - daemon.daemon.FileDescriptorRange(15, 20), + (5, 10), + (0, 3), + (15, 20), ], }, 'expected_os_closerange_calls': [