From c1ad6e7474ffbce83189305e4c8a3b30fba2a98f Mon Sep 17 00:00:00 2001 From: Tapir Liu Date: Thu, 14 Jun 2018 11:43:30 -0400 Subject: [PATCH 1/6] Implement Repeat with Builder to avoid one unnecessary allocation. benchmark old ns/op new ns/op delta BenchmarkRepeat/length-1-4 290 225 -22.41% BenchmarkRepeat/length-5-4 621 391 -37.04% BenchmarkRepeat/length-10-4 900 556 -38.22% benchmark old allocs new allocs delta BenchmarkRepeat/length-1-4 2 1 -50.00% BenchmarkRepeat/length-5-4 2 1 -50.00% BenchmarkRepeat/length-10-4 2 1 -50.00% --- src/strings/strings.go | 23 ++++++++++++++++------- src/strings/strings_test.go | 16 +++++++++++++++- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/strings/strings.go b/src/strings/strings.go index adbbe742fcebe6..a0a31f7c58e51c 100644 --- a/src/strings/strings.go +++ b/src/strings/strings.go @@ -541,17 +541,26 @@ func Repeat(s string, count int) string { // See Issue golang.org/issue/16237 if count < 0 { panic("strings: negative Repeat count") - } else if count > 0 && len(s)*count/count != len(s) { + } else if count == 0 { + return "" + } else if len(s)*count/count != len(s) { panic("strings: Repeat count causes overflow") } - b := make([]byte, len(s)*count) - bp := copy(b, s) - for bp < len(b) { - copy(b[bp:], b[:bp]) - bp *= 2 + k, n := len(s), len(s)*count + var b Builder + b.Grow(n) + b.WriteString(s) + for k < n { + if k + k < n { + b.WriteString(b.String()) + k = k + k + } else { + b.WriteString(b.String()[:n-k]) + break + } } - return string(b) + return b.String() } // ToUpper returns a copy of the string s with all Unicode letters mapped to their upper case. diff --git a/src/strings/strings_test.go b/src/strings/strings_test.go index 78bc573e5f0bf1..ffc4ab09647a6b 100644 --- a/src/strings/strings_test.go +++ b/src/strings/strings_test.go @@ -1648,7 +1648,21 @@ func BenchmarkSplitNMultiByteSeparator(b *testing.B) { func BenchmarkRepeat(b *testing.B) { for i := 0; i < b.N; i++ { - Repeat("-", 80) + b.Run("length-1", func(b *testing.B) { + for i := 0; i < b.N; i++ { + Repeat("-", 80) + } + }) + b.Run("length-5", func(b *testing.B) { + for i := 0; i < b.N; i++ { + Repeat("-!@#$", 80) + } + }) + b.Run("length-10", func(b *testing.B) { + for i := 0; i < b.N; i++ { + Repeat("-!@#$abcde", 80) + } + }) } } From 3d2e842624b64b0eedb8663d0f13e4facb1434cf Mon Sep 17 00:00:00 2001 From: Tapir Liu Date: Fri, 15 Jun 2018 11:57:01 -0400 Subject: [PATCH 2/6] remove outer loop in Repeat benchmark --- src/strings/strings_test.go | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/strings/strings_test.go b/src/strings/strings_test.go index ffc4ab09647a6b..e9d82bc2e63557 100644 --- a/src/strings/strings_test.go +++ b/src/strings/strings_test.go @@ -1647,23 +1647,21 @@ func BenchmarkSplitNMultiByteSeparator(b *testing.B) { } func BenchmarkRepeat(b *testing.B) { - for i := 0; i < b.N; i++ { - b.Run("length-1", func(b *testing.B) { - for i := 0; i < b.N; i++ { - Repeat("-", 80) - } - }) - b.Run("length-5", func(b *testing.B) { - for i := 0; i < b.N; i++ { - Repeat("-!@#$", 80) - } - }) - b.Run("length-10", func(b *testing.B) { - for i := 0; i < b.N; i++ { - Repeat("-!@#$abcde", 80) - } - }) - } + b.Run("length-1", func(b *testing.B) { + for i := 0; i < b.N; i++ { + Repeat("-", 80) + } + }) + b.Run("length-5", func(b *testing.B) { + for i := 0; i < b.N; i++ { + Repeat("-!@#$", 80) + } + }) + b.Run("length-10", func(b *testing.B) { + for i := 0; i < b.N; i++ { + Repeat("-!@#$abcde", 80) + } + }) } func BenchmarkIndexAnyASCII(b *testing.B) { From 3d336b29b2e7bde263528c00735a35e4783ae5f8 Mon Sep 17 00:00:00 2001 From: go101 Date: Tue, 21 Aug 2018 12:13:47 -0400 Subject: [PATCH 3/6] Adjust a little for Repeat function. --- src/strings/strings.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/strings/strings.go b/src/strings/strings.go index e250d99bae7bf2..75b6dd6b67fde5 100644 --- a/src/strings/strings.go +++ b/src/strings/strings.go @@ -547,16 +547,15 @@ func Repeat(s string, count int) string { panic("strings: Repeat count causes overflow") } - k, n := len(s), len(s)*count + n := len(s)*count var b Builder b.Grow(n) b.WriteString(s) - for k < n { - if k + k < n { + for b.Len() < n { + if b.Len() <= n>>1 { b.WriteString(b.String()) - k = k + k } else { - b.WriteString(b.String()[:n-k]) + b.WriteString(b.String()[:n-b.Len()]) break } } From c49153f03861abc5aa84f8067c27fbb54ef6de43 Mon Sep 17 00:00:00 2001 From: go101 Date: Tue, 21 Aug 2018 13:38:15 -0400 Subject: [PATCH 4/6] move the case of count==0 in Repeat function to the most top of the function --- src/strings/strings.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/strings/strings.go b/src/strings/strings.go index 75b6dd6b67fde5..4e21aff1bb56ae 100644 --- a/src/strings/strings.go +++ b/src/strings/strings.go @@ -535,19 +535,21 @@ func Map(mapping func(rune) rune, s string) string { // It panics if count is negative or if // the result of (len(s) * count) overflows. func Repeat(s string, count int) string { + if count == 0 { + return "" + } + // Since we cannot return an error on overflow, // we should panic if the repeat will generate // an overflow. // See Issue golang.org/issue/16237 if count < 0 { panic("strings: negative Repeat count") - } else if count == 0 { - return "" } else if len(s)*count/count != len(s) { panic("strings: Repeat count causes overflow") } - n := len(s)*count + n := len(s) * count var b Builder b.Grow(n) b.WriteString(s) From 54d85542b0ee5fa030346f06a5ce4e505aae0fe2 Mon Sep 17 00:00:00 2001 From: go101 Date: Tue, 21 Aug 2018 14:26:31 -0400 Subject: [PATCH 5/6] change n>>1 to n/2; new BenchmarkRepeat --- src/strings/strings.go | 2 +- src/strings/strings_test.go | 23 +++++++++-------------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/strings/strings.go b/src/strings/strings.go index 4e21aff1bb56ae..59f70da845512d 100644 --- a/src/strings/strings.go +++ b/src/strings/strings.go @@ -554,7 +554,7 @@ func Repeat(s string, count int) string { b.Grow(n) b.WriteString(s) for b.Len() < n { - if b.Len() <= n>>1 { + if b.Len() <= n/2 { b.WriteString(b.String()) } else { b.WriteString(b.String()[:n-b.Len()]) diff --git a/src/strings/strings_test.go b/src/strings/strings_test.go index e9d82bc2e63557..50bac5d465ffb0 100644 --- a/src/strings/strings_test.go +++ b/src/strings/strings_test.go @@ -1647,21 +1647,16 @@ func BenchmarkSplitNMultiByteSeparator(b *testing.B) { } func BenchmarkRepeat(b *testing.B) { - b.Run("length-1", func(b *testing.B) { - for i := 0; i < b.N; i++ { - Repeat("-", 80) - } - }) - b.Run("length-5", func(b *testing.B) { - for i := 0; i < b.N; i++ { - Repeat("-!@#$", 80) - } - }) - b.Run("length-10", func(b *testing.B) { - for i := 0; i < b.N; i++ { - Repeat("-!@#$abcde", 80) + s := Repeat("-", 16) + for n := 1; n <= 16; n <<= 1 { + for _, c := range []int{2, 6, 10} { + b.Run(fmt.Sprintf("length=%d-count=%d", n, c), func(b *testing.B) { + for i := 0; i < b.N; i++ { + Repeat(s[:n], c) + } + }) } - }) + } } func BenchmarkIndexAnyASCII(b *testing.B) { From 4b2c73f3bfa0b3789268b9ea6e1ecdb984e8087c Mon Sep 17 00:00:00 2001 From: go101 Date: Wed, 22 Aug 2018 11:55:40 -0400 Subject: [PATCH 6/6] adjust Repeat benchmark --- src/strings/strings_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/strings/strings_test.go b/src/strings/strings_test.go index 50bac5d465ffb0..b5c648574939f7 100644 --- a/src/strings/strings_test.go +++ b/src/strings/strings_test.go @@ -1647,10 +1647,10 @@ func BenchmarkSplitNMultiByteSeparator(b *testing.B) { } func BenchmarkRepeat(b *testing.B) { - s := Repeat("-", 16) - for n := 1; n <= 16; n <<= 1 { - for _, c := range []int{2, 6, 10} { - b.Run(fmt.Sprintf("length=%d-count=%d", n, c), func(b *testing.B) { + s := "0123456789" + for _, n := range []int{5, 10} { + for _, c := range []int{1, 2, 6} { + b.Run(fmt.Sprintf("%dx%d", n, c), func(b *testing.B) { for i := 0; i < b.N; i++ { Repeat(s[:n], c) }