Skip to content

Commit

Permalink
Memoize git shell commands
Browse files Browse the repository at this point in the history
We have a shell script that invokes `chef install` and `chef export`
against a setup with ~30 cookbooks. It was annoyingly slow so we dug
into why that was the case. We found that these commands call
`ChefCLI::CookbookProfiler::Git` in a loop which ends up shelling out
the same handful of git commands repeatedly. For example, one run
produced the following duplicate git commands (sorted by dupe count):

```
 74   execve("/usr/bin/git", ["git", "rev-parse", "HEAD"],
 46   execve("/usr/bin/git", ["git", "rev-parse", "HEAD"],
 41   execve("/usr/bin/git", ["git", "rev-parse", "--abbrev-ref", "HEAD"],
 39   execve("/usr/bin/git", ["git", "diff-files", "--quiet"],
 25   execve("/usr/bin/git", ["git", "branch", "-r", "--contains", "<snip>"],
 23   execve("/usr/lib/git-core/git", ["/usr/lib/git-core/git", "status", "--porcelain=2", "-uno"],
 23   execve("/usr/bin/git", ["git", "config", "--get", "branch.HEAD.remote"],
 21   execve("/usr/bin/git", ["git", "diff-files", "--quiet"],
 19   execve("/usr/bin/git", ["git", "rev-parse", "--abbrev-ref", "HEAD"],
 17   execve("/usr/bin/git", ["git", "config", "--get", "branch.<snip>.remote"],
 16   execve("/usr/bin/git", ["git", "config", "--get", "remote.origin.url"],
 14   execve("/usr/bin/git", ["git", "branch", "-r", "--contains", "<snip>"],
 14   execve("/usr/bin/git", ["git", "branch", "-r", "--contains", "<snip>"],
 12   execve("/usr/bin/git", ["git", "config", "--get", "remote.origin.url"],
 11   execve("/usr/bin/git", ["git", "config", "--get", "branch.<snip>.remote"],
  9   execve("/usr/bin/git", ["git", "config", "--get", "branch.HEAD.remote"],
  7   execve("/usr/bin/git", ["git", "branch", "-r", "--contains", "<snip>"],
  5   execve("/usr/lib/git-core/git", ["/usr/lib/git-core/git", "status", "--porcelain=2", "-uno"],
```

Adding a simple memoization brought down our run from ~45s to ~1s.

To be a little safer, we could wrap this is a `git_memo` method to
make it more explicit. I thought I'd collect some initial feedback
on the general idea first.

Signed-off-by: Adam Saponara <as@php.net>
  • Loading branch information
adsr committed May 8, 2023
1 parent 9ecb91a commit 9fcaf64
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
17 changes: 15 additions & 2 deletions lib/chef-cli/cookbook_profiler/git.rb
Expand Up @@ -23,8 +23,14 @@ class Git

include Helpers

@@git_memo = {}

attr_reader :cookbook_path

def self.uncache
@@git_memo = {}
end

def initialize(cookbook_path)
@cookbook_path = cookbook_path
@unborn_branch = nil
Expand Down Expand Up @@ -111,8 +117,15 @@ def git!(subcommand, options = {})
end

def git(subcommand, options = {})
options = { cwd: cookbook_path }.merge(options)
system_command("git #{subcommand}", options)
memo_key = [subcommand, options]
if @@git_memo.has_key?(memo_key)
rv = @@git_memo[memo_key]
else
options = { cwd: cookbook_path }.merge(options)
rv = system_command("git #{subcommand}", options)
@@git_memo[memo_key] = rv
end
rv
end

def detect_current_branch
Expand Down
3 changes: 2 additions & 1 deletion spec/unit/cookbook_profiler/git_spec.rb
Expand Up @@ -25,7 +25,8 @@

include ChefCLI::Helpers

let(:git_profiler) do
let!(:git_profiler) do
ChefCLI::CookbookProfiler::Git.uncache
ChefCLI::CookbookProfiler::Git.new(cookbook_path)
end

Expand Down

0 comments on commit 9fcaf64

Please sign in to comment.