Skip to content

Commit

Permalink
Extract all "import gc" to module level
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EliahKagan committed Dec 12, 2023
1 parent 68272aa commit d70ba69
Show file tree
Hide file tree
Showing 10 changed files with 11 additions and 20 deletions.
3 changes: 1 addition & 2 deletions git/objects/submodule/base.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions test/performance/test_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

"""Performance tests for commits (iteration, traversal, and serialization)."""

import gc
from io import BytesIO
from time import time
import sys
Expand All @@ -17,8 +18,6 @@

class TestPerformance(TestBigRepoRW, TestCommitSerialization):
def tearDown(self):
import gc

gc.collect()

# ref with about 100 commits in its history.
Expand Down
3 changes: 1 addition & 2 deletions test/performance/test_streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

"""Performance tests for data streaming."""

import gc
import os
import subprocess
import sys
Expand Down Expand Up @@ -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)

Expand Down
3 changes: 1 addition & 2 deletions test/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,8 +21,6 @@

class TestBase(_TestBase):
def tearDown(self):
import gc

gc.collect()

type_tuples = (
Expand Down
3 changes: 1 addition & 2 deletions test/test_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions test/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions test/test_quick_doc.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 1 addition & 2 deletions test/test_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -105,8 +106,6 @@ def assert_received_message(self):

class TestRemote(TestBase):
def tearDown(self):
import gc

gc.collect()

def _print_fetchhead(self, repo):
Expand Down
3 changes: 1 addition & 2 deletions test/test_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
3 changes: 1 addition & 2 deletions test/test_submodule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit d70ba69

Please sign in to comment.