Skip to content

Commit

Permalink
Add a simple LRU cache to common git cmds
Browse files Browse the repository at this point in the history
Simple commands such as ref existence, id lookup,
and distance lookup are pretty common and could be
called with the same args at a higher level such that
its difficult to dedupe between calls. To speed things
up, add a lru_cache decorator so that later calls are
looked up. We don't add this to everything since
more complex functions are unlikely to be called with
the same args multiple times.

These calls are mostly pure functions since revup
generally doesn't change the git database. There are
a few exceptions to that, when it runs "reset" for certain
commands. In these cases, we empty all caches just to be safe.

Topic: cache
Reviewers: brian-k
  • Loading branch information
jerry-skydio committed Apr 13, 2023
1 parent ce3a07d commit 3f03c26
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 10 deletions.
2 changes: 1 addition & 1 deletion revup/amend.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,5 +237,5 @@ async def get_has_unstaged() -> bool:
git_env = {
"GIT_REFLOG_ACTION": "reset --soft (revup amend)",
}
await git_ctx.git("reset", "--soft", new_commit, env=git_env)
await git_ctx.soft_reset(new_commit, git_env)
return 0
3 changes: 2 additions & 1 deletion revup/cherry_pick.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ async def main(args: argparse.Namespace, git_ctx: git.Git) -> int:
branch_to_pick = args.branch[0]
remote_branch_to_pick = git_ctx.ensure_branch_prefix(branch_to_pick)
branch_exists, remote_branch_exists = await asyncio.gather(
git_ctx.commit_exists(branch_to_pick), git_ctx.commit_exists(remote_branch_to_pick)
git_ctx.is_branch_or_commit(branch_to_pick),
git_ctx.is_branch_or_commit(remote_branch_to_pick),
)
if remote_branch_exists and not branch_exists:
logging.warning(
Expand Down
31 changes: 25 additions & 6 deletions revup/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import shutil
import tempfile
from dataclasses import dataclass
from functools import lru_cache
from typing import Any, Dict, List, NamedTuple, Optional, Pattern, Tuple

from revup import shell
Expand Down Expand Up @@ -181,7 +182,7 @@ async def get_editor() -> str:
git_ctx.git_stdout("--version"),
get_email(),
get_editor(),
git_ctx.commit_exists(f"{remote_name}/{main_branch}"),
git_ctx.is_branch_or_commit(f"{remote_name}/{main_branch}"),
)

if git_version:
Expand Down Expand Up @@ -263,6 +264,18 @@ def __init__(
if self.keep_temp:
os.makedirs(self.get_scratch_dir(), exist_ok=True)

def clear_cache(self) -> None:
"""
Clear caches for all functions that are decorated with lru_cache(). This is generally
overdoing it since its usually sufficient to only clear certain entries, however most usages
currently happen at the end of the program so it isn't worth optimizing now.
"""
self.is_branch_or_commit.cache_clear() # pylint: disable=no-member
self.to_commit_hash.cache_clear() # pylint: disable=no-member
self.fork_point.cache_clear() # pylint: disable=no-member
self.distance_to_fork_point.cache_clear() # pylint: disable=no-member
self.have_identical_trees.cache_clear() # pylint: disable=no-member

def get_scratch_dir(self) -> str:
"""
Get the name of the current scratch directory. Any contents will be deleted
Expand Down Expand Up @@ -344,23 +357,21 @@ async def rev_list(
rev_list_args.extend(["--not", exclude])
return await self.git_stdout(*rev_list_args)

@lru_cache(maxsize=None)
async def is_branch_or_commit(self, obj: str) -> bool:
return await self.git_return_code("rev-parse", "--verify", "--quiet", obj) == 0

async def verify_branch_or_commit(self, obj: str) -> None:
if not await self.is_branch_or_commit(obj):
raise RevupUsageException(f"{obj} is not a commit or branch name!")

async def commit_exists(self, obj: str) -> bool:
return (
await self.git_return_code("rev-parse", "--verify", "--quiet", obj + "^{commit}") == 0
)

@lru_cache(maxsize=None)
async def to_commit_hash(self, ref: str) -> GitCommitHash:
return GitCommitHash(
await self.git_stdout("rev-parse", "--verify", "--quiet", ref + "^{commit}")
)

@lru_cache(maxsize=None)
async def fork_point(self, ref: str, baseRef: str) -> GitCommitHash:
"""
Define the fork-point of your branch and a base branch as the commit at
Expand Down Expand Up @@ -390,6 +401,7 @@ async def fork_point(self, ref: str, baseRef: str) -> GitCommitHash:
return GitCommitHash(ref)
return GitCommitHash(f"{commit}~")

@lru_cache(maxsize=None)
async def distance_to_fork_point(self, ref: str, baseRef: str, max_n: int = 0) -> int:
"""
Return number of commits between ref and its fork point with baseRef, up to the given max.
Expand Down Expand Up @@ -417,6 +429,7 @@ async def is_ancestor(self, ref: str, ancestor: str) -> bool:
return True
return await self.distance_to_fork_point(ref, ancestor, 1) == 0

@lru_cache(maxsize=None)
async def have_identical_trees(self, ref1: GitCommitHash, ref2: GitCommitHash) -> bool:
"""
Return whether two commit-ish have the same trees, which indicate that
Expand Down Expand Up @@ -761,3 +774,9 @@ async def make_virtual_diff_target(
)

return await self.commit_tree(new_commit_info)

async def soft_reset(self, new_commit: GitCommitHash, env: Dict) -> None:
await self.git("reset", "--soft", new_commit, env=env)

# TODO: only strictly needs to drop entries for HEAD
self.clear_cache()
4 changes: 2 additions & 2 deletions revup/topic_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ async def fetch_git_refs(self) -> None:

to_fetch = set()
for _, _, _, review in self.all_reviews_iter():
if review.pr_info is not None and not await self.git_ctx.commit_exists(
if review.pr_info is not None and not await self.git_ctx.is_branch_or_commit(
review.pr_info.headRefOid
):
to_fetch.add(review.pr_info.headRefOid)
Expand Down Expand Up @@ -1293,7 +1293,7 @@ async def restack(self, topicless_last: bool) -> GitCommitHash:
git_env = {
"GIT_REFLOG_ACTION": "reset --soft (revup restack)",
}
await self.git_ctx.git("reset", "--soft", new_parent, env=git_env)
await self.git_ctx.soft_reset(new_parent, git_env)
return new_parent

def num_reviews_changed(self) -> int:
Expand Down

0 comments on commit 3f03c26

Please sign in to comment.