Store Pipe objects by value in slice of Pipes

This saves some memory at the cost of a slight performance increase (I suppose
reallocting the slice when adding new Pipes is slightly more expensive now).

Performance of the BenchmarkRenderCommitGraph benchmark is 130μs before, 175μs
after. I'm guessing this is still acceptable.
This commit is contained in:
Stefan Haller 2025-04-16 20:22:39 +02:00
parent 18e5b0a650
commit 28aa26f30a
3 changed files with 43 additions and 43 deletions

View file

@ -28,7 +28,7 @@ type pipeSetCacheKey struct {
}
var (
pipeSetCache = make(map[pipeSetCacheKey][][]*graph.Pipe)
pipeSetCache = make(map[pipeSetCacheKey][][]graph.Pipe)
mutex deadlock.Mutex
)
@ -245,7 +245,7 @@ func indexOfFirstNonTODOCommit(commits []*models.Commit) int {
return 0
}
func loadPipesets(commits []*models.Commit) [][]*graph.Pipe {
func loadPipesets(commits []*models.Commit) [][]graph.Pipe {
// given that our cache key is a commit hash and a commit count, it's very important that we don't actually try to render pipes
// when dealing with things like filtered commits.
cacheKey := pipeSetCacheKey{

View file

@ -56,20 +56,20 @@ func RenderCommitGraph(commits []*models.Commit, selectedCommitHashPtr *string,
return lines
}
func GetPipeSets(commits []*models.Commit, getStyle func(c *models.Commit) style.TextStyle) [][]*Pipe {
func GetPipeSets(commits []*models.Commit, getStyle func(c *models.Commit) style.TextStyle) [][]Pipe {
if len(commits) == 0 {
return nil
}
pipes := []*Pipe{{fromPos: 0, toPos: 0, fromHash: &StartCommitHash, toHash: commits[0].HashPtr(), kind: STARTS, style: style.FgDefault}}
pipes := []Pipe{{fromPos: 0, toPos: 0, fromHash: &StartCommitHash, toHash: commits[0].HashPtr(), kind: STARTS, style: style.FgDefault}}
return lo.Map(commits, func(commit *models.Commit, _ int) []*Pipe {
return lo.Map(commits, func(commit *models.Commit, _ int) []Pipe {
pipes = getNextPipes(pipes, commit, getStyle)
return pipes
})
}
func RenderAux(pipeSets [][]*Pipe, commits []*models.Commit, selectedCommitHashPtr *string) []string {
func RenderAux(pipeSets [][]Pipe, commits []*models.Commit, selectedCommitHashPtr *string) []string {
maxProcs := runtime.GOMAXPROCS(0)
// splitting up the rendering of the graph into multiple goroutines allows us to render the graph in parallel
@ -106,7 +106,7 @@ func RenderAux(pipeSets [][]*Pipe, commits []*models.Commit, selectedCommitHashP
return lo.Flatten(chunks)
}
func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *models.Commit) style.TextStyle) []*Pipe {
func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *models.Commit) style.TextStyle) []Pipe {
maxPos := 0
for _, pipe := range prevPipes {
if pipe.toPos > maxPos {
@ -116,11 +116,11 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod
// a pipe that terminated in the previous line has no bearing on the current line
// so we'll filter those out
currentPipes := lo.Filter(prevPipes, func(pipe *Pipe, _ int) bool {
currentPipes := lo.Filter(prevPipes, func(pipe Pipe, _ int) bool {
return pipe.kind != TERMINATES
})
newPipes := make([]*Pipe, 0, len(currentPipes)+len(commit.ParentPtrs()))
newPipes := make([]Pipe, 0, len(currentPipes)+len(commit.ParentPtrs()))
// start by assuming that we've got a brand new commit not related to any preceding commit.
// (this only happens when we're doing `git log --all`). These will be tacked onto the far end.
pos := maxPos + 1
@ -143,7 +143,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod
} else {
toHash = commit.ParentPtrs()[0]
}
newPipes = append(newPipes, &Pipe{
newPipes = append(newPipes, Pipe{
fromPos: pos,
toPos: pos,
fromHash: commit.HashPtr(),
@ -195,7 +195,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod
for _, pipe := range currentPipes {
if equalHashes(pipe.toHash, commit.HashPtr()) {
// terminating here
newPipes = append(newPipes, &Pipe{
newPipes = append(newPipes, Pipe{
fromPos: pipe.toPos,
toPos: pos,
fromHash: pipe.fromHash,
@ -207,7 +207,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod
} else if pipe.toPos < pos {
// continuing here
availablePos := getNextAvailablePosForContinuingPipe()
newPipes = append(newPipes, &Pipe{
newPipes = append(newPipes, Pipe{
fromPos: pipe.toPos,
toPos: availablePos,
fromHash: pipe.fromHash,
@ -223,7 +223,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod
for _, parent := range commit.ParentPtrs()[1:] {
availablePos := getNextAvailablePosForNewPipe()
// need to act as if continuing pipes are going to continue on the same line.
newPipes = append(newPipes, &Pipe{
newPipes = append(newPipes, Pipe{
fromPos: pos,
toPos: availablePos,
fromHash: commit.HashPtr(),
@ -247,7 +247,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod
last = i
}
}
newPipes = append(newPipes, &Pipe{
newPipes = append(newPipes, Pipe{
fromPos: pipe.toPos,
toPos: last,
fromHash: pipe.fromHash,
@ -260,7 +260,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod
}
// not efficient but doing it for now: sorting my pipes by toPos, then by kind
slices.SortFunc(newPipes, func(a, b *Pipe) int {
slices.SortFunc(newPipes, func(a, b Pipe) int {
if a.toPos == b.toPos {
return cmp.Compare(a.kind, b.kind)
}
@ -271,7 +271,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod
}
func renderPipeSet(
pipes []*Pipe,
pipes []Pipe,
selectedCommitHashPtr *string,
prevCommit *models.Commit,
) string {
@ -330,19 +330,19 @@ func renderPipeSet(
// so we have our commit pos again, now it's time to build the cells.
// we'll handle the one that's sourced from our selected commit last so that it can override the other cells.
selectedPipes, nonSelectedPipes := utils.Partition(pipes, func(pipe *Pipe) bool {
selectedPipes, nonSelectedPipes := utils.Partition(pipes, func(pipe Pipe) bool {
return highlight && equalHashes(pipe.fromHash, selectedCommitHashPtr)
})
for _, pipe := range nonSelectedPipes {
if pipe.kind == STARTS {
renderPipe(pipe, pipe.style, true)
renderPipe(&pipe, pipe.style, true)
}
}
for _, pipe := range nonSelectedPipes {
if pipe.kind != STARTS && !(pipe.kind == TERMINATES && pipe.fromPos == commitPos && pipe.toPos == commitPos) {
renderPipe(pipe, pipe.style, false)
renderPipe(&pipe, pipe.style, false)
}
}
@ -352,7 +352,7 @@ func renderPipeSet(
}
}
for _, pipe := range selectedPipes {
renderPipe(pipe, highlightStyle, true)
renderPipe(&pipe, highlightStyle, true)
if pipe.toPos == commitPos {
cells[pipe.toPos].setStyle(highlightStyle)
}

View file

@ -261,7 +261,7 @@ func TestRenderPipeSet(t *testing.T) {
tests := []struct {
name string
pipes []*Pipe
pipes []Pipe
commit *models.Commit
prevCommit *models.Commit
expectedStr string
@ -269,7 +269,7 @@ func TestRenderPipeSet(t *testing.T) {
}{
{
name: "single cell",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: TERMINATES, style: cyan},
{fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("c"), kind: STARTS, style: green},
},
@ -279,7 +279,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "single cell, selected",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("selected"), kind: TERMINATES, style: cyan},
{fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("c"), kind: STARTS, style: green},
},
@ -289,7 +289,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "terminating hook and starting hook, selected",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("selected"), kind: TERMINATES, style: cyan},
{fromPos: 1, toPos: 0, fromHash: pool("c"), toHash: pool("selected"), kind: TERMINATES, style: yellow},
{fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("d"), kind: STARTS, style: green},
@ -303,7 +303,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "terminating hook and starting hook, prioritise the terminating one",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: TERMINATES, style: red},
{fromPos: 1, toPos: 0, fromHash: pool("c"), toHash: pool("b"), kind: TERMINATES, style: magenta},
{fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("d"), kind: STARTS, style: green},
@ -317,7 +317,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "starting and terminating pipe sharing some space",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: yellow},
{fromPos: 1, toPos: 1, fromHash: pool("b1"), toHash: pool("b2"), kind: CONTINUES, style: magenta},
@ -332,7 +332,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "starting and terminating pipe sharing some space, with selection",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("selected"), kind: TERMINATES, style: red},
{fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("a3"), kind: STARTS, style: yellow},
{fromPos: 1, toPos: 1, fromHash: pool("b1"), toHash: pool("b2"), kind: CONTINUES, style: magenta},
@ -347,7 +347,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "many terminating pipes",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: yellow},
{fromPos: 1, toPos: 0, fromHash: pool("b1"), toHash: pool("a2"), kind: TERMINATES, style: magenta},
@ -361,7 +361,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "starting pipe passing through",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: yellow},
{fromPos: 0, toPos: 3, fromHash: pool("a2"), toHash: pool("d3"), kind: STARTS, style: yellow},
@ -376,7 +376,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "starting and terminating path crossing continuing path",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: yellow},
{fromPos: 0, toPos: 1, fromHash: pool("a2"), toHash: pool("b3"), kind: STARTS, style: yellow},
@ -391,7 +391,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "another clash of starting and terminating paths",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: yellow},
{fromPos: 0, toPos: 1, fromHash: pool("a2"), toHash: pool("b3"), kind: STARTS, style: yellow},
@ -406,7 +406,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "commit whose previous commit is selected",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: yellow},
},
@ -418,7 +418,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "commit whose previous commit is selected and is a merge commit",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 1, toPos: 1, fromHash: pool("selected"), toHash: pool("b3"), kind: CONTINUES, style: red},
},
@ -430,7 +430,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "commit whose previous commit is selected and is a merge commit, with continuing pipe inbetween",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 1, toPos: 1, fromHash: pool("z1"), toHash: pool("z3"), kind: CONTINUES, style: green},
{fromPos: 2, toPos: 2, fromHash: pool("selected"), toHash: pool("b3"), kind: CONTINUES, style: red},
@ -443,7 +443,7 @@ func TestRenderPipeSet(t *testing.T) {
},
{
name: "when previous commit is selected, not a merge commit, and spawns a continuing pipe",
pipes: []*Pipe{
pipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: red},
{fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: green},
{fromPos: 0, toPos: 1, fromHash: pool("a2"), toHash: pool("b3"), kind: STARTS, style: green},
@ -486,25 +486,25 @@ func TestGetNextPipes(t *testing.T) {
pool := func(s string) *string { return hashPool.Add(s) }
tests := []struct {
prevPipes []*Pipe
prevPipes []Pipe
commit *models.Commit
expected []*Pipe
expected []Pipe
}{
{
prevPipes: []*Pipe{
prevPipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: STARTS, style: style.FgDefault},
},
commit: models.NewCommit(hashPool, models.NewCommitOpts{
Hash: "b",
Parents: []string{"c"},
}),
expected: []*Pipe{
expected: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: TERMINATES, style: style.FgDefault},
{fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("c"), kind: STARTS, style: style.FgDefault},
},
},
{
prevPipes: []*Pipe{
prevPipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: TERMINATES, style: style.FgDefault},
{fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("c"), kind: STARTS, style: style.FgDefault},
{fromPos: 0, toPos: 1, fromHash: pool("b"), toHash: pool("d"), kind: STARTS, style: style.FgDefault},
@ -513,21 +513,21 @@ func TestGetNextPipes(t *testing.T) {
Hash: "d",
Parents: []string{"e"},
}),
expected: []*Pipe{
expected: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("c"), kind: CONTINUES, style: style.FgDefault},
{fromPos: 1, toPos: 1, fromHash: pool("b"), toHash: pool("d"), kind: TERMINATES, style: style.FgDefault},
{fromPos: 1, toPos: 1, fromHash: pool("d"), toHash: pool("e"), kind: STARTS, style: style.FgDefault},
},
},
{
prevPipes: []*Pipe{
prevPipes: []Pipe{
{fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("root"), kind: TERMINATES, style: style.FgDefault},
},
commit: models.NewCommit(hashPool, models.NewCommitOpts{
Hash: "root",
Parents: []string{},
}),
expected: []*Pipe{
expected: []Pipe{
{fromPos: 1, toPos: 1, fromHash: pool("root"), toHash: pool(models.EmptyTreeCommitHash), kind: STARTS, style: style.FgDefault},
},
},