[go] strconv: optimize Atoi for common case
Aliaksandr Valialkin (Gerrit)
Aliaksandr Valialkin has uploaded this change for review.
strconv: optimize Atoi for common case
strconv.Atoi is mostly used for parsing non-negative integers.
Opimize it for this case.
Benchmark results:
GOARCH=amd64
name old time/op new time/op delta
Atoi/1-4 16.7ns ± 1% 6.1ns ± 3% -63.35% (p=0.008 n=5+5)
Atoi/12-4 18.2ns ± 0% 6.3ns ± 0% -65.13% (p=0.000 n=4+5)
Atoi/123-4 19.3ns ± 1% 6.9ns ± 1% -64.22% (p=0.008 n=5+5)
Atoi/1234-4 20.3ns ± 1% 7.6ns ± 1% -62.68% (p=0.008 n=5+5)
Atoi/12345-4 21.2ns ± 0% 8.5ns ± 1% -59.85% (p=0.008 n=5+5)
Atoi/123456-4 22.3ns ± 0% 9.1ns ± 1% -59.11% (p=0.008 n=5+5)
Atoi/1234567-4 23.8ns ± 0% 9.5ns ± 0% -59.96% (p=0.016 n=4+5)
Atoi/12345678-4 24.9ns ± 1% 10.2ns ± 1% -59.15% (p=0.008 n=5+5)
Atoi/123456789-4 26.4ns ± 3% 10.9ns ± 1% -58.68% (p=0.008 n=5+5)
Atoi/1234567890-4 27.5ns ± 1% 11.8ns ± 1% -57.23% (p=0.008 n=5+5)
AtoiNeg/1-4 15.3ns ± 1% 15.9ns ± 3% +4.18% (p=0.008 n=5+5)
AtoiNeg/12-4 16.6ns ± 1% 17.3ns ± 1% +3.85% (p=0.008 n=5+5)
AtoiNeg/123-4 17.9ns ± 0% 18.6ns ± 1% +3.79% (p=0.008 n=5+5)
AtoiNeg/1234-4 19.2ns ± 1% 19.7ns ± 3% +2.50% (p=0.008 n=5+5)
AtoiNeg/12345-4 20.6ns ± 0% 20.8ns ± 0% +1.17% (p=0.016 n=5+4)
AtoiNeg/123456-4 21.7ns ± 1% 22.1ns ± 1% +1.85% (p=0.008 n=5+5)
AtoiNeg/1234567-4 23.1ns ± 1% 23.6ns ± 3% +2.17% (p=0.024 n=5+5)
AtoiNeg/12345678-4 24.4ns ± 1% 24.8ns ± 2% ~ (p=0.087 n=5+5)
AtoiNeg/123456789-4 25.8ns ± 1% 26.4ns ± 1% +2.25% (p=0.008 n=5+5)
AtoiNeg/1234567890-4 26.9ns ± 1% 27.4ns ± 0% +1.71% (p=0.016 n=5+4)
GOARCH=386
name old time/op new time/op delta
Atoi/1-4 51.7ns ± 1% 6.3ns ± 1% -87.72% (p=0.008 n=5+5)
Atoi/12-4 77.9ns ± 5% 9.3ns ± 4% -88.02% (p=0.008 n=5+5)
Atoi/123-4 102ns ± 2% 12ns ± 3% -88.28% (p=0.008 n=5+5)
Atoi/1234-4 122ns ± 2% 15ns ± 3% -87.92% (p=0.008 n=5+5)
Atoi/12345-4 149ns ± 1% 18ns ± 1% -88.20% (p=0.008 n=5+5)
Atoi/123456-4 173ns ± 5% 20ns ± 5% -88.24% (p=0.008 n=5+5)
Atoi/1234567-4 192ns ± 5% 23ns ± 1% -88.23% (p=0.008 n=5+5)
Atoi/12345678-4 221ns ± 6% 26ns ± 2% -88.39% (p=0.008 n=5+5)
Atoi/123456789-4 241ns ± 1% 28ns ± 1% -88.33% (p=0.016 n=4+5)
Atoi/1234567890-4 265ns ± 4% 263ns ± 1% ~ (p=0.794 n=5+5)
AtoiNeg/1-4 52.1ns ± 4% 52.6ns ± 1% ~ (p=0.190 n=5+4)
AtoiNeg/12-4 74.7ns ± 1% 77.9ns ± 6% ~ (p=0.254 n=5+5)
AtoiNeg/123-4 100ns ± 2% 101ns ± 4% ~ (p=0.389 n=5+5)
AtoiNeg/1234-4 123ns ± 6% 122ns ± 1% ~ (p=0.873 n=5+4)
AtoiNeg/12345-4 142ns ± 1% 145ns ± 0% +2.11% (p=0.016 n=5+4)
AtoiNeg/123456-4 165ns ± 1% 175ns ± 7% +6.30% (p=0.008 n=5+5)
AtoiNeg/1234567-4 193ns ± 6% 190ns ± 1% ~ (p=0.738 n=5+5)
AtoiNeg/12345678-4 215ns ± 4% 235ns ±25% ~ (p=0.071 n=5+5)
AtoiNeg/123456789-4 235ns ± 2% 249ns ± 7% ~ (p=0.095 n=5+5)
AtoiNeg/1234567890-4 258ns ± 1% 261ns ± 3% ~ (p=0.698 n=4+5)
Fixes #20557
Change-Id: Ib6245d88cffd4b037419e2bf8e4a71b86c6d773f
---
M src/strconv/atoi.go
M src/strconv/atoi_test.go
2 files changed, 79 insertions(+), 22 deletions(-)
diff --git a/src/strconv/atoi.go b/src/strconv/atoi.go
index 66df149..6279738 100644
--- a/src/strconv/atoi.go
+++ b/src/strconv/atoi.go
@@ -199,10 +199,25 @@
// Atoi returns the result of ParseInt(s, 10, 0) converted to type int.
func Atoi(s string) (int, error) {
- const fnAtoi = "Atoi"
+ if len(s) > 0 && s[0] != '-' && s[0] != '+' &&
+ (intSize == 32 && len(s) < 10 || intSize == 64 && len(s) < 19) {
+ // Fast path for positive integers
+ var n int
+ for i := 0; i < len(s); i++ {
+ ch := s[i]
+ v := ch - '0'
+ if v > 9 {
+ return 0, &NumError{"Atoi", s, ErrSyntax}
+ }
+ n = n*10 + int(v)
+ }
+ return n, nil
+ }
+
+ // slow path
i64, err := ParseInt(s, 10, 0)
if nerr, ok := err.(*NumError); ok {
- nerr.Func = fnAtoi
+ nerr.Func = "Atoi"
}
return int(i64), err
}
diff --git a/src/strconv/atoi_test.go b/src/strconv/atoi_test.go
index d608505..0992c92 100644
--- a/src/strconv/atoi_test.go
+++ b/src/strconv/atoi_test.go
@@ -6,6 +6,7 @@
import (
"errors"
+ "fmt"
"reflect"
. "strconv"
"testing"
@@ -286,7 +287,7 @@
test := &atoui32tests[i]
out, err := ParseUint(test.in, 10, 0)
if test.out != uint32(out) || !reflect.DeepEqual(test.err, err) {
- t.Errorf("Atoui(%q) = %v, %v want %v, %v",
+ t.Errorf("ParseUint(%q) = %v, %v want %v, %v",
test.in, out, err, test.out, test.err)
}
}
@@ -295,7 +296,7 @@
test := &atoui64tests[i]
out, err := ParseUint(test.in, 10, 0)
if test.out != uint64(out) || !reflect.DeepEqual(test.err, err) {
- t.Errorf("Atoui(%q) = %v, %v want %v, %v",
+ t.Errorf("ParseUint(%q) = %v, %v want %v, %v",
test.in, out, err, test.out, test.err)
}
}
@@ -309,7 +310,7 @@
test := &atoi32tests[i]
out, err := ParseInt(test.in, 10, 0)
if test.out != int32(out) || !reflect.DeepEqual(test.err, err) {
- t.Errorf("Atoi(%q) = %v, %v want %v, %v",
+ t.Errorf("ParseInt(%q) = %v, %v want %v, %v",
test.in, out, err, test.out, test.err)
}
}
@@ -318,13 +319,46 @@
test := &atoi64tests[i]
out, err := ParseInt(test.in, 10, 0)
if test.out != int64(out) || !reflect.DeepEqual(test.err, err) {
- t.Errorf("Atoi(%q) = %v, %v want %v, %v",
+ t.Errorf("ParseInt(%q) = %v, %v want %v, %v",
test.in, out, err, test.out, test.err)
}
}
}
}
+func TestAtoi(t *testing.T) {
+ switch IntSize {
+ case 32:
+ for i := range atoi32tests {
+ test := &atoi32tests[i]
+ out, err := Atoi(test.in)
+ var errAtoi error
+ if test.err != nil {
+ numErr := test.err.(*NumError)
+ errAtoi = &NumError{"Atoi", numErr.Num, numErr.Err}
+ }
+ if test.out != int32(out) || !reflect.DeepEqual(errAtoi, err) {
+ t.Errorf("Atoi(%q) = %v, %v want %v, %v",
+ test.in, out, err, test.out, errAtoi)
+ }
+ }
+ case 64:
+ for i := range atoi64tests {
+ test := &atoi64tests[i]
+ out, err := Atoi(test.in)
+ var errAtoi error
+ if test.err != nil {
+ numErr := test.err.(*NumError)
+ errAtoi = &NumError{"Atoi", numErr.Num, numErr.Err}
+ }
+ if test.out != int64(out) || !reflect.DeepEqual(errAtoi, err) {
+ t.Errorf("Atoi(%q) = %v, %v want %v, %v",
+ test.in, out, err, test.out, errAtoi)
+ }
+ }
+ }
+}
+
func TestNumError(t *testing.T) {
for _, test := range numErrorTests {
err := &NumError{
@@ -339,25 +373,33 @@
}
func BenchmarkAtoi(b *testing.B) {
- for i := 0; i < b.N; i++ {
- ParseInt("12345678", 10, 0)
+ n := 1
+ sink := 0
+ for i := 0; i < 10; i++ {
+ s := fmt.Sprintf("%d", n)
+ b.Run(s, func(b *testing.B) {
+ for j := 0; j < b.N; j++ {
+ out, _ := Atoi(s)
+ sink += out
+ }
+ BenchSink += sink
+ })
+ n = n*10 + (i+2)%10
}
}
func BenchmarkAtoiNeg(b *testing.B) {
- for i := 0; i < b.N; i++ {
- ParseInt("-12345678", 10, 0)
- }
-}
-
-func BenchmarkAtoi64(b *testing.B) {
- for i := 0; i < b.N; i++ {
- ParseInt("12345678901234", 10, 64)
- }
-}
-
-func BenchmarkAtoi64Neg(b *testing.B) {
- for i := 0; i < b.N; i++ {
- ParseInt("-12345678901234", 10, 64)
+ n := 1
+ sink := 0
+ for i := 0; i < 10; i++ {
+ s := fmt.Sprintf("%d", -n)
+ b.Run(s[1:], func(b *testing.B) {
+ for j := 0; j < b.N; j++ {
+ out, _ := Atoi(s)
+ sink += out
+ }
+ BenchSink += sink
+ })
+ n = n*10 + (i+2)%10
}
}
To view, visit change 44692. To unsubscribe, visit settings.
Ian Lance Taylor (Gerrit)
Ian Lance Taylor posted comments on this change.
Patch set 1:
R=go1.10
Keith Randall (Gerrit)
Keith Randall posted comments on this change.
Patch set 1:Code-Review +2
Wait until 1.10 opens to submit.
R=go1.10
(2 comments)
File src/strconv/atoi_test.go:
You should keep these benchmarks. Perhaps rename them to Atoi->ParseInt?
Patch Set #1, Line 381:
for j := 0; j < b.N; j++ {I would put the sink := 0 here, no point in capturing that variable with a closure.
To view, visit change 44692. To unsubscribe, visit settings.
Martin Möhrmann (Gerrit)
Martin Möhrmann posted comments on this change.
Patch set 1:
(5 comments)
Patch Set #1, Line 202:
s[0] != '+'since we creating branches here anyway could we do something like (not tested):
var neg bool
if len(s) > 0 && (intSize == 32 && len(s) < 10 || intSize == 64 && len(s) < 19)
switch s[0] {
case '-':
neg = true
case '+':
s = s[1:]
default
}
....
if neg {
n = -n
}
}if the switch is not performant we can try if else.
to capture negative cases in the fast path?
Patch Set #1, Line 203:
(intSize == 32 && len(s) < 10i think we can fold the length checks for lower and upper bound into one comparison:
(intSize == 32 && uint(len(s)-1) < 9 || intSize == 64 && uint(len(s)-1) < 18)and add a comment that this is (intSize == 32 && 0 < len(s) && len(s) < 10 || ...)
Josh put an optimization for it in the compiler not sure if it is covered in this case.
If so we should use the spelled out variant in the code.Patch Set #1, Line 206:
for i := 0; i < len(s); i++ {not tested if it is overall slower code but this range doesnt make a string copy and reads more compact:
for ch := range []byte(s) {
ch = ch - '0'
...
File src/strconv/atoi_test.go:
Patch Set #1, Line 385:
BenchSink += sinkwe could remove sink and always add directly do BenchSink += out above
Patch Set #1, Line 391:
BenchmarkAtoiNegCan we do subbenchmarks Atoi/Pos/ Atoi/Neg? This way there is also no confusion with the Atoi bechmark having changed.
To view, visit change 44692. To unsubscribe, visit settings.
Martin Möhrmann (Gerrit)
tested the below is maybe 0.3ns slower for me on positive numbers vs current CL but brings the fast path speedup also to negative numbers.
// fast path
if intSize == 32 && uint(len(s)-1) < 9 || intSize == 64 && uint(len(s)-1) < 18 {
s0 := s
if s[0] == '+' || s[0] == '-' {
s = s[1:]
if len(s) < 1 {
return 0, &NumError{"Atoi", s0, ErrSyntax}
}
}
var n int
for _, ch := range []byte(s) {
ch -= '0'
if ch > 9 {
return 0, &NumError{"Atoi", s0, ErrSyntax}
}
n = n*10 + int(ch)
}
if s0[0] == '-' {
n = -n
}return n, nil
}
Martin Möhrmann (Gerrit)
One last note while having just run the benchmarks:
I dont think having benchmarks for many different lengths of numbers adds much here. One benchmark for Atoi/Pos with a long number and that same number with '-' for Atoi/Neg would be sufficient for performance testing. I think adding more benchmarks than that just makes pkg wide benchmarking longer with little gain.
Aliaksandr Valialkin (Gerrit)
Aliaksandr Valialkin uploaded patch set #2 to this change.
strconv: optimize Atoi for common case
Benchmark results on GOOS=linux:
GOARCH=amd64
name old time/op new time/op delta
Atoi/Pos/7bit-4 19.3ns ± 1% 8.4ns ± 1% -56.28% (p=0.000 n=10+10)
Atoi/Pos/29bit-4 25.8ns ± 1% 12.5ns ± 3% -51.47% (p=0.000 n=9+10)
Atoi/Pos/31bit-4 27.3ns ± 2% 13.1ns ± 1% -52.05% (p=0.000 n=10+10)
Atoi/Pos/56bit-4 36.0ns ± 1% 18.1ns ± 0% -49.79% (p=0.000 n=10+9)
Atoi/Pos/63bit-4 41.4ns ± 9% 39.0ns ± 1% -5.85% (p=0.000 n=10+10)
Atoi/Neg/7bit-4 17.8ns ± 2% 7.5ns ± 1% -57.79% (p=0.000 n=10+10)
Atoi/Neg/29bit-4 25.7ns ± 2% 13.0ns ± 1% -49.28% (p=0.000 n=10+10)
Atoi/Neg/31bit-4 27.2ns ± 1% 13.9ns ± 0% -48.84% (p=0.000 n=10+7)
Atoi/Neg/56bit-4 36.0ns ± 1% 19.3ns ± 0% -46.55% (p=0.000 n=10+8)
Atoi/Neg/63bit-4 38.6ns ± 0% 38.8ns ± 1% ~ (p=0.267 n=10+10)
GOARCH=386
name old time/op new time/op delta
Atoi/Pos/7bit-4 87.2ns ± 1% 7.5ns ± 1% -91.37% (p=0.000 n=9+9)
Atoi/Pos/29bit-4 208ns ± 0% 13ns ± 1% -93.71% (p=0.000 n=10+10)
Atoi/Pos/31bit-4 229ns ± 2% 228ns ± 0% -0.57% (p=0.048 n=10+8)
Atoi/Neg/7bit-4 86.4ns ± 1% 7.9ns ± 1% -90.86% (p=0.000 n=9+9)
Atoi/Neg/29bit-4 207ns ± 0% 207ns ± 1% ~ (p=0.173 n=10+9)
Atoi/Neg/31bit-4 227ns ± 0% 228ns ± 1% +0.64% (p=0.000 n=6+9)
Fixes #20557
Change-Id: Ib6245d88cffd4b037419e2bf8e4a71b86c6d773f
---
M src/strconv/atoi.go
M src/strconv/atoi_test.go
2 files changed, 92 insertions(+), 18 deletions(-)
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Aliaksandr Valialkin (Gerrit)
Aliaksandr Valialkin uploaded patch set #4 to this change.
strconv: optimize Atoi for common case
Benchmark results on GOOS=linux:
GOARCH=amd64
name old time/op new time/op delta
Atoi/Pos/7bit-4 19.3ns ± 1% 8.4ns ± 1% -56.28% (p=0.000 n=10+10)
Atoi/Pos/29bit-4 25.8ns ± 1% 12.5ns ± 3% -51.47% (p=0.000 n=9+10)
Atoi/Pos/31bit-4 27.3ns ± 2% 13.1ns ± 1% -52.05% (p=0.000 n=10+10)
Atoi/Pos/56bit-4 36.0ns ± 1% 18.1ns ± 0% -49.79% (p=0.000 n=10+9)
Atoi/Pos/63bit-4 41.4ns ± 9% 39.0ns ± 1% -5.85% (p=0.000 n=10+10)
Atoi/Neg/7bit-4 17.8ns ± 2% 7.5ns ± 1% -57.79% (p=0.000 n=10+10)
Atoi/Neg/29bit-4 25.7ns ± 2% 13.0ns ± 1% -49.28% (p=0.000 n=10+10)
Atoi/Neg/31bit-4 27.2ns ± 1% 13.9ns ± 0% -48.84% (p=0.000 n=10+7)
Atoi/Neg/56bit-4 36.0ns ± 1% 19.3ns ± 0% -46.55% (p=0.000 n=10+8)
Atoi/Neg/63bit-4 38.6ns ± 0% 38.8ns ± 1% ~ (p=0.267 n=10+10)
GOARCH=386
name old time/op new time/op delta
Atoi/Pos/7bit-4 87.2ns ± 1% 7.5ns ± 1% -91.37% (p=0.000 n=9+9)
Atoi/Pos/29bit-4 208ns ± 0% 13ns ± 1% -93.71% (p=0.000 n=10+10)
Atoi/Pos/31bit-4 229ns ± 2% 228ns ± 0% -0.57% (p=0.048 n=10+8)
Atoi/Neg/7bit-4 86.4ns ± 1% 7.9ns ± 1% -90.86% (p=0.000 n=9+9)
Atoi/Neg/29bit-4 207ns ± 0% 207ns ± 1% ~ (p=0.173 n=10+9)
Atoi/Neg/31bit-4 227ns ± 0% 228ns ± 1% +0.64% (p=0.000 n=6+9)
Fixes #20557
Change-Id: Ib6245d88cffd4b037419e2bf8e4a71b86c6d773f
---
M src/strconv/atoi.go
M src/strconv/atoi_test.go
2 files changed, 88 insertions(+), 18 deletions(-)
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Aliaksandr Valialkin (Gerrit)
Updated the CL according to comments
7 comments:
since we creating branches here anyway could we do something like (not tested): […]
Done
Patch Set #1, Line 203:
f intSize == 32 && uint(len(si think we can fold the length checks for lower and upper bound into one comparison: […]
Done
Patch Set #1, Line 206:
if s[0] == '-' || s[0] == '+'not tested if it is overall slower code but this range doesnt make a string copy and reads more comp […]
Done
File src/strconv/atoi_test.go:
You should keep these benchmarks. […]
Done
I would put the sink := 0 here, no point in capturing that variable with a closure.
Done
Patch Set #1, Line 385:
, baseErrStub},
we could remove sink and always add directly do BenchSink += out above
Done
Patch Set #1, Line 391:
TestParseIntBitSCan we do subbenchmarks Atoi/Pos/ Atoi/Neg? This way there is also no confusion with the Atoi bechma […]
Done
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Martin Möhrmann (Gerrit)
Do we have seperate Atoi tests?
If not should be good time to add them and use the test tables for ParseInt if we can reuse them (even if we might need to filter some entries).
2 comments:
Patch Set #4, Line 202:
func Atoi(s string) (int, error) {i think we should keep const fnAtoi = "Atoi" for consistency with other functions and replace "Atoi" in the NumErrors.
It should not impact performance.
Patch Set #4, Line 203:
uint(len(s)-1) < 9i think that the compiler is now smart enough to transform:
l := len(s)
(0 < s && s < 10)to the optimized code we use here.
We can check with objdump if thats the case.
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Aliaksandr Valialkin (Gerrit)
Aliaksandr Valialkin uploaded patch set #5 to this change.
strconv: optimize Atoi for common case
Benchmark results on GOOS=linux:
GOARCH=amd64
name old time/op new time/op delta
Atoi/Pos/7bit-4 19.3ns ± 1% 8.4ns ± 1% -56.28% (p=0.000 n=10+10)
Atoi/Pos/29bit-4 25.8ns ± 1% 12.5ns ± 3% -51.47% (p=0.000 n=9+10)
Atoi/Pos/31bit-4 27.3ns ± 2% 13.1ns ± 1% -52.05% (p=0.000 n=10+10)
Atoi/Pos/56bit-4 36.0ns ± 1% 18.1ns ± 0% -49.79% (p=0.000 n=10+9)
Atoi/Pos/63bit-4 41.4ns ± 9% 39.0ns ± 1% -5.85% (p=0.000 n=10+10)
Atoi/Neg/7bit-4 17.8ns ± 2% 7.5ns ± 1% -57.79% (p=0.000 n=10+10)
Atoi/Neg/29bit-4 25.7ns ± 2% 13.0ns ± 1% -49.28% (p=0.000 n=10+10)
Atoi/Neg/31bit-4 27.2ns ± 1% 13.9ns ± 0% -48.84% (p=0.000 n=10+7)
Atoi/Neg/56bit-4 36.0ns ± 1% 19.3ns ± 0% -46.55% (p=0.000 n=10+8)
Atoi/Neg/63bit-4 38.6ns ± 0% 38.8ns ± 1% ~ (p=0.267 n=10+10)
GOARCH=386
name old time/op new time/op delta
Atoi/Pos/7bit-4 87.2ns ± 1% 7.5ns ± 1% -91.37% (p=0.000 n=9+9)
Atoi/Pos/29bit-4 208ns ± 0% 13ns ± 1% -93.71% (p=0.000 n=10+10)
Atoi/Pos/31bit-4 229ns ± 2% 228ns ± 0% -0.57% (p=0.048 n=10+8)
Atoi/Neg/7bit-4 86.4ns ± 1% 7.9ns ± 1% -90.86% (p=0.000 n=9+9)
Atoi/Neg/29bit-4 207ns ± 0% 207ns ± 1% ~ (p=0.173 n=10+9)
Atoi/Neg/31bit-4 227ns ± 0% 228ns ± 1% +0.64% (p=0.000 n=6+9)
Fixes #20557
Change-Id: Ib6245d88cffd4b037419e2bf8e4a71b86c6d773f
---
M src/strconv/atoi.go
M src/strconv/atoi_test.go
2 files changed, 119 insertions(+), 16 deletions(-)
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Aliaksandr Valialkin (Gerrit)
Aliaksandr Valialkin uploaded patch set #6 to this change.
strconv: optimize Atoi for common case
Benchmark results on GOOS=linux:
GOARCH=amd64
name old time/op new time/op delta
Atoi/Pos/7bit-4 19.3ns ± 1% 8.4ns ± 1% -56.28% (p=0.000 n=10+10)
Atoi/Pos/29bit-4 25.8ns ± 1% 12.5ns ± 3% -51.47% (p=0.000 n=9+10)
Atoi/Pos/31bit-4 27.3ns ± 2% 13.1ns ± 1% -52.05% (p=0.000 n=10+10)
Atoi/Pos/56bit-4 36.0ns ± 1% 18.1ns ± 0% -49.79% (p=0.000 n=10+9)
Atoi/Pos/63bit-4 41.4ns ± 9% 39.0ns ± 1% -5.85% (p=0.000 n=10+10)
Atoi/Neg/7bit-4 17.8ns ± 2% 7.5ns ± 1% -57.79% (p=0.000 n=10+10)
Atoi/Neg/29bit-4 25.7ns ± 2% 13.0ns ± 1% -49.28% (p=0.000 n=10+10)
Atoi/Neg/31bit-4 27.2ns ± 1% 13.9ns ± 0% -48.84% (p=0.000 n=10+7)
Atoi/Neg/56bit-4 36.0ns ± 1% 19.3ns ± 0% -46.55% (p=0.000 n=10+8)
Atoi/Neg/63bit-4 38.6ns ± 0% 38.8ns ± 1% ~ (p=0.267 n=10+10)
GOARCH=386
name old time/op new time/op delta
Atoi/Pos/7bit-4 87.2ns ± 1% 7.5ns ± 1% -91.37% (p=0.000 n=9+9)
Atoi/Pos/29bit-4 208ns ± 0% 13ns ± 1% -93.71% (p=0.000 n=10+10)
Atoi/Pos/31bit-4 229ns ± 2% 228ns ± 0% -0.57% (p=0.048 n=10+8)
Atoi/Neg/7bit-4 86.4ns ± 1% 7.9ns ± 1% -90.86% (p=0.000 n=9+9)
Atoi/Neg/29bit-4 207ns ± 0% 207ns ± 1% ~ (p=0.173 n=10+9)
Atoi/Neg/31bit-4 227ns ± 0% 228ns ± 1% +0.64% (p=0.000 n=6+9)
Fixes #20557
Change-Id: Ib6245d88cffd4b037419e2bf8e4a71b86c6d773f
---
M src/strconv/atoi.go
M src/strconv/atoi_test.go
2 files changed, 121 insertions(+), 16 deletions(-)
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Aliaksandr Valialkin (Gerrit)
Patch Set 4:
(2 comments)
Do we have seperate Atoi tests?
If not should be good time to add them and use the test tables for ParseInt if we can reuse them (even if we might need to filter some entries).
Added missing Atoi tests
2 comments:
Patch Set #4, Line 202:
func Atoi(s string) (int, error) {
i think we should keep const fnAtoi = "Atoi" for consistency with other functions and replace "Atoi" […]
Done
i think that the compiler is now smart enough to transform: […]
Yes, the code is identical except register naming.
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Martin Möhrmann (Gerrit)
thanks!
Can we reconfirm the benchmarks if not done so already?
+Trybots but then looks good to me.
Patch set 6:Run-TryBot +1Code-Review +1
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=af343cfe
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 6:TryBot-Result +1
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Robert Griesemer (Gerrit)
8 comments:
s0 := s[0] and avoid the repeated indexing operations below
nit pick: n := 0
File src/strconv/atoi_test.go:
Patch Set #6, Line 361:
for i := range parseInt32Testsfor _, test := range parseInt32Tests { ...
The copy hardly matters here. No need to make the code more complicated.
Patch Set #6, Line 362:
parseInt32Testsremove
if test.out != int64(out) ...
(int(test.out) may truncate)
Patch Set #6, Line 374:
rangesee above
Patch Set #6, Line 375:
parseInt64Testssee above
Patch Set #6, Line 493:
casesdefine a type TestCase struct {...} ?
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Aliaksandr Valialkin (Gerrit)
Aliaksandr Valialkin uploaded patch set #7 to this change.
strconv: optimize Atoi for common case
Benchmark results on GOOS=linux:
GOARCH=amd64
name old time/op new time/op delta
Atoi/Pos/7bit-4 20.1ns ± 2% 8.6ns ± 1% -57.34% (p=0.000 n=10+10)
Atoi/Pos/26bit-4 25.8ns ± 7% 11.9ns ± 0% -53.91% (p=0.000 n=10+8)
Atoi/Pos/31bit-4 27.3ns ± 2% 13.2ns ± 1% -51.56% (p=0.000 n=10+10)
Atoi/Pos/56bit-4 37.2ns ± 5% 18.2ns ± 1% -51.26% (p=0.000 n=10+10)
Atoi/Pos/63bit-4 38.7ns ± 1% 38.6ns ± 1% ~ (p=0.297 n=9+10)
Atoi/Neg/7bit-4 17.6ns ± 1% 7.2ns ± 0% -59.22% (p=0.000 n=10+10)
Atoi/Neg/26bit-4 24.4ns ± 1% 12.4ns ± 1% -49.28% (p=0.000 n=10+10)
Atoi/Neg/31bit-4 26.9ns ± 0% 14.0ns ± 1% -47.88% (p=0.000 n=7+10)
Atoi/Neg/56bit-4 36.2ns ± 1% 19.5ns ± 0% -46.24% (p=0.000 n=10+9)
Atoi/Neg/63bit-4 38.9ns ± 1% 38.8ns ± 1% ~ (p=0.385 n=9+10)
GOARCH=386
name old time/op new time/op delta
Atoi/Pos/7bit-4 89.6ns ± 1% 8.2ns ± 1% -90.84% (p=0.000 n=9+10)
Atoi/Pos/26bit-4 187ns ± 2% 12ns ± 1% -93.71% (p=0.000 n=10+9)
Atoi/Pos/31bit-4 225ns ± 1% 225ns ± 1% ~ (p=0.995 n=10+10)
Atoi/Neg/7bit-4 86.2ns ± 1% 8.5ns ± 1% -90.14% (p=0.000 n=10+10)
Atoi/Neg/26bit-4 183ns ± 1% 13ns ± 1% -92.77% (p=0.000 n=9+10)
Atoi/Neg/31bit-4 223ns ± 0% 223ns ± 0% ~ (p=0.247 n=8+9)
Fixes #20557
Change-Id: Ib6245d88cffd4b037419e2bf8e4a71b86c6d773f
---
M src/strconv/atoi.go
M src/strconv/atoi_test.go
2 files changed, 117 insertions(+), 16 deletions(-)
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Aliaksandr Valialkin (Gerrit)
Patch Set 6:
(8 comments)
Updated the CL according to comments
8 comments:
s0 := s[0] and avoid the repeated indexing operations below
s0 is used for constructing NumError below on lines 213 and 221, so the proposed change will make these lines uglier.
nit pick: n := 0
Patch Set #6, Line 361:
for i := range parseInt32Tests
for _, test := range parseInt32Tests { ... […]
I just copy-pasted TestParseInt, which uses the same for loop.
It would be better to make the proposed change for all the tests in this file in the follow-up CL for the sake of consistency.
Patch Set #6, Line 362:
parseInt32Testsremove
See the reply above
if test.out != int64(out) ... […]
int(test.out) cannot truncate since it has type int32 - see parseInt32Test struct definition above.
Patch Set #6, Line 374:
rangesee above
Ack
Patch Set #6, Line 375:
parseInt64Testssee above
Ack
Patch Set #6, Line 493:
cases
define a type TestCase struct {... […]
Done
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Robert Griesemer (Gerrit)
Patch set 7:Code-Review +2
4 comments:
s0 is used for constructing NumError below on lines 213 and 221, so the proposed change will make th […]
Ack
File src/strconv/atoi_test.go:
Patch Set #6, Line 361:
for i := range parseInt32Tests
I just copy-pasted TestParseInt, which uses the same for loop. […]
Ack
Patch Set #6, Line 362:
parseInt32TestsSee the reply above
Ack
int(test.out) cannot truncate since it has type int32 - see parseInt32Test struct definition above.
ACK (I looked at the wrong struct for Int64)
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Robert Griesemer (Gerrit)
Robert Griesemer merged this change.
strconv: optimize Atoi for common case
Benchmark results on GOOS=linux:
GOARCH=amd64
name old time/op new time/op delta
Atoi/Pos/7bit-4 20.1ns ± 2% 8.6ns ± 1% -57.34% (p=0.000 n=10+10)
Atoi/Pos/26bit-4 25.8ns ± 7% 11.9ns ± 0% -53.91% (p=0.000 n=10+8)
Atoi/Pos/31bit-4 27.3ns ± 2% 13.2ns ± 1% -51.56% (p=0.000 n=10+10)
Atoi/Pos/56bit-4 37.2ns ± 5% 18.2ns ± 1% -51.26% (p=0.000 n=10+10)
Atoi/Pos/63bit-4 38.7ns ± 1% 38.6ns ± 1% ~ (p=0.297 n=9+10)
Atoi/Neg/7bit-4 17.6ns ± 1% 7.2ns ± 0% -59.22% (p=0.000 n=10+10)
Atoi/Neg/26bit-4 24.4ns ± 1% 12.4ns ± 1% -49.28% (p=0.000 n=10+10)
Atoi/Neg/31bit-4 26.9ns ± 0% 14.0ns ± 1% -47.88% (p=0.000 n=7+10)
Atoi/Neg/56bit-4 36.2ns ± 1% 19.5ns ± 0% -46.24% (p=0.000 n=10+9)
Atoi/Neg/63bit-4 38.9ns ± 1% 38.8ns ± 1% ~ (p=0.385 n=9+10)
GOARCH=386
name old time/op new time/op delta
Atoi/Pos/7bit-4 89.6ns ± 1% 8.2ns ± 1% -90.84% (p=0.000 n=9+10)
Atoi/Pos/26bit-4 187ns ± 2% 12ns ± 1% -93.71% (p=0.000 n=10+9)
Atoi/Pos/31bit-4 225ns ± 1% 225ns ± 1% ~ (p=0.995 n=10+10)
Atoi/Neg/7bit-4 86.2ns ± 1% 8.5ns ± 1% -90.14% (p=0.000 n=10+10)
Atoi/Neg/26bit-4 183ns ± 1% 13ns ± 1% -92.77% (p=0.000 n=9+10)
Atoi/Neg/31bit-4 223ns ± 0% 223ns ± 0% ~ (p=0.247 n=8+9)
Fixes #20557
Change-Id: Ib6245d88cffd4b037419e2bf8e4a71b86c6d773f
Reviewed-on: https://go-review.googlesource.com/44692
Reviewed-by: Robert Griesemer <g...@golang.org>
Reviewed-by: Keith Randall <k...@golang.org>
---
M src/strconv/atoi.go
M src/strconv/atoi_test.go
2 files changed, 117 insertions(+), 16 deletions(-)
diff --git a/src/strconv/atoi.go b/src/strconv/atoi.go
index e1ac427..bebed04 100644
--- a/src/strconv/atoi.go
+++ b/src/strconv/atoi.go
@@ -201,6 +201,34 @@
// Atoi returns the result of ParseInt(s, 10, 0) converted to type int.
func Atoi(s string) (int, error) {
const fnAtoi = "Atoi"
+
+ sLen := len(s)
+ if intSize == 32 && (0 < sLen && sLen < 10) ||
+ intSize == 64 && (0 < sLen && sLen < 19) {
+ // Fast path for small integers that fit int type.
+ s0 := s
+ if s[0] == '-' || s[0] == '+' {
+ s = s[1:]
+ if len(s) < 1 {
+ return 0, &NumError{fnAtoi, s0, ErrSyntax}
+ }
+ }
+
+ n := 0
+ for _, ch := range []byte(s) {
+ ch -= '0'
+ if ch > 9 {
+ return 0, &NumError{fnAtoi, s0, ErrSyntax}
+ }
+ n = n*10 + int(ch)
+ }
+ if s0[0] == '-' {
+ n = -n
+ }
+ return n, nil
+ }
+
+ // Slow path for invalid or big integers.
i64, err := ParseInt(s, 10, 0)
if nerr, ok := err.(*NumError); ok {
nerr.Func = fnAtoi
diff --git a/src/strconv/atoi_test.go b/src/strconv/atoi_test.go
index 94844c7..e2f505a 100644
--- a/src/strconv/atoi_test.go
+++ b/src/strconv/atoi_test.go
@@ -6,6 +6,7 @@
import (
"errors"
+ "fmt"
"reflect"
. "strconv"
"testing"
@@ -354,6 +355,37 @@
}
}
+func TestAtoi(t *testing.T) {
+ switch IntSize {
+ case 32:
+ for i := range parseInt32Tests {
+ test := &parseInt32Tests[i]
+ out, err := Atoi(test.in)
+ var testErr error
+ if test.err != nil {
+ testErr = &NumError{"Atoi", test.in, test.err.(*NumError).Err}
+ }
+ if int(test.out) != out || !reflect.DeepEqual(testErr, err) {
+ t.Errorf("Atoi(%q) = %v, %v want %v, %v",
+ test.in, out, err, test.out, testErr)
+ }
+ }
+ case 64:
+ for i := range parseInt64Tests {
+ test := &parseInt64Tests[i]
+ out, err := Atoi(test.in)
+ var testErr error
+ if test.err != nil {
+ testErr = &NumError{"Atoi", test.in, test.err.(*NumError).Err}
+ }
+ if test.out != int64(out) || !reflect.DeepEqual(testErr, err) {
+ t.Errorf("Atoi(%q) = %v, %v want %v, %v",
+ test.in, out, err, test.out, testErr)
+ }
+ }
+ }
+}
+
func bitSizeErrStub(name string, bitSize int) error {
return BitSizeError(name, "0", bitSize)
}
@@ -448,26 +480,67 @@
}
}
+func BenchmarkParseInt(b *testing.B) {
+ b.Run("Pos", func(b *testing.B) {
+ benchmarkParseInt(b, 1)
+ })
+ b.Run("Neg", func(b *testing.B) {
+ benchmarkParseInt(b, -1)
+ })
+}
+
+type benchCase struct {
+ name string
+ num int64
+}
+
+func benchmarkParseInt(b *testing.B, neg int) {
+ cases := []benchCase{
+ {"7bit", 1<<7 - 1},
+ {"26bit", 1<<26 - 1},
+ {"31bit", 1<<31 - 1},
+ {"56bit", 1<<56 - 1},
+ {"63bit", 1<<63 - 1},
+ }
+ for _, cs := range cases {
+ b.Run(cs.name, func(b *testing.B) {
+ s := fmt.Sprintf("%d", cs.num*int64(neg))
+ for i := 0; i < b.N; i++ {
+ out, _ := ParseInt(s, 10, 64)
+ BenchSink += int(out)
+ }
+ })
+ }
+}
+
func BenchmarkAtoi(b *testing.B) {
- for i := 0; i < b.N; i++ {
- ParseInt("12345678", 10, 0)
- }
+ b.Run("Pos", func(b *testing.B) {
+ benchmarkAtoi(b, 1)
+ })
+ b.Run("Neg", func(b *testing.B) {
+ benchmarkAtoi(b, -1)
+ })
}
-func BenchmarkAtoiNeg(b *testing.B) {
- for i := 0; i < b.N; i++ {
- ParseInt("-12345678", 10, 0)
+func benchmarkAtoi(b *testing.B, neg int) {
+ cases := []benchCase{
+ {"7bit", 1<<7 - 1},
+ {"26bit", 1<<26 - 1},
+ {"31bit", 1<<31 - 1},
}
-}
-
-func BenchmarkAtoi64(b *testing.B) {
- for i := 0; i < b.N; i++ {
- ParseInt("12345678901234", 10, 64)
+ if IntSize == 64 {
+ cases = append(cases, []benchCase{
+ {"56bit", 1<<56 - 1},
+ {"63bit", 1<<63 - 1},
+ }...)
}
-}
-
-func BenchmarkAtoi64Neg(b *testing.B) {
- for i := 0; i < b.N; i++ {
- ParseInt("-12345678901234", 10, 64)
+ for _, cs := range cases {
+ b.Run(cs.name, func(b *testing.B) {
+ s := fmt.Sprintf("%d", cs.num*int64(neg))
+ for i := 0; i < b.N; i++ {
+ out, _ := Atoi(s)
+ BenchSink += out
+ }
+ })
}
}
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Martin Möhrmann (Gerrit)
1 comment:
File src/strconv/atoi_test.go:
Patch Set #6, Line 361:
for i := range parseInt32TestsAck
I made a not yet ready cleanup CL for the tests in strconv where i also changed that. The tests in general could benefit from a bit of consolidation.
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.
Aliaksandr Valialkin (Gerrit)
1 comment:
Patch Set #6, Line 361:
for i := range parseInt32Tests
I made a not yet ready cleanup CL for the tests in strconv where i also changed that. […]
OK, waiting for your cleanup CL then.
To view, visit change 44692. To unsubscribe, or for help writing mail filters, visit settings.