entireio / entireio/cli

Potential improvements to the repo benchmarks

Open
#753 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Go
Stars
5.1k
Forks
475
Avg merge
1d 11h
Merged PRs (30d)
178

Description

Problem or use case

This is not a feature request for the CLI, it's a suggestion for improving the current (already great) benchmarking suite (tried to start a discussion for it, but couldn't https://github.com/entireio/cli/issues/752).

I tried using existing benchmarks to see if there are low hanging fruits to improve CPU and memory allocation, but the profiles generated using pprof are dominated by test setup code due to in memory go-git operations, preventing from seeing any application code in the profiles.

You can see below pprof memory profile for benchmarks of the cli package
Image

Most of the allocations are test related work.

Desired behavior
To identify bottlenecks it would be great if the benchmarks CPU and Memory profiles would consist of mostly application code and not test code.
Proposed solution

The proposed solution would be to reduce usage of go-git when setting up tests, and offload that work to the git CLI. Please note the purpose here is not to make the tests run faster (they won't) or consume less memory (they won't), but to offload work outside the main test Go process so it's easier to identity bottlenecks in application code.
There are other potential ways to deal with test setup code impacting profiling of application code, but this seems to be the most aligned with the current approach in the codebase (I've seen other places where go-git usage was replaced by git CLI invocation calls).

See below memory profile with relevant changes

Image

Where it's easier to identify allocations for application code.
This fix will help with CPU profiles as well

Alternatives or workarounds

Haven't looked into it but maybe it would be possible to pre-create variations of NewBenchRepo then use those instead of recreating them each time.
Not sure it's worth the effort, as the goal is not to make the benchmarks faster/more efficient (at least for now), but to be able to distinguish between test code to application code when using them for profiling.

Another option is to separate the concerns, and create new dedicated set of tests only for profiling.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the cli package benchmarks and their NewBenchRepo setup, then read the existing go-git usage and the approach referenced in pull request 553. Run the benchmarks with pprof before and after the setup change; done means CPU and memory profiles emphasize application allocations rather than test setup work.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, go
Domain
performance, testing-qa
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.