From 44fef2aa4db70bfbe7bfd2abbd5b12eb776cde1d Mon Sep 17 00:00:00 2001 From: Julien Cretel Date: Thu, 20 Mar 2025 20:42:05 +0000 Subject: [PATCH] bytes: speed up Replace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL is to package bytes what CL 657935 was to package strings. Add fuzz test and benchmarks for Replace. The length of parameter old does not change. Move the corresponding length check outside the loop. Use range-over-int loops where possible. Some benchmark results (no changes to allocations): goos: darwin goarch: amd64 pkg: bytes cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz │ old │ new │ │ sec/op │ sec/op vs base │ Replace/"hello"_"l"_"L"_0-8 27.83n ± 2% 27.22n ± 1% -2.17% (p=0.000 n=20) Replace/"hello"_"l"_"L"_-1-8 60.64n ± 0% 57.97n ± 1% -4.40% (p=0.000 n=20) Replace/"hello"_"x"_"X"_-1-8 33.99n ± 0% 33.88n ± 0% ~ (p=0.140 n=20) Replace/""_"x"_"X"_-1-8 10.40n ± 1% 10.57n ± 0% +1.64% (p=0.000 n=20) Replace/"radar"_"r"_""_-1-8 62.63n ± 0% 61.39n ± 0% -1.98% (p=0.000 n=20) Replace/""_""_"<>"_-1-8 29.76n ± 1% 24.18n ± 1% -18.75% (p=0.000 n=20) Replace/"banana"_"a"_"<>"_-1-8 77.00n ± 0% 77.10n ± 1% ~ (p=0.525 n=20) Replace/"banana"_"a"_"<>"_1-8 44.24n ± 0% 43.57n ± 1% -1.54% (p=0.000 n=20) Replace/"banana"_"a"_"<>"_1000-8 78.23n ± 0% 77.16n ± 1% -1.36% (p=0.000 n=20) Replace/"banana"_"an"_"<>"_-1-8 72.78n ± 1% 69.97n ± 1% -3.85% (p=0.000 n=20) Replace/"banana"_"ana"_"<>"_-1-8 54.41n ± 0% 54.04n ± 1% -0.67% (p=0.033 n=20) Replace/"banana"_""_"<>"_-1-8 116.8n ± 1% 103.5n ± 1% -11.42% (p=0.000 n=20) Replace/"banana"_""_"<>"_10-8 117.2n ± 1% 103.6n ± 0% -11.60% (p=0.000 n=20) Replace/"banana"_""_"<>"_6-8 105.30n ± 0% 92.50n ± 0% -12.16% (p=0.000 n=20) Replace/"banana"_""_"<>"_5-8 91.81n ± 0% 79.87n ± 1% -13.01% (p=0.000 n=20) Replace/"banana"_""_"<>"_1-8 35.87n ± 1% 30.33n ± 1% -15.43% (p=0.000 n=20) Replace/"banana"_"a"_"a"_-1-8 70.84n ± 0% 68.83n ± 0% -2.84% (p=0.000 n=20) Replace/"banana"_"a"_"a"_1-8 44.27n ± 1% 43.47n ± 1% -1.80% (p=0.000 n=20) Replace/"☺☻☹"_""_"<>"_-1-8 104.25n ± 1% 93.33n ± 0% -10.48% (p=0.000 n=20) geomean 56.31n 52.88n -6.09% Change-Id: I5daf44ccfd887da445d8c681415c32de7c2b85d1 GitHub-Last-Rev: d1caf1f0845a402a026764068a1db4dcf73e9017 GitHub-Pull-Request: golang/go#72967 Reviewed-on: https://go-review.googlesource.com/c/go/+/659515 Reviewed-by: Dmitri Shuralyov Auto-Submit: Jorropo Reviewed-by: Jorropo Reviewed-by: Cherry Mui LUCI-TryBot-Result: Go LUCI --- src/bytes/bytes.go | 25 +++++++------ src/bytes/bytes_test.go | 81 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/src/bytes/bytes.go b/src/bytes/bytes.go index 4bc375df19..8198415c3e 100644 --- a/src/bytes/bytes.go +++ b/src/bytes/bytes.go @@ -1192,19 +1192,22 @@ func Replace(s, old, new []byte, n int) []byte { t := make([]byte, len(s)+n*(len(new)-len(old))) w := 0 start := 0 - for i := 0; i < n; i++ { - j := start - if len(old) == 0 { - if i > 0 { - _, wid := utf8.DecodeRune(s[start:]) - j += wid - } - } else { - j += Index(s[start:], old) + if len(old) > 0 { + for range n { + j := start + Index(s[start:], old) + w += copy(t[w:], s[start:j]) + w += copy(t[w:], new) + start = j + len(old) } - w += copy(t[w:], s[start:j]) + } else { // len(old) == 0 w += copy(t[w:], new) - start = j + len(old) + for range n - 1 { + _, wid := utf8.DecodeRune(s[start:]) + j := start + wid + w += copy(t[w:], s[start:j]) + w += copy(t[w:], new) + start = j + } } w += copy(t[w:], s[start:]) return t[0:w] diff --git a/src/bytes/bytes_test.go b/src/bytes/bytes_test.go index ead581718a..14b52a8035 100644 --- a/src/bytes/bytes_test.go +++ b/src/bytes/bytes_test.go @@ -7,6 +7,7 @@ package bytes_test import ( . "bytes" "fmt" + "internal/asan" "internal/testenv" "iter" "math" @@ -1786,9 +1787,20 @@ var ReplaceTests = []ReplaceTest{ func TestReplace(t *testing.T) { for _, tt := range ReplaceTests { - in := append([]byte(tt.in), ""...) + var ( + in = []byte(tt.in) + old = []byte(tt.old) + new = []byte(tt.new) + ) + if !asan.Enabled { + allocs := testing.AllocsPerRun(10, func() { Replace(in, old, new, tt.n) }) + if allocs > 1 { + t.Errorf("Replace(%q, %q, %q, %d) allocates %.2f objects", tt.in, tt.old, tt.new, tt.n, allocs) + } + } + in = append(in, ""...) in = in[:len(tt.in)] - out := Replace(in, []byte(tt.old), []byte(tt.new), tt.n) + out := Replace(in, old, new, tt.n) if s := string(out); s != tt.out { t.Errorf("Replace(%q, %q, %q, %d) = %q, want %q", tt.in, tt.old, tt.new, tt.n, s, tt.out) } @@ -1796,7 +1808,7 @@ func TestReplace(t *testing.T) { t.Errorf("Replace(%q, %q, %q, %d) didn't copy", tt.in, tt.old, tt.new, tt.n) } if tt.n == -1 { - out := ReplaceAll(in, []byte(tt.old), []byte(tt.new)) + out := ReplaceAll(in, old, new) if s := string(out); s != tt.out { t.Errorf("ReplaceAll(%q, %q, %q) = %q, want %q", tt.in, tt.old, tt.new, s, tt.out) } @@ -1804,6 +1816,69 @@ func TestReplace(t *testing.T) { } } +func FuzzReplace(f *testing.F) { + for _, tt := range ReplaceTests { + f.Add([]byte(tt.in), []byte(tt.old), []byte(tt.new), tt.n) + } + f.Fuzz(func(t *testing.T, in, old, new []byte, n int) { + differentImpl := func(in, old, new []byte, n int) []byte { + var out Buffer + if n < 0 { + n = math.MaxInt + } + for i := 0; i < len(in); { + if n == 0 { + out.Write(in[i:]) + break + } + if HasPrefix(in[i:], old) { + out.Write(new) + i += len(old) + n-- + if len(old) != 0 { + continue + } + if i == len(in) { + break + } + } + if len(old) == 0 { + _, length := utf8.DecodeRune(in[i:]) + out.Write(in[i : i+length]) + i += length + } else { + out.WriteByte(in[i]) + i++ + } + } + if len(old) == 0 && n != 0 { + out.Write(new) + } + return out.Bytes() + } + if simple, replace := differentImpl(in, old, new, n), Replace(in, old, new, n); !slices.Equal(simple, replace) { + t.Errorf("The two implementations do not match %q != %q for Replace(%q, %q, %q, %d)", simple, replace, in, old, new, n) + } + }) +} + +func BenchmarkReplace(b *testing.B) { + for _, tt := range ReplaceTests { + desc := fmt.Sprintf("%q %q %q %d", tt.in, tt.old, tt.new, tt.n) + var ( + in = []byte(tt.in) + old = []byte(tt.old) + new = []byte(tt.new) + ) + b.Run(desc, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + Replace(in, old, new, tt.n) + } + }) + } +} + type TitleTest struct { in, out string }