From d70ba6941cf0f005f3f247c70233f51a0cc35969 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 12 Dec 2023 06:52:35 -0500 Subject: [PATCH] Extract all "import gc" to module level The gc module was already imported at module scope in git/repo/base.py, since f1a82e4 (#555). Importing the top-level git module or any submodule of it runs that import statement. Because the gc module is already imported, reimporting it is fast. Imports that there is no specific reason to do locally should be at module scope. Having them local decreased readability, in part because of how black inserts a black line between them and gc.collect() calls they are imported to allow. An alternative to this change would be to remove the preexisting top-level "import gc" (there is also another one in the test suite) and replace it with a local import as well. I am unsure if that would affect performance and, if so, whether the effect would be good or bad, since the small delay of the import might potentially be less desirable to an applicaion if it occurs while the work of the application is already in progress. If a gc.collect() call runs as a consequence of a finally block or __del__ method being called during interpreter shutdown, then in (very) rare cases the variable may have been set to None. But this does not appear to have been the intent behind making the imports local. More importantly, a local import should not be expected to succeed, or the imported module usable, in such a situation. --- git/objects/submodule/base.py | 3 +-- test/performance/test_commit.py | 3 +-- test/performance/test_streams.py | 3 +-- test/test_base.py | 3 +-- test/test_docs.py | 3 +-- test/test_git.py | 3 +-- test/test_quick_doc.py | 4 ++-- test/test_remote.py | 3 +-- test/test_repo.py | 3 +-- test/test_submodule.py | 3 +-- 10 files changed, 11 insertions(+), 20 deletions(-) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index acf16b556..651d9535c 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -1,6 +1,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import gc from io import BytesIO import logging import os @@ -1079,8 +1080,6 @@ def remove( self._clear_cache() wtd = mod.working_tree_dir del mod # Release file-handles (Windows). - import gc - gc.collect() rmtree(str(wtd)) # END delete tree if possible diff --git a/test/performance/test_commit.py b/test/performance/test_commit.py index 9e136a6c1..00d768f0a 100644 --- a/test/performance/test_commit.py +++ b/test/performance/test_commit.py @@ -5,6 +5,7 @@ """Performance tests for commits (iteration, traversal, and serialization).""" +import gc from io import BytesIO from time import time import sys @@ -17,8 +18,6 @@ class TestPerformance(TestBigRepoRW, TestCommitSerialization): def tearDown(self): - import gc - gc.collect() # ref with about 100 commits in its history. diff --git a/test/performance/test_streams.py b/test/performance/test_streams.py index 9ee7cf5e2..ba5cbe415 100644 --- a/test/performance/test_streams.py +++ b/test/performance/test_streams.py @@ -3,6 +3,7 @@ """Performance tests for data streaming.""" +import gc import os import subprocess import sys @@ -92,8 +93,6 @@ def test_large_data_streaming(self, rwrepo): # del db file so git has something to do. ostream = None - import gc - gc.collect() os.remove(db_file) diff --git a/test/test_base.py b/test/test_base.py index a667b194c..cdf82b74d 100644 --- a/test/test_base.py +++ b/test/test_base.py @@ -3,6 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import gc import os import sys import tempfile @@ -20,8 +21,6 @@ class TestBase(_TestBase): def tearDown(self): - import gc - gc.collect() type_tuples = ( diff --git a/test/test_docs.py b/test/test_docs.py index a03a9c71b..48922eba8 100644 --- a/test/test_docs.py +++ b/test/test_docs.py @@ -3,6 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import gc import os import sys @@ -16,8 +17,6 @@ class Tutorials(TestBase): def tearDown(self): - import gc - gc.collect() # ACTUALLY skipped by git.util.rmtree (in local onerror function), from the last call to it via diff --git a/test/test_git.py b/test/test_git.py index 5986a07da..6d0ae82d0 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -3,6 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import gc import inspect import logging import os @@ -34,8 +35,6 @@ def setUpClass(cls): cls.git = Git(cls.rorepo.working_dir) def tearDown(self): - import gc - gc.collect() def _assert_logged_for_popen(self, log_watcher, name, value): diff --git a/test/test_quick_doc.py b/test/test_quick_doc.py index 504dca237..4ef75f4aa 100644 --- a/test/test_quick_doc.py +++ b/test/test_quick_doc.py @@ -1,14 +1,14 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import gc + from test.lib import TestBase from test.lib.helper import with_rw_directory class QuickDoc(TestBase): def tearDown(self): - import gc - gc.collect() @with_rw_directory diff --git a/test/test_remote.py b/test/test_remote.py index 080718a1c..df6034326 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -3,6 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import gc import os import os.path as osp from pathlib import Path @@ -105,8 +106,6 @@ def assert_received_message(self): class TestRemote(TestBase): def tearDown(self): - import gc - gc.collect() def _print_fetchhead(self, repo): diff --git a/test/test_repo.py b/test/test_repo.py index 7560c519f..007b8ecb0 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -3,6 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import gc import glob import io from io import BytesIO @@ -72,8 +73,6 @@ def tearDown(self): if osp.isfile(lfp): raise AssertionError("Previous TC left hanging git-lock file: {}".format(lfp)) - import gc - gc.collect() def test_new_should_raise_on_invalid_repo_location(self): diff --git a/test/test_submodule.py b/test/test_submodule.py index b68ca3fed..852a5ef6f 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -2,6 +2,7 @@ # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ import contextlib +import gc import os import os.path as osp from pathlib import Path @@ -61,8 +62,6 @@ def update(self, op, cur_count, max_count, message=""): class TestSubmodule(TestBase): def tearDown(self): - import gc - gc.collect() k_subm_current = "c15a6e1923a14bc760851913858a3942a4193cdb"