From 864acacaa38d7ad231031b78f8d62999dd9a3022 Mon Sep 17 00:00:00 2001 From: Julien Cretel Date: Thu, 20 Mar 2025 12:59:45 +0100 Subject: [PATCH 1/3] bytes: add FuzzReplace test --- src/bytes/bytes_test.go | 46 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/bytes/bytes_test.go b/src/bytes/bytes_test.go index ead581718ace8f..a9ee654c87e86b 100644 --- a/src/bytes/bytes_test.go +++ b/src/bytes/bytes_test.go @@ -1804,6 +1804,52 @@ 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) + } + }) +} + type TitleTest struct { in, out string } From 6d2edacaf8b6b4410821ada42ad1c2b7f91b9bf2 Mon Sep 17 00:00:00 2001 From: Julien Cretel Date: Thu, 20 Mar 2025 10:43:37 +0100 Subject: [PATCH 2/3] bytes: add benchmarks for Replace --- src/bytes/bytes_test.go | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/src/bytes/bytes_test.go b/src/bytes/bytes_test.go index a9ee654c87e86b..14b52a80358bb7 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) } @@ -1850,6 +1862,23 @@ func FuzzReplace(f *testing.F) { }) } +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 } From d1caf1f0845a402a026764068a1db4dcf73e9017 Mon Sep 17 00:00:00 2001 From: Julien Cretel Date: Thu, 20 Mar 2025 12:35:11 +0100 Subject: [PATCH 3/3] 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. 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% --- src/bytes/bytes.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/bytes/bytes.go b/src/bytes/bytes.go index 4bc375df192ce4..8198415c3e1938 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) - } - w += copy(t[w:], s[start:j]) + 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) + } + } 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]