From e27bc15bbd7955ec87f2ca8e8d19eae8cb9120e5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Apr 2025 16:29:36 +0200 Subject: [PATCH] Store Commit.Hash by pointer (kept in a pool of hashes) This in itself is not an improvement, because hashes are unique (they are shared between real commits and rebase todos, but there are so few of those that it doesn't matter). However, it becomes an improvement once we also store parent hashes in the same pool; but the real motivation for this change is to also reuse the hash pointers in Pipe objects later in the branch. This will be a big win because in a merge-heavy git repo there are many more Pipe instances than commits. --- pkg/commands/git_commands/commit_loader.go | 53 ++++++++++--------- .../git_commands/commit_loader_test.go | 21 +++++--- pkg/commands/git_commands/rebase_test.go | 4 +- .../git_commands/reflog_commit_loader.go | 9 ++-- .../git_commands/reflog_commit_loader_test.go | 12 +++-- pkg/commands/models/commit.go | 8 +-- pkg/gui/controllers/helpers/refresh_helper.go | 8 +-- .../controllers/helpers/sub_commits_helper.go | 1 + .../controllers/local_commits_controller.go | 2 +- pkg/gui/gui.go | 1 + pkg/gui/presentation/commits_test.go | 4 +- pkg/gui/presentation/files_test.go | 4 +- pkg/gui/presentation/graph/graph_test.go | 50 +++++++++-------- pkg/gui/types/common.go | 2 + pkg/utils/string_pool.go | 14 +++++ 15 files changed, 119 insertions(+), 74 deletions(-) create mode 100644 pkg/utils/string_pool.go diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 89589d78f..3878dd95a 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -66,6 +66,7 @@ type GetCommitsOptions struct { // If non-empty, show divergence from this ref (left-right log) RefToShowDivergenceFrom string MainBranches *MainBranches + HashPool *utils.StringPool } // GetCommits obtains the commits of the current branch @@ -74,7 +75,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, if opts.IncludeRebaseCommits && opts.FilterPath == "" { var err error - commits, err = self.MergeRebasingCommits(commits) + commits, err = self.MergeRebasingCommits(opts.HashPool, commits) if err != nil { return nil, err } @@ -89,7 +90,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, defer wg.Done() logErr = self.getLogCmd(opts).RunAndProcessLines(func(line string) (bool, error) { - commit := self.extractCommitFromLine(line, opts.RefToShowDivergenceFrom != "") + commit := self.extractCommitFromLine(opts.HashPool, line, opts.RefToShowDivergenceFrom != "") commits = append(commits, commit) return false, nil }) @@ -159,7 +160,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, return commits, nil } -func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*models.Commit, error) { +func (self *CommitLoader) MergeRebasingCommits(hashPool *utils.StringPool, commits []*models.Commit) ([]*models.Commit, error) { // chances are we have as many commits as last time so we'll set the capacity to be the old length result := make([]*models.Commit, 0, len(commits)) for i, commit := range commits { @@ -172,7 +173,7 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod workingTreeState := self.getWorkingTreeState() addConflictedRebasingCommit := true if workingTreeState.CherryPicking || workingTreeState.Reverting { - sequencerCommits, err := self.getHydratedSequencerCommits(workingTreeState) + sequencerCommits, err := self.getHydratedSequencerCommits(hashPool, workingTreeState) if err != nil { return nil, err } @@ -181,7 +182,7 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod } if workingTreeState.Rebasing { - rebasingCommits, err := self.getHydratedRebasingCommits(addConflictedRebasingCommit) + rebasingCommits, err := self.getHydratedRebasingCommits(hashPool, addConflictedRebasingCommit) if err != nil { return nil, err } @@ -196,7 +197,7 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod // then puts them into a commit object // example input: // 8ad01fe32fcc20f07bc6693f87aa4977c327f1e1|10 hours ago|Jesse Duffield| (HEAD -> master, tag: v0.15.2)|refresh commits when adding a tag -func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool) *models.Commit { +func (self *CommitLoader) extractCommitFromLine(hashPool *utils.StringPool, line string, showDivergence bool) *models.Commit { split := strings.SplitN(line, "\x00", 8) hash := split[0] @@ -234,7 +235,7 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool parents = strings.Split(parentHashes, " ") } - return models.NewCommit(models.NewCommitOpts{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: hash, Name: message, Tags: tags, @@ -247,13 +248,13 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool }) } -func (self *CommitLoader) getHydratedRebasingCommits(addConflictingCommit bool) ([]*models.Commit, error) { +func (self *CommitLoader) getHydratedRebasingCommits(hashPool *utils.StringPool, addConflictingCommit bool) ([]*models.Commit, error) { todoFileHasShortHashes := self.version.IsOlderThan(2, 25, 2) - return self.getHydratedTodoCommits(self.getRebasingCommits(addConflictingCommit), todoFileHasShortHashes) + return self.getHydratedTodoCommits(hashPool, self.getRebasingCommits(hashPool, addConflictingCommit), todoFileHasShortHashes) } -func (self *CommitLoader) getHydratedSequencerCommits(workingTreeState models.WorkingTreeState) ([]*models.Commit, error) { - commits := self.getSequencerCommits() +func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool, workingTreeState models.WorkingTreeState) ([]*models.Commit, error) { + commits := self.getSequencerCommits(hashPool) if len(commits) > 0 { // If we have any commits in .git/sequencer/todo, then the last one of // those is the conflicting one. @@ -262,16 +263,16 @@ func (self *CommitLoader) getHydratedSequencerCommits(workingTreeState models.Wo // For single-commit cherry-picks and reverts, git apparently doesn't // use the sequencer; in that case, CHERRY_PICK_HEAD or REVERT_HEAD is // our conflicting commit, so synthesize it here. - conflicedCommit := self.getConflictedSequencerCommit(workingTreeState) + conflicedCommit := self.getConflictedSequencerCommit(hashPool, workingTreeState) if conflicedCommit != nil { commits = append(commits, conflicedCommit) } } - return self.getHydratedTodoCommits(commits, true) + return self.getHydratedTodoCommits(hashPool, commits, true) } -func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, todoFileHasShortHashes bool) ([]*models.Commit, error) { +func (self *CommitLoader) getHydratedTodoCommits(hashPool *utils.StringPool, todoCommits []*models.Commit, todoFileHasShortHashes bool) ([]*models.Commit, error) { if len(todoCommits) == 0 { return nil, nil } @@ -292,7 +293,7 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t fullCommits := map[string]*models.Commit{} err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { - commit := self.extractCommitFromLine(line, false) + commit := self.extractCommitFromLine(hashPool, line, false) fullCommits[commit.Hash()] = commit return false, nil }) @@ -331,7 +332,7 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t // git-rebase-todo example: // pick ac446ae94ee560bdb8d1d057278657b251aaef17 ac446ae // pick afb893148791a2fbd8091aeb81deba4930c73031 afb8931 -func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*models.Commit { +func (self *CommitLoader) getRebasingCommits(hashPool *utils.StringPool, addConflictingCommit bool) []*models.Commit { bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo")) if err != nil { self.Log.Error(fmt.Sprintf("error occurred reading git-rebase-todo: %s", err.Error())) @@ -350,7 +351,7 @@ func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*model // See if the current commit couldn't be applied because it conflicted; if // so, add a fake entry for it if addConflictingCommit { - if conflictedCommit := self.getConflictedCommit(todos); conflictedCommit != nil { + if conflictedCommit := self.getConflictedCommit(hashPool, todos); conflictedCommit != nil { commits = append(commits, conflictedCommit) } } @@ -364,7 +365,7 @@ func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*model // Command does not have a commit associated, skip continue } - commits = utils.Prepend(commits, models.NewCommit(models.NewCommitOpts{ + commits = utils.Prepend(commits, models.NewCommit(hashPool, models.NewCommitOpts{ Hash: t.Commit, Name: t.Msg, Status: models.StatusRebasing, @@ -375,7 +376,7 @@ func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*model return commits } -func (self *CommitLoader) getConflictedCommit(todos []todo.Todo) *models.Commit { +func (self *CommitLoader) getConflictedCommit(hashPool *utils.StringPool, todos []todo.Todo) *models.Commit { bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/done")) if err != nil { self.Log.Error(fmt.Sprintf("error occurred reading rebase-merge/done: %s", err.Error())) @@ -391,10 +392,10 @@ func (self *CommitLoader) getConflictedCommit(todos []todo.Todo) *models.Commit amendFileExists, _ := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/amend")) messageFileExists, _ := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/message")) - return self.getConflictedCommitImpl(todos, doneTodos, amendFileExists, messageFileExists) + return self.getConflictedCommitImpl(hashPool, todos, doneTodos, amendFileExists, messageFileExists) } -func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos []todo.Todo, amendFileExists bool, messageFileExists bool) *models.Commit { +func (self *CommitLoader) getConflictedCommitImpl(hashPool *utils.StringPool, todos []todo.Todo, doneTodos []todo.Todo, amendFileExists bool, messageFileExists bool) *models.Commit { // Should never be possible, but just to be safe: if len(doneTodos) == 0 { self.Log.Error("no done entries in rebase-merge/done file") @@ -465,14 +466,14 @@ func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos [ // Any other todo that has a commit associated with it must have failed with // a conflict, otherwise we wouldn't have stopped the rebase: - return models.NewCommit(models.NewCommitOpts{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: lastTodo.Commit, Action: lastTodo.Command, Status: models.StatusConflicted, }) } -func (self *CommitLoader) getSequencerCommits() []*models.Commit { +func (self *CommitLoader) getSequencerCommits(hashPool *utils.StringPool) []*models.Commit { bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer/todo")) if err != nil { self.Log.Error(fmt.Sprintf("error occurred reading sequencer/todo: %s", err.Error())) @@ -493,7 +494,7 @@ func (self *CommitLoader) getSequencerCommits() []*models.Commit { // Command does not have a commit associated, skip continue } - commits = utils.Prepend(commits, models.NewCommit(models.NewCommitOpts{ + commits = utils.Prepend(commits, models.NewCommit(hashPool, models.NewCommitOpts{ Hash: t.Commit, Name: t.Msg, Status: models.StatusCherryPickingOrReverting, @@ -504,7 +505,7 @@ func (self *CommitLoader) getSequencerCommits() []*models.Commit { return commits } -func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.WorkingTreeState) *models.Commit { +func (self *CommitLoader) getConflictedSequencerCommit(hashPool *utils.StringPool, workingTreeState models.WorkingTreeState) *models.Commit { var shaFile string var action todo.TodoCommand if workingTreeState.CherryPicking { @@ -526,7 +527,7 @@ func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.W if len(lines) == 0 { return nil } - return models.NewCommit(models.NewCommitOpts{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: lines[0], Status: models.StatusConflicted, Action: action, diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 67fc0429c..8658e25c4 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -314,13 +314,16 @@ func TestGetCommits(t *testing.T) { }, } + hashPool := &utils.StringPool{} + common.UserConfig().Git.MainBranches = scenario.mainBranches opts := scenario.opts opts.MainBranches = NewMainBranches(common, cmd) + opts.HashPool = hashPool commits, err := builder.GetCommits(opts) expectedCommits := lo.Map(scenario.expectedCommitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) assert.Equal(t, expectedCommits, commits) assert.Equal(t, scenario.expectedError, err) @@ -330,6 +333,8 @@ func TestGetCommits(t *testing.T) { } func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { + hashPool := &utils.StringPool{} + scenarios := []struct { testName string todos []todo.Todo @@ -359,7 +364,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, }, amendFileExists: false, - expectedResult: models.NewCommit(models.NewCommitOpts{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Pick, Status: models.StatusConflicted, @@ -460,7 +465,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, }, amendFileExists: false, - expectedResult: models.NewCommit(models.NewCommitOpts{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Pick, Status: models.StatusConflicted, @@ -489,7 +494,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, amendFileExists: false, messageFileExists: true, - expectedResult: models.NewCommit(models.NewCommitOpts{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Edit, Status: models.StatusConflicted, @@ -526,7 +531,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, } - hash := builder.getConflictedCommitImpl(scenario.todos, scenario.doneTodos, scenario.amendFileExists, scenario.messageFileExists) + hash := builder.getConflictedCommitImpl(hashPool, scenario.todos, scenario.doneTodos, scenario.amendFileExists, scenario.messageFileExists) assert.Equal(t, scenario.expectedResult, hash) }) } @@ -573,11 +578,13 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { for _, scenario := range scenarios { t.Run(scenario.testName, func(t *testing.T) { + hashPool := &utils.StringPool{} + commits := lo.Map(scenario.commitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) setCommitMergedStatuses(scenario.ancestor, commits) expectedCommits := lo.Map(scenario.expectedCommitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) assert.Equal(t, expectedCommits, commits) }) } diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go index d60295de2..feb523973 100644 --- a/pkg/commands/git_commands/rebase_test.go +++ b/pkg/commands/git_commands/rebase_test.go @@ -10,6 +10,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/git_config" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" "github.com/stretchr/testify/assert" ) @@ -158,8 +159,9 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses), }) + hashPool := &utils.StringPool{} commits := lo.Map(s.commitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) s.test(instance.DiscardOldFileChanges(commits, s.commitIndex, s.fileName)) s.runner.CheckForMissingCalls() diff --git a/pkg/commands/git_commands/reflog_commit_loader.go b/pkg/commands/git_commands/reflog_commit_loader.go index 7ddefd94e..10453a1e6 100644 --- a/pkg/commands/git_commands/reflog_commit_loader.go +++ b/pkg/commands/git_commands/reflog_commit_loader.go @@ -7,6 +7,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" + "github.com/jesseduffield/lazygit/pkg/utils" ) type ReflogCommitLoader struct { @@ -23,7 +24,7 @@ func NewReflogCommitLoader(common *common.Common, cmd oscommands.ICmdObjBuilder) // GetReflogCommits only returns the new reflog commits since the given lastReflogCommit // if none is passed (i.e. it's value is nil) then we get all the reflog commits -func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit, filterPath string, filterAuthor string) ([]*models.Commit, bool, error) { +func (self *ReflogCommitLoader) GetReflogCommits(hashPool *utils.StringPool, lastReflogCommit *models.Commit, filterPath string, filterAuthor string) ([]*models.Commit, bool, error) { commits := make([]*models.Commit, 0) cmdArgs := NewGitCmd("log"). @@ -39,7 +40,7 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit onlyObtainedNewReflogCommits := false err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { - commit, ok := self.parseLine(line) + commit, ok := self.parseLine(hashPool, line) if !ok { return false, nil } @@ -68,7 +69,7 @@ func (self *ReflogCommitLoader) sameReflogCommit(a *models.Commit, b *models.Com return a.Hash() == b.Hash() && a.UnixTimestamp == b.UnixTimestamp && a.Name == b.Name } -func (self *ReflogCommitLoader) parseLine(line string) (*models.Commit, bool) { +func (self *ReflogCommitLoader) parseLine(hashPool *utils.StringPool, line string) (*models.Commit, bool) { fields := strings.SplitN(line, "\x00", 4) if len(fields) <= 3 { return nil, false @@ -82,7 +83,7 @@ func (self *ReflogCommitLoader) parseLine(line string) (*models.Commit, bool) { parents = strings.Split(parentHashes, " ") } - return models.NewCommit(models.NewCommitOpts{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: fields[0], Name: fields[2], UnixTimestamp: int64(unixTimestamp), diff --git a/pkg/commands/git_commands/reflog_commit_loader_test.go b/pkg/commands/git_commands/reflog_commit_loader_test.go index 2448a1d23..9aa8536f5 100644 --- a/pkg/commands/git_commands/reflog_commit_loader_test.go +++ b/pkg/commands/git_commands/reflog_commit_loader_test.go @@ -32,6 +32,8 @@ func TestGetReflogCommits(t *testing.T) { expectedError error } + hashPool := &utils.StringPool{} + scenarios := []scenario{ { testName: "no reflog entries", @@ -94,7 +96,7 @@ func TestGetReflogCommits(t *testing.T) { runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p"}, reflogOutput, nil), - lastReflogCommit: models.NewCommit(models.NewCommitOpts{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, @@ -118,7 +120,7 @@ func TestGetReflogCommits(t *testing.T) { runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p", "--follow", "--", "path"}, reflogOutput, nil), - lastReflogCommit: models.NewCommit(models.NewCommitOpts{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, @@ -143,7 +145,7 @@ func TestGetReflogCommits(t *testing.T) { runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p", "--author=John Doe "}, reflogOutput, nil), - lastReflogCommit: models.NewCommit(models.NewCommitOpts{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, @@ -183,14 +185,14 @@ func TestGetReflogCommits(t *testing.T) { cmd: oscommands.NewDummyCmdObjBuilder(scenario.runner), } - commits, onlyObtainednew, err := builder.GetReflogCommits(scenario.lastReflogCommit, scenario.filterPath, scenario.filterAuthor) + commits, onlyObtainednew, err := builder.GetReflogCommits(hashPool, scenario.lastReflogCommit, scenario.filterPath, scenario.filterAuthor) assert.Equal(t, scenario.expectedOnlyObtainedNew, onlyObtainednew) assert.Equal(t, scenario.expectedError, err) t.Logf("actual commits: \n%s", litter.Sdump(commits)) var expectedCommits []*models.Commit if scenario.expectedCommitOpts != nil { expectedCommits = lo.Map(scenario.expectedCommitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) } assert.Equal(t, expectedCommits, commits) diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index 7078ab51e..bde7092db 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -42,7 +42,7 @@ const ( // Commit : A git commit type Commit struct { - hash string + hash *string Name string Status CommitStatus Action todo.TodoCommand @@ -71,9 +71,9 @@ type NewCommitOpts struct { Parents []string } -func NewCommit(opts NewCommitOpts) *Commit { +func NewCommit(hashPool *utils.StringPool, opts NewCommitOpts) *Commit { return &Commit{ - hash: opts.Hash, + hash: hashPool.Add(opts.Hash), Name: opts.Name, Status: opts.Status, Action: opts.Action, @@ -88,7 +88,7 @@ func NewCommit(opts NewCommitOpts) *Commit { } func (c *Commit) Hash() string { - return c.hash + return *c.hash } func (c *Commit) ShortHash() string { diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 93ad110f3..64bed0713 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -332,6 +332,7 @@ func (self *RefreshHelper) refreshCommitsWithLimit() error { RefForPushedStatus: checkedOutBranchName, All: self.c.Contexts().LocalCommits.GetShowWholeGitGraph(), MainBranches: self.c.Model().MainBranches, + HashPool: self.c.Model().HashPool, }, ) if err != nil { @@ -360,6 +361,7 @@ func (self *RefreshHelper) refreshSubCommitsWithLimit() error { RefToShowDivergenceFrom: self.c.Contexts().SubCommits.GetRefToShowDivergenceFrom(), RefForPushedStatus: self.c.Contexts().SubCommits.GetRef().FullRefName(), MainBranches: self.c.Model().MainBranches, + HashPool: self.c.Model().HashPool, }, ) if err != nil { @@ -406,7 +408,7 @@ func (self *RefreshHelper) refreshRebaseCommits() error { self.c.Mutexes().LocalCommitsMutex.Lock() defer self.c.Mutexes().LocalCommitsMutex.Unlock() - updatedCommits, err := self.c.Git().Loaders.CommitLoader.MergeRebasingCommits(self.c.Model().Commits) + updatedCommits, err := self.c.Git().Loaders.CommitLoader.MergeRebasingCommits(self.c.Model().HashPool, self.c.Model().Commits) if err != nil { return err } @@ -455,7 +457,7 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele // which allows us to order them correctly. So if we're filtering we'll just // manually load all the reflog commits here var err error - reflogCommits, _, err = self.c.Git().Loaders.ReflogCommitLoader.GetReflogCommits(nil, "", "") + reflogCommits, _, err = self.c.Git().Loaders.ReflogCommitLoader.GetReflogCommits(self.c.Model().HashPool, nil, "", "") if err != nil { self.c.Log.Error(err) } @@ -624,7 +626,7 @@ func (self *RefreshHelper) refreshReflogCommits() error { refresh := func(stateCommits *[]*models.Commit, filterPath string, filterAuthor string) error { commits, onlyObtainedNewReflogCommits, err := self.c.Git().Loaders.ReflogCommitLoader. - GetReflogCommits(lastReflogCommit, filterPath, filterAuthor) + GetReflogCommits(self.c.Model().HashPool, lastReflogCommit, filterPath, filterAuthor) if err != nil { return err } diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index 3e8f88b7d..cffd8e8ed 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/pkg/gui/controllers/helpers/sub_commits_helper.go @@ -45,6 +45,7 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error { RefForPushedStatus: opts.Ref.FullRefName(), RefToShowDivergenceFrom: opts.RefToShowDivergenceFrom, MainBranches: self.c.Model().MainBranches, + HashPool: self.c.Model().HashPool, }, ) if err != nil { diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 86c149740..ad844f90e 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -564,7 +564,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( for _, c := range commitsToEdit[:len(commitsToEdit)-1] { // Merge commits can't be set to "edit", so just skip them if !c.IsMerge() { - todos = append(todos, models.NewCommit(models.NewCommitOpts{Hash: c.Hash(), Action: todo.Pick})) + todos = append(todos, models.NewCommit(self.c.Model().HashPool, models.NewCommitOpts{Hash: c.Hash(), Action: todo.Pick})) } } if len(todos) > 0 { diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 90a2a64d4..54c14c0c5 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -558,6 +558,7 @@ func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context { FilesTrie: patricia.NewTrie(), Authors: map[string]*models.Author{}, MainBranches: git_commands.NewMainBranches(gui.c.Common, gui.os.Cmd), + HashPool: &utils.StringPool{}, }, Modes: &types.Modes{ Filtering: filtering.New(startArgs.FilterPath, ""), diff --git a/pkg/gui/presentation/commits_test.go b/pkg/gui/presentation/commits_test.go index 13fb396e7..5729e819e 100644 --- a/pkg/gui/presentation/commits_test.go +++ b/pkg/gui/presentation/commits_test.go @@ -544,8 +544,10 @@ func TestGetCommitListDisplayStrings(t *testing.T) { for _, s := range scenarios { if !focusing || s.focus { t.Run(s.testName, func(t *testing.T) { + hashPool := &utils.StringPool{} + commits := lo.Map(s.commitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) result := GetCommitListDisplayStrings( common, diff --git a/pkg/gui/presentation/files_test.go b/pkg/gui/presentation/files_test.go index ac97aa2bc..9a16eac08 100644 --- a/pkg/gui/presentation/files_test.go +++ b/pkg/gui/presentation/files_test.go @@ -149,8 +149,10 @@ func TestRenderCommitFileTree(t *testing.T) { for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { + hashPool := &utils.StringPool{} + viewModel := filetree.NewCommitFileTreeViewModel(func() []*models.CommitFile { return s.files }, utils.NewDummyLog(), true) - viewModel.SetRef(&models.Commit{}) + viewModel.SetRef(models.NewCommit(hashPool, models.NewCommitOpts{Hash: "1234"})) viewModel.SetTree() for _, path := range s.collapsedPaths { viewModel.ToggleCollapsed(path) diff --git a/pkg/gui/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index a4e6aabb3..63d8cb049 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -219,9 +219,11 @@ func TestRenderCommitGraph(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + hashPool := &utils.StringPool{} + getStyle := func(c *models.Commit) style.TextStyle { return style.FgDefault } commits := lo.Map(test.commitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) lines := RenderCommitGraph(commits, "blah", getStyle) trimmedExpectedOutput := "" @@ -254,6 +256,8 @@ func TestRenderPipeSet(t *testing.T) { magenta := style.FgMagenta nothing := style.Nothing + hashPool := &utils.StringPool{} + tests := []struct { name string pipes []*Pipe @@ -268,7 +272,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: TERMINATES, style: cyan}, {fromPos: 0, toPos: 0, fromHash: "b", toHash: "c", kind: STARTS, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "◯", expectedStyles: []style.TextStyle{green}, }, @@ -278,7 +282,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "a", toHash: "selected", kind: TERMINATES, style: cyan}, {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "c", kind: STARTS, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "◯", expectedStyles: []style.TextStyle{highlightStyle}, }, @@ -290,7 +294,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "d", kind: STARTS, style: green}, {fromPos: 0, toPos: 1, fromHash: "selected", toHash: "e", kind: STARTS, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "⏣─╮", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, @@ -304,7 +308,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "b", toHash: "d", kind: STARTS, style: green}, {fromPos: 0, toPos: 1, fromHash: "b", toHash: "e", kind: STARTS, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "⏣─│", expectedStyles: []style.TextStyle{ green, green, magenta, @@ -319,7 +323,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 3, toPos: 0, fromHash: "e1", toHash: "a2", kind: TERMINATES, style: green}, {fromPos: 0, toPos: 2, fromHash: "a2", toHash: "c3", kind: STARTS, style: yellow}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣─│─┬─╯", expectedStyles: []style.TextStyle{ yellow, yellow, magenta, yellow, yellow, green, green, @@ -334,7 +338,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 3, toPos: 0, fromHash: "e1", toHash: "selected", kind: TERMINATES, style: green}, {fromPos: 0, toPos: 2, fromHash: "selected", toHash: "c3", kind: STARTS, style: yellow}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣───╮ ╯", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, highlightStyle, highlightStyle, nothing, green, @@ -348,7 +352,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 1, toPos: 0, fromHash: "b1", toHash: "a2", kind: TERMINATES, style: magenta}, {fromPos: 2, toPos: 0, fromHash: "c1", toHash: "a2", kind: TERMINATES, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "◯─┴─╯", expectedStyles: []style.TextStyle{ yellow, magenta, magenta, green, green, @@ -363,7 +367,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 1, toPos: 1, fromHash: "b1", toHash: "b3", kind: CONTINUES, style: magenta}, {fromPos: 2, toPos: 2, fromHash: "c1", toHash: "c3", kind: CONTINUES, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣─│─│─╮", expectedStyles: []style.TextStyle{ yellow, yellow, magenta, yellow, green, yellow, yellow, @@ -378,7 +382,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 1, toPos: 1, fromHash: "b1", toHash: "a2", kind: CONTINUES, style: green}, {fromPos: 2, toPos: 0, fromHash: "c1", toHash: "a2", kind: TERMINATES, style: magenta}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣─│─╯", expectedStyles: []style.TextStyle{ yellow, yellow, green, magenta, magenta, @@ -393,7 +397,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 2, toPos: 2, fromHash: "c1", toHash: "c3", kind: CONTINUES, style: green}, {fromPos: 3, toPos: 0, fromHash: "d1", toHash: "a2", kind: TERMINATES, style: magenta}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣─┬─│─╯", expectedStyles: []style.TextStyle{ yellow, yellow, yellow, magenta, green, magenta, magenta, @@ -405,7 +409,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "◯", expectedStyles: []style.TextStyle{ yellow, @@ -417,7 +421,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, {fromPos: 1, toPos: 1, fromHash: "selected", toHash: "b3", kind: CONTINUES, style: red}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "◯ │", expectedStyles: []style.TextStyle{ highlightStyle, nothing, highlightStyle, @@ -430,7 +434,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 1, toPos: 1, fromHash: "z1", toHash: "z3", kind: CONTINUES, style: green}, {fromPos: 2, toPos: 2, fromHash: "selected", toHash: "b3", kind: CONTINUES, style: red}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "◯ │ │", expectedStyles: []style.TextStyle{ highlightStyle, nothing, green, nothing, highlightStyle, @@ -444,7 +448,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 1, fromHash: "a2", toHash: "b3", kind: STARTS, style: green}, {fromPos: 1, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: yellow}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "⏣─╯", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, @@ -477,6 +481,8 @@ func TestRenderPipeSet(t *testing.T) { } func TestGetNextPipes(t *testing.T) { + hashPool := &utils.StringPool{} + tests := []struct { prevPipes []*Pipe commit *models.Commit @@ -486,7 +492,7 @@ func TestGetNextPipes(t *testing.T) { prevPipes: []*Pipe{ {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: STARTS, style: style.FgDefault}, }, - commit: models.NewCommit(models.NewCommitOpts{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "b", Parents: []string{"c"}, }), @@ -501,7 +507,7 @@ func TestGetNextPipes(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "b", toHash: "c", kind: STARTS, style: style.FgDefault}, {fromPos: 0, toPos: 1, fromHash: "b", toHash: "d", kind: STARTS, style: style.FgDefault}, }, - commit: models.NewCommit(models.NewCommitOpts{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "d", Parents: []string{"e"}, }), @@ -515,7 +521,7 @@ func TestGetNextPipes(t *testing.T) { prevPipes: []*Pipe{ {fromPos: 0, toPos: 0, fromHash: "a", toHash: "root", kind: TERMINATES, style: style.FgDefault}, }, - commit: models.NewCommit(models.NewCommitOpts{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "root", Parents: []string{}, }), @@ -557,8 +563,10 @@ func BenchmarkRenderCommitGraph(b *testing.B) { } func generateCommits(count int) []*models.Commit { + hashPool := &utils.StringPool{} + rnd := rand.New(rand.NewSource(1234)) - pool := []*models.Commit{models.NewCommit(models.NewCommitOpts{Hash: "a", AuthorName: "A"})} + pool := []*models.Commit{models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a", AuthorName: "A"})} commits := make([]*models.Commit, 0, count) authorPool := []string{"A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"} for len(commits) < count { @@ -575,7 +583,7 @@ func generateCommits(count int) []*models.Commit { if reuseParent { newParent = pool[j] } else { - newParent = models.NewCommit(models.NewCommitOpts{ + newParent = models.NewCommit(hashPool, models.NewCommitOpts{ Hash: fmt.Sprintf("%s%d", currentCommit.Hash(), j), AuthorName: authorPool[rnd.Intn(len(authorPool))], }) @@ -584,7 +592,7 @@ func generateCommits(count int) []*models.Commit { parentHashes = append(parentHashes, newParent.Hash()) } - changedCommit := models.NewCommit(models.NewCommitOpts{ + changedCommit := models.NewCommit(hashPool, models.NewCommitOpts{ Hash: currentCommit.Hash(), AuthorName: currentCommit.AuthorName, Parents: parentHashes, diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 863d7c200..061fee125 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -305,6 +305,8 @@ type Model struct { FilesTrie *patricia.Trie Authors map[string]*models.Author + + HashPool *utils.StringPool } // if you add a new mutex here be sure to instantiate it. We're using pointers to diff --git a/pkg/utils/string_pool.go b/pkg/utils/string_pool.go new file mode 100644 index 000000000..98932f4d5 --- /dev/null +++ b/pkg/utils/string_pool.go @@ -0,0 +1,14 @@ +package utils + +import "sync" + +// A simple string pool implementation that can help reduce memory usage for +// cases where the same string is used multiple times. +type StringPool struct { + sync.Map +} + +func (self *StringPool) Add(s string) *string { + poolEntry, _ := self.LoadOrStore(s, &s) + return poolEntry.(*string) +}