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.

Author: stefanhaller

pkg/gui/presentation/commits.go

@@ -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{

pkg/gui/presentation/graph/graph.go

@@ -56,20 +56,20 @@ func RenderCommitGraph(commits []*models.Commit, selectedCommitHash *string, get
 	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, selectedCommitHash *string) []string {
+func RenderAux(pipeSets [][]Pipe, commits []*models.Commit, selectedCommitHash *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, selectedCommitHash
 	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,
 	selectedCommitHash *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, selectedCommitHash)
 	})
 
 	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)
 		}

pkg/gui/presentation/graph/graph_test.go

@@ -261,15 +261,15 @@ func TestRenderPipeSet(t *testing.T) {
 
 	tests := []struct {
 		name           string
-		pipes          []*Pipe
+		pipes          []Pipe
 		commit         *models.Commit
 		prevCommit     *models.Commit
 		expectedStr    string
 		expectedStyles []style.TextStyle
 	}{
 		{
 			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},
 			},
 		},