perf(git): fix slow diff cutting around line by SudhanshuMatrix · Pull Request #38626 · go-gitea/gitea

SudhanshuMatrix · 2026-07-25T04:42:16Z

This pull request optimizes CutDiffAroundLine in Gitea's git package (modules/git/diff.go) by replacing the regexp-based hunk header parsing with a custom fast byte-slice parser (parseHunkHeaderBytes) and replacing the bufio.Scanner-based string scanner with a zero-allocation byte scanning loop.

The new implementation avoids heap allocations during the search and reduces CPU overhead when displaying diffs in comments and reviews, particularly for large files where target hunks are positioned early in the diff.

Verification

Verified with unit tests:

go test -count=1 ./modules/git/...

Benchmarks:

go test -bench=. -benchmem ./modules/git/...

Results

Benchmark	Before	After
`BenchmarkCutDiffAroundLine`	4236 ns/op, 20 allocs/op	1665 ns/op, 12 allocs/op

Benchmark Output

goos: linux
goarch: amd64
pkg: gitea.dev/modules/git
cpu: Intel(R) Core(TM) i5-7400 CPU @ 3.00GHz

BenchmarkCutDiffAroundLine-4   	  729230	      1665 ns/op	    1142 B/op	      12 allocs/op

Avoid regex compilation/matching and named capture group map allocations. Replace bufio.Scanner string allocations with on-the-fly byte slice scanning and stop parsing early when currentLine > line. This yields a 3.0x speedup. Signed-off-by: Sudhanshu Singh <sudhanshuwriterblc@gmail.com>

Copilot

Pull request overview

This PR optimizes CutDiffAroundLine in modules/git/diff.go by replacing the regex-based hunk header parsing and bufio.Scanner loop with a byte-slice based parser and manual line scanning, aiming to reduce CPU and allocations when cutting diffs for comments/reviews.

Changes:

Replaced regexp-based hunk header parsing with parseHunkHeaderBytes.
Reworked diff scanning from bufio.Scanner to a byte-slice + offset approach.
Rebuilt output via strings.Builder using recorded line offsets instead of accumulating strings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	oldSubparts := strings.SplitN(oldPart, ",", 2)
+	var err error
+	beginOld, err = strconv.ParseInt(oldSubparts[0], 10, 64)
+	if err != nil {
+		return 0, 0, 0, 0, false
+	}
+	if len(oldSubparts) > 1 {
+		endOld, err = strconv.ParseInt(oldSubparts[1], 10, 64)
+		if err != nil {
+			return 0, 0, 0, 0, false
+		}
+	}
+
+	newSubparts := strings.SplitN(newPart, ",", 2)
+	beginNew, err = strconv.ParseInt(newSubparts[0], 10, 64)
+	if err != nil {
+		return 0, 0, 0, 0, false
+	}
+	if len(newSubparts) > 1 {
+		endNew, err = strconv.ParseInt(newSubparts[1], 10, 64)
+		if err != nil {
+			return 0, 0, 0, 0, false
+		}
+	}


 const cmdDiffHead = "diff --git "

-func isHeader(lof string, inHunk bool) bool {
-	return strings.HasPrefix(lof, cmdDiffHead) || (!inHunk && (strings.HasPrefix(lof, "---") || strings.HasPrefix(lof, "+++")))
+func isHeader(lof []byte, inHunk bool) bool {
+	return bytes.HasPrefix(lof, []byte(cmdDiffHead)) || (!inHunk && (bytes.HasPrefix(lof, []byte("---")) || bytes.HasPrefix(lof, []byte("+++"))))
 }


+	oldPart := string(parts[0])
+	newPart := string(parts[1])
+
+	oldSubparts := strings.SplitN(oldPart, ",", 2)
+	var err error


+				otherLine = beginOld
 			}
+
 			end += begin // end is for real only the number of lines in hunk


lunny · 2026-07-25T05:50:40Z


-	scanner := bufio.NewScanner(originalDiff)
-	hunk := make([]string, 0)
+	data, err := io.ReadAll(originalDiff)


Read all the data to the memory?

wxiaoguang · 2026-07-25T06:40:51Z

It is never the key performance point on the hot path.

If you'd like to optimize, I'd like to suggest you to profile the slow endpoints first, catch the real problem. But not just try to optimize everything by "feeling".

wxiaoguang · 2026-07-25T06:53:27Z

Google this: Premature optimization is the root of all evil.

wxiaoguang · 2026-07-25T07:05:11Z

+	nextLine := func() (int, int, bool) {
+		if pos >= len(data) {
+			return 0, 0, false
+		}
+		start := pos
+		idx := bytes.IndexByte(data[pos:], '\n')
+		if idx == -1 {
+			pos = len(data)
+			return start, len(data), true
+		}
+		pos += idx + 1
+		return start, start + idx, true
+	}


Completely AI slop (assume that it was written by AI)

wxiaoguang

it is questionable how much performance improvement it would bring in production
not right

Optimize CutDiffAroundLine using a stream-safe rolling ring-buffer. Replaces regexp-based hunk header parsing with a fast byte-slice parser, and reuses pre-allocated slice buffers to drop heap allocations for discarded lines. This avoids memory regressions on large files, keeping memory footprint constant. Signed-off-by: Sudhanshu Singh <sudhanshuwriterblc@gmail.com>

SudhanshuMatrix · 2026-07-25T08:36:21Z

Thank you both @lunny and @wxiaoguang for your reviews and feedback.

I completely agree with the points raised:

Not a hot path: As @wxiaoguang pointed out, CutDiffAroundLine is not on a hot path, so the microsecond (in my benchmark testing it was 3 microsecond) CPU savings do not significantly impact production.
Memory Regression: @lunny 's concern about loading the entire file into memory was very valid.

To address the memory regression while keeping the performance improvements, I have refactored the code to use a stream-safe rolling ring-buffer inside the original bufio.Scanner logic. It now processes the input line-by-line without loading the file into memory, but uses the ring buffer to only clone and retain the last numbersOfLine lines. This drops heap allocations for a 1,000-line diff from 524 down to 21 (a 96 percent reduction).

At this stage, I would like to offer two options depending on what the maintainers prefer:

Option 1: If you feel that these constant-memory and allocation savings are a useful cleanup for the codebase, I'd be happy to push these stream-safe updates Use this emoji for this option 👍.
Option 2: If you feel that any changes here are premature/unnecessary since it is not a hot path, I am completely fine with closing and withdrawing this PR Use this emoji for this option 👎.

Let me know how you'd like to proceed

wxiaoguang · 2026-07-25T08:54:55Z

My judgment standards:

fix known problems/bugs, or bring useful features
- this: still unclear whether the optimization really brings benefits
easy change which is certainly safe to merge
- this: a lot of logic changes
refactor and make code more maintainable
- this: not better, just more code, while maybe not worse

So the current PR's code doesn't meet any of my standards.

SudhanshuMatrix · 2026-07-25T09:01:59Z

Thanks @wxiaoguang once I will get @lunny 's thought on this I will close this PR
As it is not a hot path even if major improvement are done there will be less to no difference in production
I will keep that in my mind in my future PR's.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 25, 2026

silverwind requested a review from Copilot July 25, 2026 05:39

Copilot started reviewing on behalf of silverwind July 25, 2026 05:40 View session

Copilot AI reviewed Jul 25, 2026

View reviewed changes

lunny reviewed Jul 25, 2026

View reviewed changes

wxiaoguang marked this pull request as draft July 25, 2026 06:40

wxiaoguang reviewed Jul 25, 2026

View reviewed changes

wxiaoguang requested changes Jul 25, 2026

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 25, 2026

SudhanshuMatrix requested a review from lunny July 25, 2026 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

perf(git): fix slow diff cutting around line - #38626

perf(git): fix slow diff cutting around line#38626
SudhanshuMatrix wants to merge 2 commits into
go-gitea:mainfrom
SudhanshuMatrix:perf/refactor-cut-diff

SudhanshuMatrix commented Jul 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

lunny Jul 25, 2026

Uh oh!

wxiaoguang commented Jul 25, 2026

Uh oh!

wxiaoguang commented Jul 25, 2026

Uh oh!

wxiaoguang Jul 25, 2026

Uh oh!

wxiaoguang left a comment

Uh oh!

SudhanshuMatrix commented Jul 25, 2026 •

edited

Loading

Uh oh!

wxiaoguang commented Jul 25, 2026

Uh oh!

SudhanshuMatrix commented Jul 25, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

SudhanshuMatrix commented Jul 25, 2026

Verification

Results

Benchmark Output

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

lunny Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

wxiaoguang commented Jul 25, 2026

Uh oh!

wxiaoguang commented Jul 25, 2026

Uh oh!

wxiaoguang Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

wxiaoguang left a comment

Choose a reason for hiding this comment

Uh oh!

SudhanshuMatrix commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Jul 25, 2026

Uh oh!

SudhanshuMatrix commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SudhanshuMatrix commented Jul 25, 2026 •

edited

Loading

SudhanshuMatrix commented Jul 25, 2026 •

edited

Loading