From c566273ea7d8d62ae2c568e6d1b8d8e4d10bdcbb Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 13 Sep 2026 08:47:13 -0400 Subject: [PATCH] Reduce diff allocations and fix line counting Hunk lines and generateAddedDiff wrote prefix+line+newline via string concat; write the three parts directly. countLines used bufio.Scanner, which stops silently on any line over 64 KiB and returned a short count; use bytes.Count instead. Deleted files now report LinesDeleted and both totals include added and deleted files, not just modified hunks. --- diff/diff.go | 49 ++++++++++++++++++++++++++++++++--------------- diff/diff_test.go | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/diff/diff.go b/diff/diff.go index 0791869..cfad245 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -2,7 +2,6 @@ package diff import ( - "bufio" "bytes" "fmt" "io" @@ -107,9 +106,9 @@ func Compare(oldReader, newReader archives.Reader) (*CompareResult, error) { result.FilesAdded++ case TypeModified: result.FilesChanged++ - result.TotalAdded += fileDiff.LinesAdded - result.TotalDeleted += fileDiff.LinesDeleted } + result.TotalAdded += fileDiff.LinesAdded + result.TotalDeleted += fileDiff.LinesDeleted result.Files = append(result.Files, fileDiff) } @@ -125,7 +124,15 @@ func compareFile(path string, oldInfo, newInfo archives.FileInfo, oldReader, new switch { case inOld && !inNew: - return FileDiff{Path: path, Type: TypeDeleted}, true + fd := FileDiff{Path: path, Type: TypeDeleted} + if content, err := readFileContent(oldReader, path); err == nil { + if !isDiffableText(content) { + fd.IsBinary = true + } else { + fd.LinesDeleted = countLines(content) + } + } + return fd, true case !inOld && inNew: fd := FileDiff{Path: path, Type: TypeAdded} @@ -248,25 +255,25 @@ func generateSimpleDiff(path string, oldContent, newContent []byte) (string, int // Context before for i := hunkOldStart; i < oldStart && i < len(oldLines); i++ { - hunk.WriteString(" " + oldLines[i] + "\n") + writeDiffLine(&hunk, ' ', oldLines[i]) } // Deleted lines for i := oldStart; i < oldStart+oldCount && i < len(oldLines); i++ { - hunk.WriteString("-" + oldLines[i] + "\n") + writeDiffLine(&hunk, '-', oldLines[i]) linesDeleted++ } // Added lines for i := newStart; i < newStart+newCount && i < len(newLines); i++ { - hunk.WriteString("+" + newLines[i] + "\n") + writeDiffLine(&hunk, '+', newLines[i]) linesAdded++ } // Context after afterStart := oldStart + oldCount for i := 0; i < contextAfter && afterStart+i < len(oldLines); i++ { - hunk.WriteString(" " + oldLines[afterStart+i] + "\n") + writeDiffLine(&hunk, ' ', oldLines[afterStart+i]) } // Calculate hunk size @@ -291,24 +298,36 @@ func generateAddedDiff(path string, content []byte) string { fmt.Fprintf(&buf, "@@ -0,0 +1,%d @@\n", len(lines)) for _, line := range lines { - buf.WriteString("+" + string(line) + "\n") + buf.WriteByte('+') + buf.Write(line) + buf.WriteByte('\n') } return buf.String() } +func writeDiffLine(b *strings.Builder, prefix byte, line string) { + b.WriteByte(prefix) + b.WriteString(line) + b.WriteByte('\n') +} + var diffPathReplacer = strings.NewReplacer("\n", "", "\r", "") func sanitizeDiffPath(path string) string { return diffPathReplacer.Replace(path) } -// countLines counts the number of lines in content. +// countLines counts the number of lines in content. bufio.Scanner is not +// used here because it silently stops on any line longer than 64 KiB +// (minified JS, source maps) and returns a short count. func countLines(content []byte) int { - scanner := bufio.NewScanner(bytes.NewReader(content)) - count := 0 - for scanner.Scan() { - count++ + if len(content) == 0 { + return 0 + } + n := bytes.Count(content, []byte{'\n'}) + if content[len(content)-1] != '\n' { + n++ } - return count + return n } diff --git a/diff/diff_test.go b/diff/diff_test.go index 3694e72..49fb3ca 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -86,11 +86,28 @@ func TestCompare(t *testing.T) { // Check deleted file if f, ok := fileMap["deleted.txt"]; !ok || f.Type != TypeDeleted { t.Error("deleted.txt should be marked as deleted") + } else if f.LinesDeleted != 1 { + t.Errorf("deleted.txt LinesDeleted = %d, want 1", f.LinesDeleted) } // Check added file if f, ok := fileMap["added.txt"]; !ok || f.Type != TypeAdded { t.Error("added.txt should be marked as added") + } else if f.LinesAdded != 1 { + t.Errorf("added.txt LinesAdded = %d, want 1", f.LinesAdded) + } + + // Totals include added and deleted files as well as modified hunks. + var wantAdded, wantDeleted int + for _, f := range result.Files { + wantAdded += f.LinesAdded + wantDeleted += f.LinesDeleted + } + if result.TotalAdded != wantAdded { + t.Errorf("TotalAdded = %d, want sum of per-file %d", result.TotalAdded, wantAdded) + } + if result.TotalDeleted != wantDeleted { + t.Errorf("TotalDeleted = %d, want sum of per-file %d", result.TotalDeleted, wantDeleted) } // Check modified files @@ -195,6 +212,8 @@ func TestCountLines(t *testing.T) { {"one line", []byte("hello"), 1}, {"three lines", []byte("line1\nline2\nline3"), 3}, {"trailing newline", []byte("line1\nline2\n"), 2}, + {"only newline", []byte("\n"), 1}, + {"long line", append(bytes.Repeat([]byte{'x'}, 100_000), '\n', 'y'), 2}, } for _, tt := range tests { @@ -207,6 +226,26 @@ func TestCountLines(t *testing.T) { } } +func BenchmarkGenerateAddedDiff(b *testing.B) { + content := bytes.Repeat([]byte("the quick brown fox jumps over the lazy dog\n"), 10_000) + b.SetBytes(int64(len(content))) + b.ReportAllocs() + for b.Loop() { + _ = generateAddedDiff("bench.txt", content) + } +} + +func BenchmarkGenerateSimpleDiff(b *testing.B) { + line := []byte("the quick brown fox jumps over the lazy dog\n") + oldContent := bytes.Repeat(line, 5000) + newContent := append(bytes.Repeat(line, 2500), bytes.Repeat([]byte("THE QUICK BROWN FOX\n"), 2500)...) + b.SetBytes(int64(len(oldContent) + len(newContent))) + b.ReportAllocs() + for b.Loop() { + _, _, _ = generateSimpleDiff("bench.txt", oldContent, newContent) + } +} + func TestCompareIdentical(t *testing.T) { files := map[string]string{ "README.md": "# Test\n",