Skip to content

Commit

Permalink
Secure against slow ahead-behind-calculations
Browse files Browse the repository at this point in the history
The commits introducing `%(ahead-behind)` indicated that its calculation
would be very fast.  That seems generally true if the "commit-graph" is
up-to-date.

Introduce a global "slow_repo" marker in the store which can maybe used
for the graph views as well.  Now probe the speed of `for-each-ref`
and fallback it is too slow.

Also run "commit-graph" and check again.  Only then set "slow_repo".
  • Loading branch information
kaste committed Apr 6, 2024
1 parent c7f6a00 commit 09d0a27
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 34 deletions.
106 changes: 72 additions & 34 deletions core/git_mixins/branches.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import re

from GitSavvy.core import store
from GitSavvy.core.git_command import mixin_base
from GitSavvy.core.git_command import mixin_base, NOT_SET
from GitSavvy.core.fns import filter_
from GitSavvy.core.exceptions import GitSavvyError
from GitSavvy.core.utils import yes_no_switch
from GitSavvy.core.utils import hprint, measure_runtime, yes_no_switch
from GitSavvy.core.runtime import run_on_new_thread

from typing import Dict, List, NamedTuple, Optional, Sequence

Expand Down Expand Up @@ -96,12 +97,11 @@ def get_branches(self, *, refs=["refs/heads", "refs/remotes"], merged=None):
"""
Return a list of local and/or remote branches.
"""
supports_ahead_behind = self.git_version >= FOR_EACH_REF_SUPPORTS_AHEAD_BEHIND

def getter() -> str:
nonlocal supports_ahead_behind
def get_branches__(probe_speed: bool, supports_ahead_behind: bool) -> List[Branch]:
WAIT_TIME = 200 # [ms]
try:
return self.git_throwing_silently(
stdout: str = self.git_throwing_silently(
"for-each-ref",
"--format={}".format(
"%00".join((
Expand All @@ -122,37 +122,75 @@ def getter() -> str:
# If `supports_ahead_behind` we don't use the `--[no-]merged` argument
# and instead filter here in Python land.
yes_no_switch("--merged", merged) if not supports_ahead_behind else None,
timeout=WAIT_TIME / 1000 if probe_speed else NOT_SET
)
except GitSavvyError as e:
if probe_speed and "timed out after" in e.stderr:

def run_commit_graph_write():
hprint(
f"`git for-each-ref` took more than {WAIT_TIME}ms which is slow "
"for our purpose. We now run `git commit-graph write` to see if "
"it gets better."
)
try:
self.git_throwing_silently("commit-graph", "write")
except GitSavvyError as err:
hprint(f"`git commit-graph write` raised: {err}")
store.update_state(self.repo_path, {"slow_repo": True})
return

with measure_runtime() as ms:
get_branches__(False, supports_ahead_behind)
elapsed = ms.get()
ok = elapsed < WAIT_TIME
hprint(
f"After `git commit-graph write` the `git for-each-ref` call "
f"{'' if ok else 'still '}takes {elapsed}ms"
)
if not ok:
hprint("Disabling sections in the branches dashboard.")

store.update_state(self.repo_path, {"slow_repo": True if not ok else False})

run_on_new_thread(run_commit_graph_write)
return get_branches__(False, False)

if "fatal: failed to find 'HEAD'" in e.stderr and supports_ahead_behind:
supports_ahead_behind = False
return getter()
else:
e.show_error_panel()
raise

stdout = getter()
branches = [
branch
for branch in (
self._parse_branch_line(line)
for line in filter_(stdout.splitlines())
)
if branch.name != "HEAD"
]
if supports_ahead_behind:
# Cache git's full output but return a filtered result if requested.
self._cache_branches(branches, refs)
if merged is True:
branches = [b for b in branches if b.distance_to_head.ahead == 0] # type: ignore[union-attr]
elif merged is False:
branches = [b for b in branches if b.distance_to_head.ahead > 0] # type: ignore[union-attr]

elif merged is None:
# For older git versions cache git's output only if it was not filtered by `merged`.
self._cache_branches(branches, refs)

return branches
return get_branches__(False, False)

e.show_error_panel()
raise

branches = [
branch
for branch in (
self._parse_branch_line(line)
for line in filter_(stdout.splitlines())
)
if branch.name != "HEAD"
]
if supports_ahead_behind:
# Cache git's full output but return a filtered result if requested.
self._cache_branches(branches, refs)
if merged is True:
branches = [b for b in branches if b.distance_to_head.ahead == 0] # type: ignore[union-attr]
elif merged is False:
branches = [b for b in branches if b.distance_to_head.ahead > 0] # type: ignore[union-attr]

elif merged is None:
# For older git versions cache git's output only if it was not filtered by `merged`.
self._cache_branches(branches, refs)

return branches

slow_repo = store.current_state(self.repo_path).get("slow_repo", None)
supports_ahead_behind = (
self.git_version >= FOR_EACH_REF_SUPPORTS_AHEAD_BEHIND
and slow_repo is not False
)
probe_speed = slow_repo is None and supports_ahead_behind
return get_branches__(probe_speed, supports_ahead_behind)

def _cache_branches(self, branches, refs):
# type: (List[Branch], Sequence[str]) -> None
Expand Down
1 change: 1 addition & 0 deletions core/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class RepoStore(TypedDict, total=False):
last_reset_mode_used: Optional[str]
short_hash_length: int
skipped_files: List[str]
slow_repo: bool
stashes: List[Stash]
recent_commits: List[Commit]
descriptions: Dict[str, str]
Expand Down

0 comments on commit 09d0a27

Please sign in to comment.