code review 8056043: runtime: Implement faster equals for strings and bytes.

runtime: Implement faster equals for strings and bytes.

(amd64)
benchmark           old ns/op    new ns/op    delta
BenchmarkEqual0            16            6  -63.15%
BenchmarkEqual9            22            7  -65.37%
BenchmarkEqual32           36            9  -74.91%
BenchmarkEqual4K         2187          120  -94.51%

benchmark            old MB/s     new MB/s  speedup
BenchmarkEqual9        392.22      1134.38    2.89x
BenchmarkEqual32       866.72      3457.39    3.99x
BenchmarkEqual4K      1872.73     33998.87   18.15x

(386)
benchmark           old ns/op    new ns/op    delta
BenchmarkEqual0            16            5  -63.85%
BenchmarkEqual9            22            7  -67.84%
BenchmarkEqual32           34           12  -64.94%
BenchmarkEqual4K         2196          113  -94.85%

benchmark            old MB/s     new MB/s  speedup
BenchmarkEqual9        405.81      1260.18    3.11x
BenchmarkEqual32       919.55      2631.21    2.86x
BenchmarkEqual4K      1864.85     36072.54   19.34x

Update issue 3751

[khr, 2013-03-27 22:14:12]

Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://khr%40golang.org@code.google.com/p/go/

[r, 2013-03-27 22:34:05]

run go vet. i think it will ask for a few tweaks to the assembly

[bradfitz, 2013-03-27 22:51:25]

LGTM but I'm not qualified to review runtime/asm_arm.s

On Wed, Mar 27, 2013 at 3:14 PM, <khr@golang.org> wrote:

> Reviewers: bradfitz,
>
> Message:
> Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
>
https://khr%40golang.org@code.**google.com/p/go/<http://40golang.org@code.goo...
>
>
> Description:
> runtime: Implement faster equals for strings and bytes.
>
> (amd64)
> benchmark            old ns/op    new ns/op    delta
> BenchmarkMegEqMap      1164712       103451  -91.12%
>
> (386)
> benchmark            old ns/op    new ns/op    delta
> BenchmarkMegEqMap      4935048       171105  -96.53%
>
> (arm)
> benchmark            old ns/op    new ns/op    delta
> BenchmarkMegEqMap     12018127      8923492  -25.75%
>
> Update issue 3751
>
> Please review this at
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>
> Affected files:
>   M src/pkg/bytes/asm_386.s
>   M src/pkg/bytes/asm_amd64.s
>   M src/pkg/bytes/asm_arm.s
>   M src/pkg/bytes/bytes_test.go
>   M src/pkg/runtime/alg.c
>   M src/pkg/runtime/asm_386.s
>   M src/pkg/runtime/asm_amd64.s
>   M src/pkg/runtime/asm_arm.s
>   M src/pkg/runtime/hashmap.c
>   M src/pkg/runtime/mapspeed_test.**go
>   M src/pkg/runtime/runtime.h
>   M src/pkg/runtime/string.goc
>
>
>

[bradfitz, 2013-03-27 23:35:20]

https://codereview.appspot.com/8056043/diff/11001/src/pkg/bytes/bytes_test.go
File src/pkg/bytes/bytes_test.go (right):

https://codereview.appspot.com/8056043/diff/11001/src/pkg/bytes/bytes_test.go...
src/pkg/bytes/bytes_test.go:64: {[]byte("abcdefgh"), []byte("abcdefgh"), 0},
comment above this line about what these are testing? (alignment)

https://codereview.appspot.com/8056043/diff/11001/src/pkg/runtime/mapspeed_te...
File src/pkg/runtime/mapspeed_test.go (right):

https://codereview.appspot.com/8056043/diff/11001/src/pkg/runtime/mapspeed_te...
src/pkg/runtime/mapspeed_test.go:121: func BenchmarkMegEqMap(b *testing.B) {
it's not clear what "Eq" in the benchmark names here.

"Mostly equal"?  (it ends in a Y)

I'd just make the lookup key be all "X" too.

[khr1, 2013-03-28 02:34:48]

On Wed, Mar 27, 2013 at 4:35 PM, <bradfitz@golang.org> wrote:

>
> https://codereview.appspot.**com/8056043/diff/11001/src/**
>
pkg/bytes/bytes_test.go<https://codereview.appspot.com/8056043/diff/11001/src/pkg/bytes/bytes_test.go>
> File src/pkg/bytes/bytes_test.go (right):
>
> https://codereview.appspot.**com/8056043/diff/11001/src/**
>
pkg/bytes/bytes_test.go#**newcode64<https://codereview.appspot.com/8056043/diff/11001/src/pkg/bytes/bytes_test.go#newcode64>
> src/pkg/bytes/bytes_test.go:**64: {[]byte("abcdefgh"), []byte("abcdefgh"),
> 0},
> comment above this line about what these are testing? (alignment)
>

Done.  Not so much testing alignment as testing around the boundary of the
chunk size of memeq.


>
> https://codereview.appspot.**com/8056043/diff/11001/src/**
>
pkg/runtime/mapspeed_test.go<https://codereview.appspot.com/8056043/diff/11001/src/pkg/runtime/mapspeed_test.go>
> File src/pkg/runtime/mapspeed_test.**go (right):
>
> https://codereview.appspot.**com/8056043/diff/11001/src/**
>
pkg/runtime/mapspeed_test.go#**newcode121<https://codereview.appspot.com/8056043/diff/11001/src/pkg/runtime/mapspeed_test.go#newcode121>
> src/pkg/runtime/mapspeed_test.**go:121: func BenchmarkMegEqMap(b
> *testing.B) {
> it's not clear what "Eq" in the benchmark names here.
>
> "Mostly equal"?  (it ends in a Y)
>
> I'd just make the lookup key be all "X" too.
>

Done.


>
>
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>

[dfc, 2013-03-28 03:17:31]

Thanks. I'll try this on some arm systems soon and get some benchmarks. I'd like
to see data from the bytes package.

[dfc, 2013-03-28 03:26:05]

https://codereview.appspot.com/8056043/diff/18001/src/pkg/bytes/asm_386.s
File src/pkg/bytes/asm_386.s (right):

https://codereview.appspot.com/8056043/diff/18001/src/pkg/bytes/asm_386.s#new...
src/pkg/bytes/asm_386.s:19: TEXT ·Equal(SB),7,$12
I would like to suggest that this function moves into the runtime package (see
the way we do time.now() as an example) so that we have all the moving parts
closer together.

[dfc, 2013-03-28 03:55:42]

Some excellent speedups on arm (chromebook) for Equals, but a
regression on some other benchmarks. A very brief look at the profile
makes me want to blame the additional branch in Index although there
doesn't appear to be any stack splitting.

benchmark                        old ns/op    new ns/op    delta
BenchmarkReadString                 125100       127392   +1.83%
BenchmarkIndexByte32                    76           90  +18.74%
BenchmarkIndexByte4K                  7605         7298   -4.04%
BenchmarkIndexByte4M               7574065      7567863   -0.08%
BenchmarkIndexByte64M            120986945    120754408   -0.19%
BenchmarkIndexBytePortable32           175          175   +0.00%
BenchmarkIndexBytePortable4K         17347        17338   -0.05%
BenchmarkIndexBytePortable4M      18353531     18341798   -0.06%
BenchmarkIndexBytePortable64M    293546342    293211533   -0.11%
BenchmarkEqual32                        77           63  -17.81%
BenchmarkEqual4K                      7289         1855  -74.55%
BenchmarkEqual4M                   8367520      3209180  -61.65%
BenchmarkEqual64M                142515029     61400849  -56.92%
BenchmarkEqualPort32                   224          224   +0.00%
BenchmarkEqualPort4K                 24263        24250   -0.05%
BenchmarkEqualPortable4M          25179603     25247419   +0.27%
BenchmarkEqualPortable64M        403745450    403779625   +0.01%
BenchmarkIndex32                       816         1302  +59.56%
BenchmarkIndex4K                    120982       192998  +59.53%
BenchmarkIndex4M                 124260183    193296808  +55.56%
BenchmarkIndex64M               1988046002   3095359752  +55.70%
BenchmarkIndexEasy32                   144          161  +11.81%
BenchmarkIndexEasy4K                  7352         7369   +0.23%
BenchmarkIndexEasy4M               7578549      7575628   -0.04%
BenchmarkIndexEasy64M            120737458    120832933   +0.08%
BenchmarkCount32                       818         1241  +51.71%
BenchmarkCount4K                    120977       188073  +55.46%
BenchmarkCount4M                 124265360    192719445  +55.09%
BenchmarkCount64M               1988792002   3085427626  +55.14%
BenchmarkCountEasy32                   142          162  +14.08%
BenchmarkCountEasy4K                  7349         7361   +0.16%
BenchmarkCountEasy4M               7574511      7574003   -0.01%
BenchmarkCountEasy64M            120729902    120718660   -0.01%
BenchmarkFields                  114759929    115037127   +0.24%
BenchmarkFieldsFunc              113932977    113976645   +0.04%
BenchmarkTrimSpace                     345          339   -1.74%

benchmark                         old MB/s     new MB/s  speedup
BenchmarkReadString                 261.93       257.22    0.98x
BenchmarkIndexByte32                419.35       353.35    0.84x
BenchmarkIndexByte4K                538.54       561.22    1.04x
BenchmarkIndexByte4M                553.77       554.23    1.00x
BenchmarkIndexByte64M               554.68       555.75    1.00x
BenchmarkIndexBytePortable32        182.72       182.41    1.00x
BenchmarkIndexBytePortable4K        236.11       236.24    1.00x
BenchmarkIndexBytePortable4M        228.53       228.67    1.00x
BenchmarkIndexBytePortable64M       228.61       228.88    1.00x
BenchmarkEqual32                    412.82       502.15    1.22x
BenchmarkEqual4K                    561.90      2207.99    3.93x
BenchmarkEqual4M                    501.26      1306.97    2.61x
BenchmarkEqual64M                   470.89      1092.96    2.32x
BenchmarkEqualPort32                142.66       142.81    1.00x
BenchmarkEqualPort4K                168.82       168.91    1.00x
BenchmarkEqualPortable4M            166.58       166.13    1.00x
BenchmarkEqualPortable64M           166.22       166.20    1.00x
BenchmarkIndex32                     39.19        24.56    0.63x
BenchmarkIndex4K                     33.86        21.22    0.63x
BenchmarkIndex4M                     33.75        21.70    0.64x
BenchmarkIndex64M                    33.76        21.68    0.64x
BenchmarkIndexEasy32                221.74       198.25    0.89x
BenchmarkIndexEasy4K                557.11       555.79    1.00x
BenchmarkIndexEasy4M                553.44       553.66    1.00x
BenchmarkIndexEasy64M               555.82       555.39    1.00x
BenchmarkCount32                     39.08        25.78    0.66x
BenchmarkCount4K                     33.86        21.78    0.64x
BenchmarkCount4M                     33.75        21.76    0.64x
BenchmarkCount64M                    33.74        21.75    0.64x
BenchmarkCountEasy32                225.27       196.43    0.87x
BenchmarkCountEasy4K                557.32       556.37    1.00x
BenchmarkCountEasy4M                553.74       553.78    1.00x
BenchmarkCountEasy64M               555.86       555.91    1.00x
BenchmarkFields                       9.14         9.12    1.00x
BenchmarkFieldsFunc                   9.20         9.20    1.00x

[khr1, 2013-03-28 04:25:42]

It looks like both the slow Index and Count ones are calling eq with 7-byte
slices.  For some reason that parameter makes it slow.  I'll see what I can
do to fix that, 7-byte eq is probably pretty common.


On Wed, Mar 27, 2013 at 8:55 PM, Dave Cheney <dave@cheney.net> wrote:

> Some excellent speedups on arm (chromebook) for Equals, but a
> regression on some other benchmarks. A very brief look at the profile
> makes me want to blame the additional branch in Index although there
> doesn't appear to be any stack splitting.
>
> benchmark                        old ns/op    new ns/op    delta
> BenchmarkReadString                 125100       127392   +1.83%
> BenchmarkIndexByte32                    76           90  +18.74%
> BenchmarkIndexByte4K                  7605         7298   -4.04%
> BenchmarkIndexByte4M               7574065      7567863   -0.08%
> BenchmarkIndexByte64M            120986945    120754408   -0.19%
> BenchmarkIndexBytePortable32           175          175   +0.00%
> BenchmarkIndexBytePortable4K         17347        17338   -0.05%
> BenchmarkIndexBytePortable4M      18353531     18341798   -0.06%
> BenchmarkIndexBytePortable64M    293546342    293211533   -0.11%
> BenchmarkEqual32                        77           63  -17.81%
> BenchmarkEqual4K                      7289         1855  -74.55%
> BenchmarkEqual4M                   8367520      3209180  -61.65%
> BenchmarkEqual64M                142515029     61400849  -56.92%
> BenchmarkEqualPort32                   224          224   +0.00%
> BenchmarkEqualPort4K                 24263        24250   -0.05%
> BenchmarkEqualPortable4M          25179603     25247419   +0.27%
> BenchmarkEqualPortable64M        403745450    403779625   +0.01%
> BenchmarkIndex32                       816         1302  +59.56%
> BenchmarkIndex4K                    120982       192998  +59.53%
> BenchmarkIndex4M                 124260183    193296808  +55.56%
> BenchmarkIndex64M               1988046002   3095359752  +55.70%
> BenchmarkIndexEasy32                   144          161  +11.81%
> BenchmarkIndexEasy4K                  7352         7369   +0.23%
> BenchmarkIndexEasy4M               7578549      7575628   -0.04%
> BenchmarkIndexEasy64M            120737458    120832933   +0.08%
> BenchmarkCount32                       818         1241  +51.71%
> BenchmarkCount4K                    120977       188073  +55.46%
> BenchmarkCount4M                 124265360    192719445  +55.09%
> BenchmarkCount64M               1988792002   3085427626  +55.14%
> BenchmarkCountEasy32                   142          162  +14.08%
> BenchmarkCountEasy4K                  7349         7361   +0.16%
> BenchmarkCountEasy4M               7574511      7574003   -0.01%
> BenchmarkCountEasy64M            120729902    120718660   -0.01%
> BenchmarkFields                  114759929    115037127   +0.24%
> BenchmarkFieldsFunc              113932977    113976645   +0.04%
> BenchmarkTrimSpace                     345          339   -1.74%
>
> benchmark                         old MB/s     new MB/s  speedup
> BenchmarkReadString                 261.93       257.22    0.98x
> BenchmarkIndexByte32                419.35       353.35    0.84x
> BenchmarkIndexByte4K                538.54       561.22    1.04x
> BenchmarkIndexByte4M                553.77       554.23    1.00x
> BenchmarkIndexByte64M               554.68       555.75    1.00x
> BenchmarkIndexBytePortable32        182.72       182.41    1.00x
> BenchmarkIndexBytePortable4K        236.11       236.24    1.00x
> BenchmarkIndexBytePortable4M        228.53       228.67    1.00x
> BenchmarkIndexBytePortable64M       228.61       228.88    1.00x
> BenchmarkEqual32                    412.82       502.15    1.22x
> BenchmarkEqual4K                    561.90      2207.99    3.93x
> BenchmarkEqual4M                    501.26      1306.97    2.61x
> BenchmarkEqual64M                   470.89      1092.96    2.32x
> BenchmarkEqualPort32                142.66       142.81    1.00x
> BenchmarkEqualPort4K                168.82       168.91    1.00x
> BenchmarkEqualPortable4M            166.58       166.13    1.00x
> BenchmarkEqualPortable64M           166.22       166.20    1.00x
> BenchmarkIndex32                     39.19        24.56    0.63x
> BenchmarkIndex4K                     33.86        21.22    0.63x
> BenchmarkIndex4M                     33.75        21.70    0.64x
> BenchmarkIndex64M                    33.76        21.68    0.64x
> BenchmarkIndexEasy32                221.74       198.25    0.89x
> BenchmarkIndexEasy4K                557.11       555.79    1.00x
> BenchmarkIndexEasy4M                553.44       553.66    1.00x
> BenchmarkIndexEasy64M               555.82       555.39    1.00x
> BenchmarkCount32                     39.08        25.78    0.66x
> BenchmarkCount4K                     33.86        21.78    0.64x
> BenchmarkCount4M                     33.75        21.76    0.64x
> BenchmarkCount64M                    33.74        21.75    0.64x
> BenchmarkCountEasy32                225.27       196.43    0.87x
> BenchmarkCountEasy4K                557.32       556.37    1.00x
> BenchmarkCountEasy4M                553.74       553.78    1.00x
> BenchmarkCountEasy64M               555.86       555.91    1.00x
> BenchmarkFields                       9.14         9.12    1.00x
> BenchmarkFieldsFunc                   9.20         9.20    1.00x
>

[remyoudompheng, 2013-03-28 07:31:24]

I have added the following benchmarks to runtime/string_test.go:

func BenchmarkCompareStringBigUnaligned(b *testing.B) {
	bytes := make([]byte, 0, 1<<20)
	for len(bytes) < 1<<20 {
		bytes = append(bytes, "Hello Gophers!"...)
	}
	s1, s2 := string(bytes), "hello"+string(bytes)
	for i := 0; i < b.N; i++ {
		if s1 != s2[len("hello"):] {
			b.Fatal("s1 != s2")
		}
	}
      b.SetBytes(int64(len(s1)))
}

func BenchmarkCompareStringBig(b *testing.B) {
	bytes := make([]byte, 0, 1<<20)
	for len(bytes) < 1<<20 {
		bytes = append(bytes, "Hello Gophers!"...)
	}
	s1, s2 := string(bytes), string(bytes)
	for i := 0; i < b.N; i++ {
		if s1 != s2 {
			b.Fatal("s1 != s2")
		}
	}
      b.SetBytes(int64(len(s1)))
}

And obtained the following results on ARM (ODROID-X)

benchmark                             old ns/op    new ns/op    delta
BenchmarkCompareStringBigUnaligned     11285388      2904806  -74.26%
BenchmarkCompareStringBig              10686704      1807713  -83.08%

benchmark                              old MB/s     new MB/s  speedup
BenchmarkCompareStringBigUnaligned        92.92       360.98    3.88x
BenchmarkCompareStringBig                 98.12       580.06    5.91x

I could squeeze a bit more (20%) on ARM by using the following form for the
unaligned case (Dave, do you want to run your benchmarks on this?):

	// B is not aligned.  Compare the word at A
	// with the value constructed from the
	// two words near B.
	// Note: this code is little-endian specific.
        // Beware that R9, R10 are reserved for m, g.
        // R11 can be used by linker.
	MOVW	(R4), R7
one_aligned:
	CMP	$4, R3
	BLO	tail
	MOVW	R7>>R5, R12
	MOVW.W	4(R4), R7
	ORR	R7<<R6, R12
	MOVW.P	4(R1), R8
	ADD	$4, R2
	SUB	$4, R3
	CMP	R8, R12
	BEQ	one_aligned
	MOVW	$0, R0
	RET

[remyoudompheng, 2013-03-28 07:43:58]

https://codereview.appspot.com/8056043/diff/18001/src/pkg/bytes/bytes_test.go
File src/pkg/bytes/bytes_test.go (right):

https://codereview.appspot.com/8056043/diff/18001/src/pkg/bytes/bytes_test.go...
src/pkg/bytes/bytes_test.go:92: 
I would have added a test like this:

func TestEqual(t *testing.T) {
      b1 := make([]byte, 64)
      b2 := make([]byte, 64)
      for l := 0; l <= 56; l++ {
            for off1 := 0; off1 < 8; off1++ {
                  for off2 := 0; off2 < 8; off2++ {
                        for diffoff := 0; diffoff <= l; diffoff++ {
                              for i := range b1 {
                                    b1[i], b2[i] = 0, 0
                              }
                              for i := 0; i < l; i++ {
                                    b1[off1+i], b2[off2+i] = byte(i), byte(i)
                              }
                              b1[off1+diffoff], b2[off2+diffoff] = 'x', 'y'
                              cmp := Equal(b1[off1:off1+l], b2[off2:off2+l])
                              if cmp != (diffoff == l) {
                                    t.Errorf("Equal(%q, %q) = %v",
b1[off1:off1+l], b2[off2:off2+l], cmp)
                              }
                        }
                  }
            }
      }
}

[remyoudompheng, 2013-03-28 07:51:10]

On 2013/03/28 07:31:24, remyoudompheng wrote:
> I could squeeze a bit more (20%) on ARM by using the following form for the
> unaligned case (Dave, do you want to run your benchmarks on this?):
> 
> 	// B is not aligned.  Compare the word at A
> 	// with the value constructed from the
> 	// two words near B.
> 	// Note: this code is little-endian specific.
>         // Beware that R9, R10 are reserved for m, g.
>         // R11 can be used by linker.
> 	MOVW	(R4), R7
> one_aligned:
> 	CMP	$4, R3
> 	BLO	tail
> 	MOVW	R7>>R5, R12
> 	MOVW.W	4(R4), R7
> 	ORR	R7<<R6, R12
> 	MOVW.P	4(R1), R8
> 	ADD	$4, R2
> 	SUB	$4, R3
> 	CMP	R8, R12
> 	BEQ	one_aligned
> 	MOVW	$0, R0
> 	RET

More precisely, I obtained this result compared to the original proposal:

benchmark                             old ns/op    new ns/op    delta
BenchmarkCompareStringBigUnaligned      2894508      2447519  -15.44%
BenchmarkCompareStringBig               1802048      1811376   +0.52%

benchmark                              old MB/s     new MB/s  speedup
BenchmarkCompareStringBigUnaligned       362.27       428.43    1.18x
BenchmarkCompareStringBig                581.89       578.89    0.99x

[khr1, 2013-03-29 07:06:48]

Dave:
I've moved bytes.Equal to the runtime files.  Good idea, that puts all that
code in one place.
Try your benchmarks again, I've special-cased <8 byte comparisons, they
should be only slightly slower than before.

Remy:
I've included your test and your optimization, thanks.
Did you check in your benchmarks, or should I include them in my patch?

Brad:
I've changed the amd64/386 code quite a bit, seems that REP;CMPSx has a
significant startup cost and we're better off just explicitly looping.


On Thu, Mar 28, 2013 at 12:51 AM, <remyoudompheng@gmail.com> wrote:

> On 2013/03/28 07:31:24, remyoudompheng wrote:
>
>> I could squeeze a bit more (20%) on ARM by using the following form
>>
> for the
>
>> unaligned case (Dave, do you want to run your benchmarks on this?):
>>
>
>          // B is not aligned.  Compare the word at A
>>         // with the value constructed from the
>>         // two words near B.
>>         // Note: this code is little-endian specific.
>>          // Beware that R9, R10 are reserved for m, g.
>>          // R11 can be used by linker.
>>         MOVW    (R4), R7
>> one_aligned:
>>         CMP     $4, R3
>>         BLO     tail
>>         MOVW    R7>>R5, R12
>>         MOVW.W  4(R4), R7
>>         ORR     R7<<R6, R12
>>         MOVW.P  4(R1), R8
>>         ADD     $4, R2
>>         SUB     $4, R3
>>         CMP     R8, R12
>>         BEQ     one_aligned
>>         MOVW    $0, R0
>>         RET
>>
>
> More precisely, I obtained this result compared to the original
> proposal:
>
>
> benchmark                             old ns/op    new ns/op    delta
> BenchmarkCompareStringBigUnali**gned      2894508      2447519  -15.44%
> BenchmarkCompareStringBig               1802048      1811376   +0.52%
>
>
> benchmark                              old MB/s     new MB/s  speedup
> BenchmarkCompareStringBigUnali**gned       362.27       428.43    1.18x
> BenchmarkCompareStringBig                581.89       578.89    0.99x
>
>
>
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>

[bradfitz, 2013-03-29 07:26:09]

LGTM

Assembly people should chime in, though.

https://codereview.appspot.com/8056043/diff/39001/src/pkg/bytes/bytes_test.go
File src/pkg/bytes/bytes_test.go (right):

https://codereview.appspot.com/8056043/diff/39001/src/pkg/bytes/bytes_test.go...
src/pkg/bytes/bytes_test.go:335: buf := make([]byte, 4)
or:
var buf [4]byte

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/asm_386.s
File src/pkg/runtime/asm_386.s (right):

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/asm_386.s#n...
src/pkg/runtime/asm_386.s:996: TEXT bytes·Equal(SB),7,$0
This seems like new precedent. At least leave a comment above "func
equalPortable" in pkg/bytes/bytes.go directing people where to find Equal.

Like:

// func Equal is implemented in assembly in pkg/runtime for
// amd64, 386, and arm.

Then grepping for "func Equal" finds it.

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/mapspeed_te...
File src/pkg/runtime/mapspeed_test.go (right):

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/mapspeed_te...
src/pkg/runtime/mapspeed_test.go:124: key := strings.Repeat("X", 1<<20)
move this up a line and just say m[key] = true

[remyoudompheng, 2013-03-29 07:34:57]

On 2013/03/29 07:06:48, khr1 wrote:
> Dave:
> I've moved bytes.Equal to the runtime files.  Good idea, that puts all that
> code in one place.
> Try your benchmarks again, I've special-cased <8 byte comparisons, they
> should be only slightly slower than before.
> 
> Remy:
> I've included your test and your optimization, thanks.
> Did you check in your benchmarks, or should I include them in my patch?

Yes, please include my string_test.go proposals if you wish.
Did you update the benchmarks in issue description?

What is the speed difference between REP MOVSQ and the SSE2 way on amd64?

[khr1, 2013-03-29 07:38:26]

On Fri, Mar 29, 2013 at 12:26 AM, <bradfitz@golang.org> wrote:

> LGTM
>
> Assembly people should chime in, though.
>
>
>
> https://codereview.appspot.**com/8056043/diff/39001/src/**
>
pkg/bytes/bytes_test.go<https://codereview.appspot.com/8056043/diff/39001/src/pkg/bytes/bytes_test.go>
> File src/pkg/bytes/bytes_test.go (right):
>
> https://codereview.appspot.**com/8056043/diff/39001/src/**
>
pkg/bytes/bytes_test.go#**newcode335<https://codereview.appspot.com/8056043/diff/39001/src/pkg/bytes/bytes_test.go#newcode335>
> src/pkg/bytes/bytes_test.go:**335: buf := make([]byte, 4)
> or:
> var buf [4]byte
>
> https://codereview.appspot.**com/8056043/diff/39001/src/**
>
pkg/runtime/asm_386.s<https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/asm_386.s>
> File src/pkg/runtime/asm_386.s (right):
>
> https://codereview.appspot.**com/8056043/diff/39001/src/**
>
pkg/runtime/asm_386.s#**newcode996<https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/asm_386.s#newcode996>
> src/pkg/runtime/asm_386.s:996: TEXT bytes·Equal(SB),7,$0
> This seems like new precedent. At least leave a comment above "func
> equalPortable" in pkg/bytes/bytes.go directing people where to find
> Equal.
>
> Like:
>
> // func Equal is implemented in assembly in pkg/runtime for
> // amd64, 386, and arm.
>
> Then grepping for "func Equal" finds it.
>
> https://codereview.appspot.**com/8056043/diff/39001/src/**
>
pkg/runtime/mapspeed_test.go<https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/mapspeed_test.go>
> File src/pkg/runtime/mapspeed_test.**go (right):
>
> https://codereview.appspot.**com/8056043/diff/39001/src/**
>
pkg/runtime/mapspeed_test.go#**newcode124<https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/mapspeed_test.go#newcode124>
> src/pkg/runtime/mapspeed_test.**go:124: key := strings.Repeat("X", 1<<20)
> move this up a line and just say m[key] = true
>

I want two different strings so the memory comparison has to happen.  With
your change, it could (and does) detect equality by just comparing pointers.


>
>
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>

[remyoudompheng, 2013-03-29 07:38:39]

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/asm_386.s
File src/pkg/runtime/asm_386.s (right):

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/asm_386.s#n...
src/pkg/runtime/asm_386.s:1027: PCMPEQB	X1, X0
Do we explicitly require SSE2 on 386 now?
Maybe we should document what is required by the runtime code (this instruction
for atomics, that one for blah...).

I don't think SSE2 is a requirement for Go 1.1, but I don't remember exactly. I
would adda CPUID check and switch to the REP MOVSL way if no SSE2 is avaiable.

[khr1, 2013-03-29 07:53:58]

The SSE way is about 2x as fast, at least when the strings are intermediate
length.  For really long strings there isn't much difference because it's
limited by memory bandwidth.

Those SSE2 instructions are already being used in
bytes/asm_amd64.s:IndexByte, so they are OK for amd64 at least.  I'll ask
around about 386.

I'll update the benchmark numbers tomorrow.


On Fri, Mar 29, 2013 at 12:34 AM, <remyoudompheng@gmail.com> wrote:

> On 2013/03/29 07:06:48, khr1 wrote:
>
>> Dave:
>> I've moved bytes.Equal to the runtime files.  Good idea, that puts all
>>
> that
>
>> code in one place.
>> Try your benchmarks again, I've special-cased <8 byte comparisons,
>>
> they
>
>> should be only slightly slower than before.
>>
>
>  Remy:
>> I've included your test and your optimization, thanks.
>> Did you check in your benchmarks, or should I include them in my
>>
> patch?
>
> Yes, please include my string_test.go proposals if you wish.
> Did you update the benchmarks in issue description?
>
> What is the speed difference between REP MOVSQ and the SSE2 way on
> amd64?
>
>
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>

[dfc, 2013-03-29 09:03:03]

Thanks Keith, I'll get to benchmarking.

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/asm_386.s
File src/pkg/runtime/asm_386.s (right):

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/asm_386.s#n...
src/pkg/runtime/asm_386.s:996: TEXT bytes·Equal(SB),7,$0
On 2013/03/29 07:26:09, bradfitz wrote:
> This seems like new precedent. At least leave a comment above "func
> equalPortable" in pkg/bytes/bytes.go directing people where to find Equal.
> 
> Like:
> 
> // func Equal is implemented in assembly in pkg/runtime for
> // amd64, 386, and arm.
> 
> Then grepping for "func Equal" finds it.

This was my suggestion. I believe the are precedents from the time package. It
does deserve a comment.

[albert.strasheim, 2013-03-29 14:25:00]

Looks very good compared to glibc 4.7.2 on a Core i7-3720QM:

BenchmarkRuntimeMemequal1K 25993.11 MB/s
BenchmarkRuntimeMemequal1M 15857.93 MB/s [2]
BenchmarkRuntimeMemequal1G 9783.48 MB/s
BenchmarkStringEqual1M     16991.31 MB/s

vs

BenchmarkMemcmp1K  12672.80 MB/s [1]
BenchmarkMemcmp1M  20434.50 MB/s [2]
BenchmarkMemcmp1G  9518.44 MB/s
BenchmarkStrncmp1M 15551.37 MB/s

[1] BenchmarkMemcmp1K has some cgo overhead.

[2] This difference might be worth a look:

looking at bytes.Equal vs memcmp:

BenchmarkBytesEqual1K   29738.67 MB/s
BenchmarkBytesEqual32K  22170.51 MB/s
BenchmarkBytesEqual64K  22458.35 MB/s
BenchmarkBytesEqual128K 21313.28 MB/s
BenchmarkBytesEqual256K	16965.64 MB/s
BenchmarkBytesEqual512K	16581.24 MB/s
BenchmarkBytesEqual1M	16706.55 MB/s

BenchmarkMemcmp32Kb     23958.85 MB/s
BenchmarkMemcmp64K      26175.43 MB/s
BenchmarkMemcmp128K     27440.97 MB/s
BenchmarkMemcmp256K     25026.15 MB/s
BenchmarkMemcmp512K     22581.35 MB/s
BenchmarkMemcmp1M       19161.69 MB/s
BenchmarkMemcmp2M       16326.83 MB/s

The Go functions have some kind of crossover point at around 256K which only
kicks in at around 2M in glibc.

code here: github.com/alberts/gofrags

[minux1, 2013-03-29 14:44:25]

On Fri, Mar 29, 2013 at 3:53 PM, Keith Randall <khr@google.com> wrote:
> The SSE way is about 2x as fast, at least when the strings are intermediate
length.  For really long strings there isn't much difference because it's
limited by memory bandwidth.
> Those SSE2 instructions are already being used in bytes/asm_amd64.s:IndexByte,
so they are OK for amd64 at least.  I'll ask around about 386.
please use cpuid to detect sse2 on 386 as we still support as old as
pentium mmx chips.
sse2 is required feature for amd64 so using it is ok.

[bradfitz, 2013-03-29 15:44:03]

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/mapspeed_te...
File src/pkg/runtime/mapspeed_test.go (right):

https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/mapspeed_te...
src/pkg/runtime/mapspeed_test.go:124: key := strings.Repeat("X", 1<<20)
On 2013/03/29 07:26:09, bradfitz wrote:
> move this up a line and just say m[key] = true

Oh, you probably did it this way so their .str fields were different, to avoid
that fastpath.  Leave a comment?

key1 := strings.Repeat("X", 1<<20)
key2 := strings.Repeat("X", 1<<20) // equal but different instance

[khr1, 2013-03-29 17:16:39]

> This was my suggestion. I believe the are precedents from the time
> package. It does deserve a comment.

There is already a declaration + comment for Equal in bytes_decl.go.
I've updated the comment to point to runtime.

> please use cpuid to detect sse2 on 386 as we still support as old as
> pentium mmx chips.

Done.

> key1 := strings.Repeat("X", 1<<20)
> key2 := strings.Repeat("X", 1<<20) // equal but different instance

Done.

> The Go functions have some kind of crossover point at around 256K which
> only kicks in at around 2M in glibc.

> [2] This difference might be worth a look:

There are a lot of things that could be making a difference here.  Maybe
they do prefetching, maybe they do the extra work to use aligned loads, ...
 I'll take a look at the glibc code, it might have some useful ideas in it.

On Fri, Mar 29, 2013 at 8:44 AM, <bradfitz@golang.org> wrote:

>
> https://codereview.appspot.**com/8056043/diff/39001/src/**
>
pkg/runtime/mapspeed_test.go<https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/mapspeed_test.go>
> File src/pkg/runtime/mapspeed_test.**go (right):
>
> https://codereview.appspot.**com/8056043/diff/39001/src/**
>
pkg/runtime/mapspeed_test.go#**newcode124<https://codereview.appspot.com/8056043/diff/39001/src/pkg/runtime/mapspeed_test.go#newcode124>
> src/pkg/runtime/mapspeed_test.**go:124: key := strings.Repeat("X", 1<<20)
> On 2013/03/29 07:26:09, bradfitz wrote:
>
>> move this up a line and just say m[key] = true
>>
>
> Oh, you probably did it this way so their .str fields were different, to
> avoid that fastpath.  Leave a comment?
>
> key1 := strings.Repeat("X", 1<<20)
> key2 := strings.Repeat("X", 1<<20) // equal but different instance
>
>
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>

[ality, 2013-03-29 17:58:09]

minux <minux.ma@gmail.com> once said:
> On Fri, Mar 29, 2013 at 3:53 PM, Keith Randall <khr@google.com> wrote:
> > I'll ask around about 386.
> please use cpuid to detect sse2 on 386 as we still support as old as
> pentium mmx chips.
> sse2 is required feature for amd64 so using it is ok.

According to my notes¹:

We haven't supported the Pentium MMX since January of 2010.
Around that time, the compiler started emitting FUCOMIP
during floating point codegen so the bare minimum was a
Pentium Pro.

The math package started using FUCOMI at the same time.

In May of 2012, Russ added the PREFETCH builtin for the
garbage collector which causes 8c to generate PREFETCHNTA.
This bumped the lower limit up to the Pentium III.

I don't know how the PREFETCH instructions are decoded
on processors older than the Pentium III. I was 11 when
it came out. :)

Does it end up as a NOP like PAUSE?
If so, the oldest we support is a Pentium Pro.
If not, the oldest is a Pentium III.

  Anthony

1. I keep a file of notes about how the toolchain differs
   from the original Plan 9 code.

[r, 2013-03-29 18:39:46]

https://codereview.appspot.com/8056043/diff/35002/src/pkg/bytes/bytes_test.go
File src/pkg/bytes/bytes_test.go (right):

https://codereview.appspot.com/8056043/diff/35002/src/pkg/bytes/bytes_test.go...
src/pkg/bytes/bytes_test.go:94: b1 := make([]byte, 64)
const size = 64

https://codereview.appspot.com/8056043/diff/35002/src/pkg/bytes/bytes_test.go...
src/pkg/bytes/bytes_test.go:102: }
outside the loop create
zeros := make([]byte, size)
then
copy(b1, zeros)
copy(b2, zeros)
makes it very clear

[ality, 2013-03-29 19:07:09]

Anthony Martin <ality@pbrane.org> once said:
> minux <minux.ma@gmail.com> once said:
> > On Fri, Mar 29, 2013 at 3:53 PM, Keith Randall <khr@google.com> wrote:
> > > I'll ask around about 386.
> > please use cpuid to detect sse2 on 386 as we still support as old as
> > pentium mmx chips.
> > sse2 is required feature for amd64 so using it is ok.
> 
> According to my notes¹:
> 
> We haven't supported the Pentium MMX since January of 2010.
> Around that time, the compiler started emitting FUCOMIP
> during floating point codegen so the bare minimum was a
> Pentium Pro.
> 
> The math package started using FUCOMI at the same time.
> 
> In May of 2012, Russ added the PREFETCH builtin for the
> garbage collector which causes 8c to generate PREFETCHNTA.
> This bumped the lower limit up to the Pentium III.
> 
> I don't know how the PREFETCH instructions are decoded
> on processors older than the Pentium III. I was 11 when
> it came out. :)
> 
> Does it end up as a NOP like PAUSE?
> If so, the oldest we support is a Pentium Pro.
> If not, the oldest is a Pentium III.
> 
>   Anthony
> 
> 1. I keep a file of notes about how the toolchain differs
>    from the original Plan 9 code.
> 

Please s/Pentium Pro/Pentium II/ before reading.
I forgot that the Pro didn't have MMX.

  Anthony

[minux1, 2013-03-30 16:15:25]

On Sat, Mar 30, 2013 at 1:58 AM, Anthony Martin <ality@pbrane.org> wrote:
> According to my notes¹:
>
> We haven't supported the Pentium MMX since January of 2010.
> Around that time, the compiler started emitting FUCOMIP
> during floating point codegen so the bare minimum was a
> Pentium Pro.
oh, seems this one is correct, we require pentium pro.
thank you for the correction.

> In May of 2012, Russ added the PREFETCH builtin for the
> garbage collector which causes 8c to generate PREFETCHNTA.
> This bumped the lower limit up to the Pentium III.
in CL 7323061, Russ disabled the generation of PREFETCH instructions
when GO386=387 (i.e. sse2 is either not detected or explicitly disabled
by the user), so this is a non-issue now and the minimum supported
386 hardware is still pentium pro.

[ality, 2013-03-30 16:51:48]

minux <minux.ma@gmail.com> once said:
> On Sat, Mar 30, 2013 at 1:58 AM, Anthony Martin <ality@pbrane.org> wrote:
> > According to my notes¹:
> >
> > We haven't supported the Pentium MMX since January of 2010.
> > Around that time, the compiler started emitting FUCOMIP
> > during floating point codegen so the bare minimum was a
> > Pentium Pro.
> oh, seems this one is correct, we require pentium pro.
> thank you for the correction.
> 
> > In May of 2012, Russ added the PREFETCH builtin for the
> > garbage collector which causes 8c to generate PREFETCHNTA.
> > This bumped the lower limit up to the Pentium III.
> in CL 7323061, Russ disabled the generation of PREFETCH instructions
> when GO386=387 (i.e. sse2 is either not detected or explicitly disabled
> by the user), so this is a non-issue now and the minimum supported
> 386 hardware is still pentium pro.

I see. I overlooked that CL. Thanks.

We should mention somewhere that the minimum requirement is
a P6 family processor (i686) with MMX extensions, i.e., the
Pentium II.

  Anthony

[minux1, 2013-03-30 17:08:48]

On Sun, Mar 31, 2013 at 12:51 AM, Anthony Martin <ality@pbrane.org> wrote:
> minux <minux.ma@gmail.com> once said:
> We should mention somewhere that the minimum requirement is
> a P6 family processor (i686) with MMX extensions, i.e., the
> Pentium II.
Agreed.

For the record,
we require MMX because sync/atomic uses MMX instructions for 64-bit atomic.
we require Pentium-Pro (P6 class) because we use FUCOMI for FP comparison
in 8g.
because Pentium Pro is the only P6 class processor that doesn't support MMX,
we need Pentium II or higher.

I'm not familiar with AMD produce line, so could someone familiar with it
supply
the info for AMD CPU and update doc/install.html?

[dfc, 2013-03-31 00:04:11]

I benchmarked patchset 10 on my chromebook, the results are mixed. The
regressions in the bytes benchmarks appear to flow through in to strings and
fmt.

lucky(~) % cat bytes.txt 
benchmark                           old ns/op    new ns/op    delta
BenchmarkReadString                    112131       127869  +14.04%
BenchmarkBufferNotEmptyWriteRead      3634051      3681321   +1.30%
BenchmarkBufferFullSmallReads          440993       441639   +0.15%
BenchmarkIndexByte32                       76           76   +0.00%
BenchmarkIndexByte4K                     7279         7286   +0.10%
BenchmarkIndexByte4M                  7533005      7564464   +0.42%
BenchmarkIndexByte64M               120355191    120813462   +0.38%
BenchmarkIndexBytePortable32              174          175   +0.57%
BenchmarkIndexBytePortable4K            17302        17342   +0.23%
BenchmarkIndexBytePortable4M         18276630     18349412   +0.40%
BenchmarkIndexBytePortable64M       292254766    293383533   +0.39%
BenchmarkEqual32                           90           60  -33.22%   
BenchmarkEqual4K                         7285         1853  -74.56%
BenchmarkEqual4M                      8829644      3517534  -60.16%
BenchmarkEqual64M                   138950637     59169793  -57.42%
BenchmarkEqualPort32                      223          224   +0.45%
BenchmarkEqualPort4K                    24201        24255   +0.22%
BenchmarkEqualPortable4M             25031372     25131132   +0.40%
BenchmarkEqualPortable64M           402236591    403740442   +0.37%
BenchmarkIndex32                          800         1182  +47.75%
BenchmarkIndex4K                       118306       178704  +51.05%
BenchmarkIndex4M                    121457245    183889629  +51.40%
BenchmarkIndex64M                  1943517375   2939630294  +51.25%
BenchmarkIndexEasy32                      142          142   +0.00%
BenchmarkIndexEasy4K                     7331         7336   +0.07%
BenchmarkIndexEasy4M                  7548436      7543304   -0.07%
BenchmarkIndexEasy64M               120458129    120302727   -0.13%
BenchmarkCount32                          806         1182  +46.65%
BenchmarkCount4K                       118344       178649  +50.96%
BenchmarkCount4M                    121673656    183400741  +50.73%
BenchmarkCount64M                  1944794002   2934607085  +50.90%
BenchmarkCountEasy32                      144          141   -2.08%
BenchmarkCountEasy4K                     7360         7335   -0.34%
BenchmarkCountEasy4M                  7554226      7564146   +0.13%
BenchmarkCountEasy64M               120317272    120715047   +0.33%
BenchmarkFields                     116213231    114222702   -1.71%
BenchmarkFieldsFunc                 114475948    113263035   -1.06%
BenchmarkTrimSpace                        342          336   -1.75%

benchmark                            old MB/s     new MB/s  speedup
BenchmarkReadString                    292.23       256.26    0.88x
BenchmarkIndexByte32                   419.49       419.49    1.00x
BenchmarkIndexByte4K                   562.70       562.14    1.00x
BenchmarkIndexByte4M                   556.79       554.47    1.00x
BenchmarkIndexByte64M                  557.59       555.48    1.00x
BenchmarkIndexBytePortable32           183.22       182.84    1.00x
BenchmarkIndexBytePortable4K           236.72       236.18    1.00x
BenchmarkIndexBytePortable4M           229.49       228.58    1.00x
BenchmarkIndexBytePortable64M          229.62       228.74    1.00x
BenchmarkEqual32                       354.47       530.72    1.50x
BenchmarkEqual4K                       562.22      2210.19    3.93x
BenchmarkEqual4M                       475.03      1192.40    2.51x
BenchmarkEqual64M                      482.97      1134.17    2.35x
BenchmarkEqualPort32                   143.47       142.77    1.00x
BenchmarkEqualPort4K                   169.24       168.87    1.00x
BenchmarkEqualPortable4M               167.56       166.90    1.00x
BenchmarkEqualPortable64M              166.84       166.22    1.00x
BenchmarkIndex32                        39.97        27.07    0.68x
BenchmarkIndex4K                        34.62        22.92    0.66x
BenchmarkIndex4M                        34.53        22.81    0.66x
BenchmarkIndex64M                       34.53        22.83    0.66x
BenchmarkIndexEasy32                   224.99       224.98    1.00x
BenchmarkIndexEasy4K                   558.69       558.33    1.00x
BenchmarkIndexEasy4M                   555.65       556.03    1.00x
BenchmarkIndexEasy64M                  557.11       557.83    1.00x
BenchmarkCount32                        39.69        27.06    0.68x
BenchmarkCount4K                        34.61        22.93    0.66x
BenchmarkCount4M                        34.47        22.87    0.66x
BenchmarkCount64M                       34.51        22.87    0.66x
BenchmarkCountEasy32                   221.56       225.87    1.02x
BenchmarkCountEasy4K                   556.51       558.36    1.00x
BenchmarkCountEasy4M                   555.23       554.50    1.00x
BenchmarkCountEasy64M                  557.77       555.93    1.00x
BenchmarkFields                          9.02         9.18    1.02x
BenchmarkFieldsFunc                      9.16         9.26    1.01x
lucky(~) % cat strings.txt 
benchmark                           old ns/op    new ns/op    delta
BenchmarkGenericNoMatch                  7555         7427   -1.69%
BenchmarkGenericMatch1                  29053        29279   +0.78%
BenchmarkGenericMatch2                 175206       188767   +7.74%
BenchmarkSingleMaxSkipping               5981         5976   -0.08%
BenchmarkSingleLongSuffixFail            6083         6043   -0.66%
BenchmarkSingleMatch                   376140       385756   +2.56%
BenchmarkByteByteNoMatch                 2301         2301   +0.00%
BenchmarkByteByteMatch                   4902         4913   +0.22%
BenchmarkByteStringMatch                 7346         7210   -1.85%
BenchmarkHTMLEscapeNew                   2305         2339   +1.48%
BenchmarkHTMLEscapeOld                   3783         3812   +0.77%
BenchmarkByteByteReplaces               18253        18348   +0.52%
BenchmarkByteByteMap                     9875         9685   -1.92%
BenchmarkIndexRune                        246          254   +3.25%
BenchmarkIndexRuneFastPath                130          116  -10.77%
BenchmarkIndex                            129          129   +0.00%
BenchmarkMapNoChanges                     799          843   +5.51%
BenchmarkIndexHard1                   7637311      7636978   -0.00%
BenchmarkIndexHard2                   7647121      7647579   +0.01%
BenchmarkIndexHard3                   7628415      7637353   +0.12%
BenchmarkCountHard1                   8250395      8260440   +0.12%
BenchmarkCountHard2                   8253451      8260638   +0.09%
BenchmarkCountHard3                   8250371      8263897   +0.16%
BenchmarkIndexTorture                   47275        47389   +0.24%
BenchmarkCountTorture                   49252        49139   -0.23%
BenchmarkCountTortureOverlapping     52097076     29447134  -43.48%
BenchmarkFields                      90120216     89680439   -0.49%
BenchmarkFieldsFunc                  90004377     89325775   -0.75%
BenchmarkSplit1                      76455569     76104520   -0.46%
BenchmarkSplit2                      20392480     19932127   -2.26%
BenchmarkSplit3                      18983460     18571189   -2.17%

benchmark                            old MB/s     new MB/s  speedup
BenchmarkSingleMaxSkipping            1671.80      1673.30    1.00x
BenchmarkSingleLongSuffixFail          164.71       165.78    1.01x
BenchmarkSingleMatch                    39.88        38.88    0.97x
BenchmarkFields                         11.64        11.69    1.00x
BenchmarkFieldsFunc                     11.65        11.74    1.01x
lucky(~) % cat fmt.txt 
benchmark                      old ns/op    new ns/op    delta
BenchmarkSprintfEmpty                424          424   +0.00%
BenchmarkSprintfString              1499         1451   -3.20%
BenchmarkSprintfInt                 1261         1438  +14.04%
BenchmarkSprintfIntInt              2042         2031   -0.54%
BenchmarkSprintfPrefixedInt         2028         2165   +6.76%
BenchmarkSprintfFloat               3579         3686   +2.99%
BenchmarkManyArgs                   5573         6130   +9.99%
BenchmarkScanInts                5413886      5166830   -4.56%
BenchmarkScanRecursiveInt        6447875      6478815   +0.48%

[bradfitz, 2013-03-31 00:54:53]

Which arch is your chromebook?

On Sat, Mar 30, 2013 at 5:04 PM, <dave@cheney.net> wrote:

> I benchmarked patchset 10 on my chromebook, the results are mixed. The
> regressions in the bytes benchmarks appear to flow through in to strings
> and fmt.
>
> lucky(~) % cat bytes.txt
>
> benchmark                           old ns/op    new ns/op    delta
> BenchmarkReadString                    112131       127869  +14.04%
> BenchmarkBufferNotEmptyWriteRe**ad      3634051      3681321   +1.30%
> BenchmarkBufferFullSmallReads          440993       441639   +0.15%
> BenchmarkIndexByte32                       76           76   +0.00%
> BenchmarkIndexByte4K                     7279         7286   +0.10%
> BenchmarkIndexByte4M                  7533005      7564464   +0.42%
> BenchmarkIndexByte64M               120355191    120813462   +0.38%
> BenchmarkIndexBytePortable32              174          175   +0.57%
> BenchmarkIndexBytePortable4K            17302        17342   +0.23%
> BenchmarkIndexBytePortable4M         18276630     18349412   +0.40%
> BenchmarkIndexBytePortable64M       292254766    293383533   +0.39%
> BenchmarkEqual32                           90           60  -33.22%
> BenchmarkEqual4K                         7285         1853  -74.56%
> BenchmarkEqual4M                      8829644      3517534  -60.16%
> BenchmarkEqual64M                   138950637     59169793  -57.42%
> BenchmarkEqualPort32                      223          224   +0.45%
> BenchmarkEqualPort4K                    24201        24255   +0.22%
> BenchmarkEqualPortable4M             25031372     25131132   +0.40%
> BenchmarkEqualPortable64M           402236591    403740442   +0.37%
> BenchmarkIndex32                          800         1182  +47.75%
> BenchmarkIndex4K                       118306       178704  +51.05%
> BenchmarkIndex4M                    121457245    183889629  +51.40%
> BenchmarkIndex64M                  1943517375   2939630294  +51.25%
> BenchmarkIndexEasy32                      142          142   +0.00%
> BenchmarkIndexEasy4K                     7331         7336   +0.07%
> BenchmarkIndexEasy4M                  7548436      7543304   -0.07%
> BenchmarkIndexEasy64M               120458129    120302727   -0.13%
> BenchmarkCount32                          806         1182  +46.65%
> BenchmarkCount4K                       118344       178649  +50.96%
> BenchmarkCount4M                    121673656    183400741  +50.73%
> BenchmarkCount64M                  1944794002   2934607085  +50.90%
> BenchmarkCountEasy32                      144          141   -2.08%
> BenchmarkCountEasy4K                     7360         7335   -0.34%
> BenchmarkCountEasy4M                  7554226      7564146   +0.13%
> BenchmarkCountEasy64M               120317272    120715047   +0.33%
> BenchmarkFields                     116213231    114222702   -1.71%
> BenchmarkFieldsFunc                 114475948    113263035   -1.06%
> BenchmarkTrimSpace                        342          336   -1.75%
>
>
> benchmark                            old MB/s     new MB/s  speedup
> BenchmarkReadString                    292.23       256.26    0.88x
> BenchmarkIndexByte32                   419.49       419.49    1.00x
> BenchmarkIndexByte4K                   562.70       562.14    1.00x
> BenchmarkIndexByte4M                   556.79       554.47    1.00x
> BenchmarkIndexByte64M                  557.59       555.48    1.00x
> BenchmarkIndexBytePortable32           183.22       182.84    1.00x
> BenchmarkIndexBytePortable4K           236.72       236.18    1.00x
> BenchmarkIndexBytePortable4M           229.49       228.58    1.00x
> BenchmarkIndexBytePortable64M          229.62       228.74    1.00x
> BenchmarkEqual32                       354.47       530.72    1.50x
> BenchmarkEqual4K                       562.22      2210.19    3.93x
> BenchmarkEqual4M                       475.03      1192.40    2.51x
> BenchmarkEqual64M                      482.97      1134.17    2.35x
> BenchmarkEqualPort32                   143.47       142.77    1.00x
> BenchmarkEqualPort4K                   169.24       168.87    1.00x
> BenchmarkEqualPortable4M               167.56       166.90    1.00x
> BenchmarkEqualPortable64M              166.84       166.22    1.00x
> BenchmarkIndex32                        39.97        27.07    0.68x
> BenchmarkIndex4K                        34.62        22.92    0.66x
> BenchmarkIndex4M                        34.53        22.81    0.66x
> BenchmarkIndex64M                       34.53        22.83    0.66x
> BenchmarkIndexEasy32                   224.99       224.98    1.00x
> BenchmarkIndexEasy4K                   558.69       558.33    1.00x
> BenchmarkIndexEasy4M                   555.65       556.03    1.00x
> BenchmarkIndexEasy64M                  557.11       557.83    1.00x
> BenchmarkCount32                        39.69        27.06    0.68x
> BenchmarkCount4K                        34.61        22.93    0.66x
> BenchmarkCount4M                        34.47        22.87    0.66x
> BenchmarkCount64M                       34.51        22.87    0.66x
> BenchmarkCountEasy32                   221.56       225.87    1.02x
> BenchmarkCountEasy4K                   556.51       558.36    1.00x
> BenchmarkCountEasy4M                   555.23       554.50    1.00x
> BenchmarkCountEasy64M                  557.77       555.93    1.00x
> BenchmarkFields                          9.02         9.18    1.02x
> BenchmarkFieldsFunc                      9.16         9.26    1.01x
> lucky(~) % cat strings.txt
>
> benchmark                           old ns/op    new ns/op    delta
> BenchmarkGenericNoMatch                  7555         7427   -1.69%
> BenchmarkGenericMatch1                  29053        29279   +0.78%
> BenchmarkGenericMatch2                 175206       188767   +7.74%
> BenchmarkSingleMaxSkipping               5981         5976   -0.08%
> BenchmarkSingleLongSuffixFail            6083         6043   -0.66%
> BenchmarkSingleMatch                   376140       385756   +2.56%
> BenchmarkByteByteNoMatch                 2301         2301   +0.00%
> BenchmarkByteByteMatch                   4902         4913   +0.22%
> BenchmarkByteStringMatch                 7346         7210   -1.85%
> BenchmarkHTMLEscapeNew                   2305         2339   +1.48%
> BenchmarkHTMLEscapeOld                   3783         3812   +0.77%
> BenchmarkByteByteReplaces               18253        18348   +0.52%
> BenchmarkByteByteMap                     9875         9685   -1.92%
> BenchmarkIndexRune                        246          254   +3.25%
> BenchmarkIndexRuneFastPath                130          116  -10.77%
> BenchmarkIndex                            129          129   +0.00%
> BenchmarkMapNoChanges                     799          843   +5.51%
> BenchmarkIndexHard1                   7637311      7636978   -0.00%
> BenchmarkIndexHard2                   7647121      7647579   +0.01%
> BenchmarkIndexHard3                   7628415      7637353   +0.12%
> BenchmarkCountHard1                   8250395      8260440   +0.12%
> BenchmarkCountHard2                   8253451      8260638   +0.09%
> BenchmarkCountHard3                   8250371      8263897   +0.16%
> BenchmarkIndexTorture                   47275        47389   +0.24%
> BenchmarkCountTorture                   49252        49139   -0.23%
> BenchmarkCountTortureOverlappi**ng     52097076     29447134  -43.48%
> BenchmarkFields                      90120216     89680439   -0.49%
> BenchmarkFieldsFunc                  90004377     89325775   -0.75%
> BenchmarkSplit1                      76455569     76104520   -0.46%
> BenchmarkSplit2                      20392480     19932127   -2.26%
> BenchmarkSplit3                      18983460     18571189   -2.17%
>
>
> benchmark                            old MB/s     new MB/s  speedup
> BenchmarkSingleMaxSkipping            1671.80      1673.30    1.00x
> BenchmarkSingleLongSuffixFail          164.71       165.78    1.01x
> BenchmarkSingleMatch                    39.88        38.88    0.97x
> BenchmarkFields                         11.64        11.69    1.00x
> BenchmarkFieldsFunc                     11.65        11.74    1.01x
> lucky(~) % cat fmt.txt
>
> benchmark                      old ns/op    new ns/op    delta
> BenchmarkSprintfEmpty                424          424   +0.00%
> BenchmarkSprintfString              1499         1451   -3.20%
> BenchmarkSprintfInt                 1261         1438  +14.04%
> BenchmarkSprintfIntInt              2042         2031   -0.54%
> BenchmarkSprintfPrefixedInt         2028         2165   +6.76%
> BenchmarkSprintfFloat               3579         3686   +2.99%
> BenchmarkManyArgs                   5573         6130   +9.99%
> BenchmarkScanInts                5413886      5166830   -4.56%
> BenchmarkScanRecursiveInt        6447875      6478815   +0.48%
>
>
>
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>

[dfc, 2013-03-31 00:57:13]

This is the dual A15 Exynos 5 version. I can do some benchmarks on a
pandaboard or a RPi but I don't expect much change.

On Sun, Mar 31, 2013 at 11:54 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> Which arch is your chromebook?
>
>
> On Sat, Mar 30, 2013 at 5:04 PM, <dave@cheney.net> wrote:
>>
>> I benchmarked patchset 10 on my chromebook, the results are mixed. The
>> regressions in the bytes benchmarks appear to flow through in to strings
>> and fmt.
>>
>> lucky(~) % cat bytes.txt
>>
>> benchmark                           old ns/op    new ns/op    delta
>> BenchmarkReadString                    112131       127869  +14.04%
>> BenchmarkBufferNotEmptyWriteRead      3634051      3681321   +1.30%
>> BenchmarkBufferFullSmallReads          440993       441639   +0.15%
>> BenchmarkIndexByte32                       76           76   +0.00%
>> BenchmarkIndexByte4K                     7279         7286   +0.10%
>> BenchmarkIndexByte4M                  7533005      7564464   +0.42%
>> BenchmarkIndexByte64M               120355191    120813462   +0.38%
>> BenchmarkIndexBytePortable32              174          175   +0.57%
>> BenchmarkIndexBytePortable4K            17302        17342   +0.23%
>> BenchmarkIndexBytePortable4M         18276630     18349412   +0.40%
>> BenchmarkIndexBytePortable64M       292254766    293383533   +0.39%
>> BenchmarkEqual32                           90           60  -33.22%
>> BenchmarkEqual4K                         7285         1853  -74.56%
>> BenchmarkEqual4M                      8829644      3517534  -60.16%
>> BenchmarkEqual64M                   138950637     59169793  -57.42%
>> BenchmarkEqualPort32                      223          224   +0.45%
>> BenchmarkEqualPort4K                    24201        24255   +0.22%
>> BenchmarkEqualPortable4M             25031372     25131132   +0.40%
>> BenchmarkEqualPortable64M           402236591    403740442   +0.37%
>> BenchmarkIndex32                          800         1182  +47.75%
>> BenchmarkIndex4K                       118306       178704  +51.05%
>> BenchmarkIndex4M                    121457245    183889629  +51.40%
>> BenchmarkIndex64M                  1943517375   2939630294  +51.25%
>> BenchmarkIndexEasy32                      142          142   +0.00%
>> BenchmarkIndexEasy4K                     7331         7336   +0.07%
>> BenchmarkIndexEasy4M                  7548436      7543304   -0.07%
>> BenchmarkIndexEasy64M               120458129    120302727   -0.13%
>> BenchmarkCount32                          806         1182  +46.65%
>> BenchmarkCount4K                       118344       178649  +50.96%
>> BenchmarkCount4M                    121673656    183400741  +50.73%
>> BenchmarkCount64M                  1944794002   2934607085  +50.90%
>> BenchmarkCountEasy32                      144          141   -2.08%
>> BenchmarkCountEasy4K                     7360         7335   -0.34%
>> BenchmarkCountEasy4M                  7554226      7564146   +0.13%
>> BenchmarkCountEasy64M               120317272    120715047   +0.33%
>> BenchmarkFields                     116213231    114222702   -1.71%
>> BenchmarkFieldsFunc                 114475948    113263035   -1.06%
>> BenchmarkTrimSpace                        342          336   -1.75%
>>
>>
>> benchmark                            old MB/s     new MB/s  speedup
>> BenchmarkReadString                    292.23       256.26    0.88x
>> BenchmarkIndexByte32                   419.49       419.49    1.00x
>> BenchmarkIndexByte4K                   562.70       562.14    1.00x
>> BenchmarkIndexByte4M                   556.79       554.47    1.00x
>> BenchmarkIndexByte64M                  557.59       555.48    1.00x
>> BenchmarkIndexBytePortable32           183.22       182.84    1.00x
>> BenchmarkIndexBytePortable4K           236.72       236.18    1.00x
>> BenchmarkIndexBytePortable4M           229.49       228.58    1.00x
>> BenchmarkIndexBytePortable64M          229.62       228.74    1.00x
>> BenchmarkEqual32                       354.47       530.72    1.50x
>> BenchmarkEqual4K                       562.22      2210.19    3.93x
>> BenchmarkEqual4M                       475.03      1192.40    2.51x
>> BenchmarkEqual64M                      482.97      1134.17    2.35x
>> BenchmarkEqualPort32                   143.47       142.77    1.00x
>> BenchmarkEqualPort4K                   169.24       168.87    1.00x
>> BenchmarkEqualPortable4M               167.56       166.90    1.00x
>> BenchmarkEqualPortable64M              166.84       166.22    1.00x
>> BenchmarkIndex32                        39.97        27.07    0.68x
>> BenchmarkIndex4K                        34.62        22.92    0.66x
>> BenchmarkIndex4M                        34.53        22.81    0.66x
>> BenchmarkIndex64M                       34.53        22.83    0.66x
>> BenchmarkIndexEasy32                   224.99       224.98    1.00x
>> BenchmarkIndexEasy4K                   558.69       558.33    1.00x
>> BenchmarkIndexEasy4M                   555.65       556.03    1.00x
>> BenchmarkIndexEasy64M                  557.11       557.83    1.00x
>> BenchmarkCount32                        39.69        27.06    0.68x
>> BenchmarkCount4K                        34.61        22.93    0.66x
>> BenchmarkCount4M                        34.47        22.87    0.66x
>> BenchmarkCount64M                       34.51        22.87    0.66x
>> BenchmarkCountEasy32                   221.56       225.87    1.02x
>> BenchmarkCountEasy4K                   556.51       558.36    1.00x
>> BenchmarkCountEasy4M                   555.23       554.50    1.00x
>> BenchmarkCountEasy64M                  557.77       555.93    1.00x
>> BenchmarkFields                          9.02         9.18    1.02x
>> BenchmarkFieldsFunc                      9.16         9.26    1.01x
>> lucky(~) % cat strings.txt
>>
>> benchmark                           old ns/op    new ns/op    delta
>> BenchmarkGenericNoMatch                  7555         7427   -1.69%
>> BenchmarkGenericMatch1                  29053        29279   +0.78%
>> BenchmarkGenericMatch2                 175206       188767   +7.74%
>> BenchmarkSingleMaxSkipping               5981         5976   -0.08%
>> BenchmarkSingleLongSuffixFail            6083         6043   -0.66%
>> BenchmarkSingleMatch                   376140       385756   +2.56%
>> BenchmarkByteByteNoMatch                 2301         2301   +0.00%
>> BenchmarkByteByteMatch                   4902         4913   +0.22%
>> BenchmarkByteStringMatch                 7346         7210   -1.85%
>> BenchmarkHTMLEscapeNew                   2305         2339   +1.48%
>> BenchmarkHTMLEscapeOld                   3783         3812   +0.77%
>> BenchmarkByteByteReplaces               18253        18348   +0.52%
>> BenchmarkByteByteMap                     9875         9685   -1.92%
>> BenchmarkIndexRune                        246          254   +3.25%
>> BenchmarkIndexRuneFastPath                130          116  -10.77%
>> BenchmarkIndex                            129          129   +0.00%
>> BenchmarkMapNoChanges                     799          843   +5.51%
>> BenchmarkIndexHard1                   7637311      7636978   -0.00%
>> BenchmarkIndexHard2                   7647121      7647579   +0.01%
>> BenchmarkIndexHard3                   7628415      7637353   +0.12%
>> BenchmarkCountHard1                   8250395      8260440   +0.12%
>> BenchmarkCountHard2                   8253451      8260638   +0.09%
>> BenchmarkCountHard3                   8250371      8263897   +0.16%
>> BenchmarkIndexTorture                   47275        47389   +0.24%
>> BenchmarkCountTorture                   49252        49139   -0.23%
>> BenchmarkCountTortureOverlapping     52097076     29447134  -43.48%
>> BenchmarkFields                      90120216     89680439   -0.49%
>> BenchmarkFieldsFunc                  90004377     89325775   -0.75%
>> BenchmarkSplit1                      76455569     76104520   -0.46%
>> BenchmarkSplit2                      20392480     19932127   -2.26%
>> BenchmarkSplit3                      18983460     18571189   -2.17%
>>
>>
>> benchmark                            old MB/s     new MB/s  speedup
>> BenchmarkSingleMaxSkipping            1671.80      1673.30    1.00x
>> BenchmarkSingleLongSuffixFail          164.71       165.78    1.01x
>> BenchmarkSingleMatch                    39.88        38.88    0.97x
>> BenchmarkFields                         11.64        11.69    1.00x
>> BenchmarkFieldsFunc                     11.65        11.74    1.01x
>> lucky(~) % cat fmt.txt
>>
>> benchmark                      old ns/op    new ns/op    delta
>> BenchmarkSprintfEmpty                424          424   +0.00%
>> BenchmarkSprintfString              1499         1451   -3.20%
>> BenchmarkSprintfInt                 1261         1438  +14.04%
>> BenchmarkSprintfIntInt              2042         2031   -0.54%
>> BenchmarkSprintfPrefixedInt         2028         2165   +6.76%
>> BenchmarkSprintfFloat               3579         3686   +2.99%
>> BenchmarkManyArgs                   5573         6130   +9.99%
>> BenchmarkScanInts                5413886      5166830   -4.56%
>> BenchmarkScanRecursiveInt        6447875      6478815   +0.48%
>>
>>
>> https://codereview.appspot.com/8056043/
>
>

[dfc, 2013-03-31 01:25:18]

Here are some linux/386 from a 4 core atom 330.

220887(~) % cat bytes.txt 
benchmark                           old ns/op    new ns/op    delta
BenchmarkReadString                    121057       126571   +4.55%
BenchmarkBufferNotEmptyWriteRead      5503485      5454785   -0.88%
BenchmarkBufferFullSmallReads          891073       889944   -0.13%
BenchmarkIndexByte32                       94           94   -0.11%
BenchmarkIndexByte4K                     7781         7813   +0.41%
BenchmarkIndexByte4M                  8619393      8483188   -1.58%
BenchmarkIndexByte64M               136653719    135896031   -0.55%
BenchmarkIndexBytePortable32              466          465   -0.21%
BenchmarkIndexBytePortable4K            56721        56661   -0.11%
BenchmarkIndexBytePortable4M         58213450     58148782   -0.11%
BenchmarkIndexBytePortable64M       931072226    930535077   -0.06%
BenchmarkEqual32                          119           63  -46.64%
BenchmarkEqual4K                        10348         2443  -76.39%
BenchmarkEqual4M                     11544390      3224185  -72.07%
BenchmarkEqual64M                   185016250     56713834  -69.35%
BenchmarkEqualPort32                      782          785   +0.38%
BenchmarkEqualPort4K                    95370        96032   +0.69%
BenchmarkEqualPortable4M             98030915     98312648   +0.29%
BenchmarkEqualPortable64M          1568407685   1573236860   +0.31%
BenchmarkIndex32                         2319         1923  -17.08%
BenchmarkIndex4K                       354957       295392  -16.78%
BenchmarkIndex4M                    364731912    303230991  -16.86%
BenchmarkIndex64M                  2147483647   2147483647  -16.86%
BenchmarkIndexEasy32                      227          210   -7.49%
BenchmarkIndexEasy4K                     7899         7901   +0.03%
BenchmarkIndexEasy4M                  8486584      8516794   +0.36%
BenchmarkIndexEasy64M               135771448    136296410   +0.39%
BenchmarkCount32                         1838         1460  -20.57%
BenchmarkCount4K                       288436       220826  -23.44%
BenchmarkCount4M                    286733235    227035260  -20.82%
BenchmarkCount64M                  2147483647   2147483647  -21.37%
BenchmarkCountEasy32                      210          193   -8.10%
BenchmarkCountEasy4K                     7888         7875   -0.16%
BenchmarkCountEasy4M                  8483266      8527767   +0.52%
BenchmarkCountEasy64M               135677742    136246780   +0.42%
BenchmarkFields                     226913045    222574539   -1.91%
BenchmarkFieldsFunc                 226840521    222770626   -1.79%
BenchmarkTrimSpace                        477          503   +5.45%

benchmark                            old MB/s     new MB/s  speedup
BenchmarkReadString                    270.68       258.89    0.96x
BenchmarkIndexByte32                   338.57       339.07    1.00x
BenchmarkIndexByte4K                   526.40       524.23    1.00x
BenchmarkIndexByte4M                   486.61       494.43    1.02x
BenchmarkIndexByte64M                  491.09       493.83    1.01x
BenchmarkIndexBytePortable32            68.55        68.69    1.00x
BenchmarkIndexBytePortable4K            72.21        72.29    1.00x
BenchmarkIndexBytePortable4M            72.05        72.13    1.00x
BenchmarkIndexBytePortable64M           72.08        72.12    1.00x
BenchmarkEqual32                       267.72       503.99    1.88x
BenchmarkEqual4K                       395.79      1676.25    4.24x
BenchmarkEqual4M                       363.32      1300.89    3.58x
BenchmarkEqual64M                      362.72      1183.29    3.26x
BenchmarkEqualPort32                    40.88        40.74    1.00x
BenchmarkEqualPort4K                    42.95        42.65    0.99x
BenchmarkEqualPortable4M                42.79        42.66    1.00x
BenchmarkEqualPortable64M               42.79        42.66    1.00x
BenchmarkIndex32                        13.80        16.64    1.21x
BenchmarkIndex4K                        11.54        13.87    1.20x
BenchmarkIndex4M                        11.50        13.83    1.20x
BenchmarkIndex64M                       11.50        13.83    1.20x
BenchmarkIndexEasy32                   140.52       152.16    1.08x
BenchmarkIndexEasy4K                   518.55       518.40    1.00x
BenchmarkIndexEasy4M                   494.23       492.47    1.00x
BenchmarkIndexEasy64M                  494.28       492.37    1.00x
BenchmarkCount32                        17.40        21.91    1.26x
BenchmarkCount4K                        14.20        18.55    1.31x
BenchmarkCount4M                        14.63        18.47    1.26x
BenchmarkCount64M                       14.53        18.48    1.27x
BenchmarkCountEasy32                   152.37       165.42    1.09x
BenchmarkCountEasy4K                   519.27       520.10    1.00x
BenchmarkCountEasy4M                   494.42       491.84    0.99x
BenchmarkCountEasy64M                  494.62       492.55    1.00x
BenchmarkFields                          4.62         4.71    1.02x
BenchmarkFieldsFunc                      4.62         4.71    1.02x
220887(~) % cat strings.txt 
benchmark                           old ns/op    new ns/op    delta
BenchmarkGenericNoMatch                 19988        18825   -5.82%
BenchmarkGenericMatch1                  68559        68689   +0.19%
BenchmarkGenericMatch2                 364551       364833   +0.08%
BenchmarkSingleMaxSkipping              16873        15462   -8.36%
BenchmarkSingleLongSuffixFail           29288        28511   -2.65%
BenchmarkSingleMatch                   826237       743121  -10.06%
BenchmarkByteByteNoMatch                 3571         3340   -6.47%
BenchmarkByteByteMatch                   9512         7961  -16.31%
BenchmarkByteStringMatch                10368        10101   -2.58%
BenchmarkHTMLEscapeNew                   3159         3015   -4.56%
BenchmarkHTMLEscapeOld                   9431         9076   -3.76%
BenchmarkByteByteReplaces               44356        44370   +0.03%
BenchmarkByteByteMap                    22273        21531   -3.33%
BenchmarkIndexRune                        606          603   -0.50%
BenchmarkIndexRuneFastPath                451          454   +0.67%
BenchmarkIndex                            499          480   -3.81%
BenchmarkMapNoChanges                    2166         2145   -0.97%
BenchmarkIndexHard1                  20219289     18614626   -7.94%
BenchmarkIndexHard2                  18818291     18609084   -1.11%
BenchmarkIndexHard3                  18784650     18606772   -0.95%
BenchmarkCountHard1                  18776488     18589004   -1.00%
BenchmarkCountHard2                  18633691     18589560   -0.24%
BenchmarkCountHard3                  19331696     18586440   -3.86%
BenchmarkIndexTorture                  104744       104592   -0.15%
BenchmarkCountTorture                  104791       104574   -0.21%
BenchmarkCountTortureOverlapping     80928224     61623014  -23.85%
BenchmarkFields                     171732962    165121290   -3.85%
BenchmarkFieldsFunc                 165616856    165492483   -0.08%
BenchmarkSplit1                     187759567    168792316  -10.10%
BenchmarkSplit2                      48277196     48175434   -0.21%
BenchmarkSplit3                      40313030     39335365   -2.43%

benchmark                            old MB/s     new MB/s  speedup
BenchmarkSingleMaxSkipping             592.64       646.75    1.09x
BenchmarkSingleLongSuffixFail           34.21        35.14    1.03x
BenchmarkSingleMatch                    18.15        20.19    1.11x
BenchmarkFields                          6.11         6.35    1.04x
BenchmarkFieldsFunc                      6.33         6.34    1.00x
220887(~) % cat fmt.txt 
benchmark                      old ns/op    new ns/op    delta
BenchmarkSprintfEmpty                681          646   -5.14%
BenchmarkSprintfString              2417         2438   +0.87%
BenchmarkSprintfInt                 1904         1947   +2.26%
BenchmarkSprintfIntInt              2835         2810   -0.88%
BenchmarkSprintfPrefixedInt         3444         3487   +1.25%
BenchmarkSprintfFloat               4997         4978   -0.38%
BenchmarkManyArgs                  10427        10465   +0.36%
BenchmarkScanInts                7494022      7444609   -0.66%
BenchmarkScanRecursiveInt        8661682      8842178   +2.08%

[dfc, 2013-03-31 03:01:21]

Here is a bit more data from the linxu/arm regression for Count4K

before:

Total: 118 samples
      64  54.2%  54.2%      118 100.0% bytes.Count
      54  45.8% 100.0%       54  45.8% bytes.Equal
       0   0.0% 100.0%       64  54.2% bytes_test.BenchmarkCount4K

http://dave.cheney.net/paste/bytes-before.svg

after:

Total: 180 samples
     133  73.9%  73.9%      139  77.2% bytes.Count
      38  21.1%  95.0%       38  21.1% runtime.memeqbody
       9   5.0% 100.0%      180 100.0% bytes.Equal
       0   0.0% 100.0%      137  76.1% bytes_test.BenchmarkCount4K
       0   0.0% 100.0%       45  25.0% bytes_test.bmCount
       0   0.0% 100.0%        4   2.2% gosched0
       0   0.0% 100.0%        4   2.2% testing.(*B).launch
       0   0.0% 100.0%        4   2.2% testing.(*B).runN

http://dave.cheney.net/paste/bytes-after.svg

[khr1, 2013-03-31 04:14:03]

I don't understand why it is so slow.  A bit slower, maybe, but not that
much.  The main loop in both the old and new implementations for n<8 are
now identical.  (The CountX benchmarks are doing n==7.)  The new
implementation differs just by a few constant moves, a compare and branch,
and a BL (together with its stack pointer bump).

I'm happy to revert to the old code if we can't fix this.  I did the arm
just for completeness, the big wins are in the x86 world.


On Sat, Mar 30, 2013 at 8:01 PM, <dave@cheney.net> wrote:

> Here is a bit more data from the linxu/arm regression for Count4K
>
> before:
>
> Total: 118 samples
>       64  54.2%  54.2%      118 100.0% bytes.Count
>       54  45.8% 100.0%       54  45.8% bytes.Equal
>        0   0.0% 100.0%       64  54.2% bytes_test.BenchmarkCount4K
>
>
http://dave.cheney.net/paste/**bytes-before.svg<http://dave.cheney.net/paste/...
>
> after:
>
> Total: 180 samples
>      133  73.9%  73.9%      139  77.2% bytes.Count
>       38  21.1%  95.0%       38  21.1% runtime.memeqbody
>        9   5.0% 100.0%      180 100.0% bytes.Equal
>        0   0.0% 100.0%      137  76.1% bytes_test.BenchmarkCount4K
>        0   0.0% 100.0%       45  25.0% bytes_test.bmCount
>        0   0.0% 100.0%        4   2.2% gosched0
>        0   0.0% 100.0%        4   2.2% testing.(*B).launch
>        0   0.0% 100.0%        4   2.2% testing.(*B).runN
>
>
http://dave.cheney.net/paste/**bytes-after.svg<http://dave.cheney.net/paste/b...
>
>
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>

[dfc, 2013-03-31 11:15:18]

Your new runtime.memeqbody is much faster than my old byte by byte attempt,
i'm experimenting with copying it directly into bytes.Equals to see if that
causes a regression in bytes.Count.


On Sun, Mar 31, 2013 at 3:14 PM, Keith Randall <khr@google.com> wrote:

> I don't understand why it is so slow.  A bit slower, maybe, but not that
> much.  The main loop in both the old and new implementations for n<8 are
> now identical.  (The CountX benchmarks are doing n==7.)  The new
> implementation differs just by a few constant moves, a compare and branch,
> and a BL (together with its stack pointer bump).
>
> I'm happy to revert to the old code if we can't fix this.  I did the arm
> just for completeness, the big wins are in the x86 world.
>
>
> On Sat, Mar 30, 2013 at 8:01 PM, <dave@cheney.net> wrote:
>
>> Here is a bit more data from the linxu/arm regression for Count4K
>>
>> before:
>>
>> Total: 118 samples
>>       64  54.2%  54.2%      118 100.0% bytes.Count
>>       54  45.8% 100.0%       54  45.8% bytes.Equal
>>        0   0.0% 100.0%       64  54.2% bytes_test.BenchmarkCount4K
>>
>>
http://dave.cheney.net/paste/**bytes-before.svg<http://dave.cheney.net/paste/...
>>
>> after:
>>
>> Total: 180 samples
>>      133  73.9%  73.9%      139  77.2% bytes.Count
>>       38  21.1%  95.0%       38  21.1% runtime.memeqbody
>>        9   5.0% 100.0%      180 100.0% bytes.Equal
>>        0   0.0% 100.0%      137  76.1% bytes_test.BenchmarkCount4K
>>        0   0.0% 100.0%       45  25.0% bytes_test.bmCount
>>        0   0.0% 100.0%        4   2.2% gosched0
>>        0   0.0% 100.0%        4   2.2% testing.(*B).launch
>>        0   0.0% 100.0%        4   2.2% testing.(*B).runN
>>
>>
http://dave.cheney.net/paste/**bytes-after.svg<http://dave.cheney.net/paste/b...
>>
>>
https://codereview.appspot.**com/8056043/<https://codereview.appspot.com/8056...
>>
>
>  --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
>

[dfc, 2013-03-31 11:33:58]

Here is another arm datapoint, from a pandaboard, Cortex-A9. 

alarm(~/go/src/pkg/bytes) % ~/go/misc/benchcmp {old,new}.txt
benchmark                           old ns/op    new ns/op    delta
BenchmarkReadString                    207691       212456   +2.29%
BenchmarkBufferNotEmptyWriteRead     10302655     10189737   -1.10%
BenchmarkBufferFullSmallReads         1472871      1447450   -1.73%
BenchmarkIndexByte32                      164          165   +0.61%
BenchmarkIndexByte4K                    13756        13779   +0.17%
BenchmarkIndexByte4M                 27057480     26680883   -1.39%
BenchmarkIndexByte64M               438272584    428702042   -2.18%
BenchmarkIndexBytePortable32              515          515   +0.00%
BenchmarkIndexBytePortable4K            56701        56541   -0.28%
BenchmarkIndexBytePortable4M         64782309     64326364   -0.70%
BenchmarkIndexBytePortable64M      1031455804   1030296066   -0.11%
BenchmarkEqual32                          188          106  -43.62%
BenchmarkEqual4K                        15506         4636  -70.10%
BenchmarkEqual4M                     45102310     46034946   +2.07%
BenchmarkEqual64M                   743546635   1001547877  +34.70%
BenchmarkEqualPort32                      807          796   -1.36%
BenchmarkEqualPort4K                    92419        90848   -1.70%
BenchmarkEqualPortable4M            106740630    108727363   +1.86%
BenchmarkEqualPortable64M          1774788812   1798989707   +1.36%
BenchmarkIndex32                         2503         2939  +17.42%
BenchmarkIndex4K                       374861       442905  +18.15%
BenchmarkIndex4M                    391445561    458860175  +17.22%
BenchmarkIndex64M                  6260006548   7347176750  +17.37%
BenchmarkIndexEasy32                      330          348   +5.45%
BenchmarkIndexEasy4K                    14001        13923   -0.56%
BenchmarkIndexEasy4M                 26652902     26637547   -0.06%
BenchmarkIndexEasy64M               428365684    428122196   -0.06%
BenchmarkCount32                         2622         2956  +12.74%
BenchmarkCount4K                       384545       444107  +15.49%
BenchmarkCount4M                    398216529    459708583  +15.44%
BenchmarkCount64M                  6369725802   7357919173  +15.51%
BenchmarkCountEasy32                      353          373   +5.67%
BenchmarkCountEasy4K                    13907        13935   +0.20%
BenchmarkCountEasy4M                 26631245     26946392   +1.18%
BenchmarkCountEasy64M               428438891    433169914   +1.10%
BenchmarkFields                     306475738    306250622   -0.07%
BenchmarkFieldsFunc                 305981589    306415421   +0.14%
BenchmarkTrimSpace                        753          733   -2.66%

benchmark                            old MB/s     new MB/s  speedup
BenchmarkReadString                    157.77       154.23    0.98x
BenchmarkIndexByte32                   194.79       193.42    0.99x
BenchmarkIndexByte4K                   297.75       297.24    1.00x
BenchmarkIndexByte4M                   155.01       157.20    1.01x
BenchmarkIndexByte64M                  153.12       156.54    1.02x
BenchmarkIndexBytePortable32            62.12        62.14    1.00x
BenchmarkIndexBytePortable4K            72.24        72.44    1.00x
BenchmarkIndexBytePortable4M            64.74        65.20    1.01x
BenchmarkIndexBytePortable64M           65.06        65.14    1.00x
BenchmarkEqual32                       169.90       300.05    1.77x
BenchmarkEqual4K                       264.16       883.50    3.34x
BenchmarkEqual4M                        93.00        91.11    0.98x
BenchmarkEqual64M                       90.26        67.01    0.74x
BenchmarkEqualPort32                    39.61        40.15    1.01x
BenchmarkEqualPort4K                    44.32        45.09    1.02x
BenchmarkEqualPortable4M                39.29        38.58    0.98x
BenchmarkEqualPortable64M               37.81        37.30    0.99x
BenchmarkIndex32                        12.78        10.89    0.85x
BenchmarkIndex4K                        10.93         9.25    0.85x
BenchmarkIndex4M                        10.71         9.14    0.85x
BenchmarkIndex64M                       10.72         9.13    0.85x
BenchmarkIndexEasy32                    96.80        91.88    0.95x
BenchmarkIndexEasy4K                   292.54       294.18    1.01x
BenchmarkIndexEasy4M                   157.37       157.46    1.00x
BenchmarkIndexEasy64M                  156.66       156.75    1.00x
BenchmarkCount32                        12.20        10.83    0.89x
BenchmarkCount4K                        10.65         9.22    0.87x
BenchmarkCount4M                        10.53         9.12    0.87x
BenchmarkCount64M                       10.54         9.12    0.87x
BenchmarkCountEasy32                    90.47        85.58    0.95x
BenchmarkCountEasy4K                   294.52       293.93    1.00x
BenchmarkCountEasy4M                   157.50       155.65    0.99x
BenchmarkCountEasy64M                  156.64       154.93    0.99x
BenchmarkFields                          3.42         3.42    1.00x
BenchmarkFieldsFunc                      3.43         3.42    1.00x

From my amateur eye, the A9 has a shorter pipeline and is probably taking a
smaller hit from the branch than the A15 in the chromebook.

[bradfitz, 2013-03-31 22:54:49]

On Sat, Mar 30, 2013 at 9:14 PM, Keith Randall <khr@google.com> wrote:

> I don't understand why it is so slow.  A bit slower, maybe, but not that
> much.  The main loop in both the old and new implementations for n<8 are
> now identical.  (The CountX benchmarks are doing n==7.)  The new
> implementation differs just by a few constant moves, a compare and branch,
> and a BL (together with its stack pointer bump).
>
> I'm happy to revert to the old code if we can't fix this.  I did the arm
> just for completeness, the big wins are in the x86 world.
>

Yeah.  I'm in favor of leaving memeq and memeqbody in runtime/asm_arm.s but
deleting or renaming bytes·Equal on arm to bytes·equalTODOIssueNNNNN where
Issue NNNN is a new bug about investigating this issue specifically on ARM.

[dfc, 2013-03-31 23:40:15]

Last night I copied khr's new code back into bytes.Equals, removing all the
indirection and still saw a slowdown in Count. I suggest, for this round revert
bytes.Equals to my crummy version for arm, but keep runtime.memeqbody as it is
an improvement over the old version in alg.c (minux do you agree?) and get this
change in. We can figure out what is going on with arm without blocking the rest
of this change. 

Cheers

Dave



On 01/04/2013, at 9:54 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> 
> 
> 
> On Sat, Mar 30, 2013 at 9:14 PM, Keith Randall <khr@google.com> wrote:
>> I don't understand why it is so slow.  A bit slower, maybe, but not that
much.  The main loop in both the old and new implementations for n<8 are now
identical.  (The CountX benchmarks are doing n==7.)  The new implementation
differs just by a few constant moves, a compare and branch, and a BL (together
with its stack pointer bump).
>> 
>> I'm happy to revert to the old code if we can't fix this.  I did the arm just
for completeness, the big wins are in the x86 world.
> 
> Yeah.  I'm in favor of leaving memeq and memeqbody in runtime/asm_arm.s but
deleting or renaming bytes·Equal on arm to bytes·equalTODOIssueNNNNN where Issue
NNNN is a new bug about investigating this issue specifically on ARM.
> 
> 

[khr1, 2013-04-01 00:17:50]

If the simple implemention really is much faster for short strings, I'm in
favor of both reverting bytes.Equals and using the same simple
implementation in runtime.  Faster equality for long strings is just not
that important.  We can always revisit if we figure out what is going on.



On Sun, Mar 31, 2013 at 4:39 PM, Dave Cheney <dave@cheney.net> wrote:

> Last night I copied khr's new code back into bytes.Equals, removing all
> the indirection and still saw a slowdown in Count. I suggest, for this
> round revert bytes.Equals to my crummy version for arm, but keep
> runtime.memeqbody as it is an improvement over the old version in alg.c
> (minux do you agree?) and get this change in. We can figure out what is
> going on with arm without blocking the rest of this change.
>
> Cheers
>
> Dave
>
>
>
> On 01/04/2013, at 9:54 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
>
>
>
>
> On Sat, Mar 30, 2013 at 9:14 PM, Keith Randall <khr@google.com> wrote:
>
>> I don't understand why it is so slow.  A bit slower, maybe, but not that
>> much.  The main loop in both the old and new implementations for n<8 are
>> now identical.  (The CountX benchmarks are doing n==7.)  The new
>> implementation differs just by a few constant moves, a compare and branch,
>> and a BL (together with its stack pointer bump).
>>
>> I'm happy to revert to the old code if we can't fix this.  I did the arm
>> just for completeness, the big wins are in the x86 world.
>>
>
> Yeah.  I'm in favor of leaving memeq and memeqbody in runtime/asm_arm.s
> but deleting or renaming bytes·Equal on arm to bytes·equalTODOIssueNNNNN
> where Issue NNNN is a new bug about investigating this issue specifically
> on ARM.
>
>
>

[dfc, 2013-04-01 00:21:03]

SGTM. Lets try that. 

On 01/04/2013, at 11:17, Keith Randall <khr@google.com> wrote:

> If the simple implemention really is much faster for short strings, I'm in
favor of both reverting bytes.Equals and using the same simple implementation in
runtime.  Faster equality for long strings is just not that important.  We can
always revisit if we figure out what is going on.
> 
> 
> 
> On Sun, Mar 31, 2013 at 4:39 PM, Dave Cheney <dave@cheney.net> wrote:
>> Last night I copied khr's new code back into bytes.Equals, removing all the
indirection and still saw a slowdown in Count. I suggest, for this round revert
bytes.Equals to my crummy version for arm, but keep runtime.memeqbody as it is
an improvement over the old version in alg.c (minux do you agree?) and get this
change in. We can figure out what is going on with arm without blocking the rest
of this change. 
>> 
>> Cheers
>> 
>> Dave
>> 
>> 
>> 
>> On 01/04/2013, at 9:54 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
>> 
>>> 
>>> 
>>> 
>>> On Sat, Mar 30, 2013 at 9:14 PM, Keith Randall <khr@google.com> wrote:
>>>> I don't understand why it is so slow.  A bit slower, maybe, but not that
much.  The main loop in both the old and new implementations for n<8 are now
identical.  (The CountX benchmarks are doing n==7.)  The new implementation
differs just by a few constant moves, a compare and branch, and a BL (together
with its stack pointer bump).
>>>> 
>>>> I'm happy to revert to the old code if we can't fix this.  I did the arm
just for completeness, the big wins are in the x86 world.
>>> 
>>> Yeah.  I'm in favor of leaving memeq and memeqbody in runtime/asm_arm.s but
deleting or renaming bytes·Equal on arm to bytes·equalTODOIssueNNNNN where Issue
NNNN is a new bug about investigating this issue specifically on ARM.
> 

[khr, 2013-04-02 23:26:20]

*** Submitted as https://code.google.com/p/go/source/detail?r=7f91fab04475 ***

runtime: Implement faster equals for strings and bytes.

(amd64)
benchmark           old ns/op    new ns/op    delta
BenchmarkEqual0            16            6  -63.15%
BenchmarkEqual9            22            7  -65.37%
BenchmarkEqual32           36            9  -74.91%
BenchmarkEqual4K         2187          120  -94.51%

benchmark            old MB/s     new MB/s  speedup
BenchmarkEqual9        392.22      1134.38    2.89x
BenchmarkEqual32       866.72      3457.39    3.99x
BenchmarkEqual4K      1872.73     33998.87   18.15x

(386)
benchmark           old ns/op    new ns/op    delta
BenchmarkEqual0            16            5  -63.85%
BenchmarkEqual9            22            7  -67.84%
BenchmarkEqual32           34           12  -64.94%
BenchmarkEqual4K         2196          113  -94.85%

benchmark            old MB/s     new MB/s  speedup
BenchmarkEqual9        405.81      1260.18    3.11x
BenchmarkEqual32       919.55      2631.21    2.86x
BenchmarkEqual4K      1864.85     36072.54   19.34x

Update issue 3751

R=bradfitz, r, khr, dave, remyoudompheng, fullung, minux.ma, ality
CC=golang-dev
https://codereview.appspot.com/8056043

[dfc, 2013-04-03 01:56:26]

Thanks khr, i'll try to integrate your faster word-at-a-time version into the
asm_arm.s

Cheers

Dave

[albert.strasheim, 2013-04-03 05:43:38]

Since this change has gone in, I'm seeing the following crash on linux/amd64:

go test -v -short -cpu 1,2,4,8,16,256 std

=== RUN TestLargeStringWrites-2
unexpected fault address 0xc2003da008
fatal error: fault
[signal 0xb code=0x2 addr=0xc2003da008 pc=0x40e6fb]

goroutine 57 [running]:
[fp=0xc20042a140] runtime.throw(0x605737)
	/build/go.tip/go/src/pkg/runtime/panic.c:473 +0x67
[fp=0xc20042a158] runtime.sigpanic()
	/build/go.tip/go/src/pkg/runtime/os_linux.c:239 +0xe7
[fp=0xc20042a210] sweepspan(0xc20008e480, 0xf8)
	/build/go.tip/go/src/pkg/runtime/mgc0.c:1625 +0x33b
[fp=0xc20042a288] runtime.parfordo(0xc20008e480)
	/build/go.tip/go/src/pkg/runtime/parfor.c:120 +0x125
[fp=0xc20042a3b8] gc(0x7f52d1892c1c)
	/build/go.tip/go/src/pkg/runtime/mgc0.c:1940 +0x2e3
----- stack segment boundary -----
[fp=0x7f52d1892c30] runtime.gc(0xc200000000)
	/build/go.tip/go/src/pkg/runtime/mgc0.c:1857 +0x11b
[fp=0x7f52d1892c88] runtime.mallocgc(0x12000, 0x100000001, 0x7f5200000001)
	/build/go.tip/go/src/pkg/runtime/zmalloc_linux_amd64.c:101 +0x1e4
[fp=0x7f52d1892cc0] makeslice1(0x4e76e0, 0x11170, 0x11170, 0x7f52d1892d08)
	/build/go.tip/go/src/pkg/runtime/slice.c:61 +0x89
[fp=0x7f52d1892cf0] runtime.makeslice(0x4e76e0, 0x11170, 0x11170, 0xc, 0x11170,
...)
	/build/go.tip/go/src/pkg/runtime/slice.c:34 +0x9a
[fp=0x7f52d1892d28] bytes.makeSlice(0x11170, 0x0, 0x0, 0x0)
	/build/go.tip/go/src/pkg/bytes/buffer.go:191 +0x61
[fp=0x7f52d1892de0] bytes.(*Buffer).grow(0xc20045a310, 0x2710, 0x2)
	/build/go.tip/go/src/pkg/bytes/buffer.go:99 +0x20b
[fp=0x7f52d1892e30] bytes.(*Buffer).WriteString(0xc20045a310, 0xc2001c4000,
0x2710, 0xc20045a310, 0x0, ...)
	/build/go.tip/go/src/pkg/bytes/buffer.go:136 +0x49
[fp=0x7f52d1892ef0] bytes_test.fillString(0xc200481120, 0x54e990, 0x13,
0xc20045a310, 0xc2004cf000, ...)
	/build/go.tip/go/src/pkg/bytes/buffer_test.go:55 +0xe8
[fp=0x7f52d1892f78] bytes_test.TestLargeStringWrites(0xc200481120)
	/build/go.tip/go/src/pkg/bytes/buffer_test.go:178 +0xd5
[fp=0x7f52d1892fa8] testing.tRunner(0xc200481120, 0x608bc8)
	/build/go.tip/go/src/pkg/testing/testing.go:349 +0x8a
[fp=0x7f52d1892fb0] runtime.goexit()
	/build/go.tip/go/src/pkg/runtime/proc.c:1214
created by testing.RunTests
	/build/go.tip/go/src/pkg/testing/testing.go:429 +0x86b

goroutine 1 [chan receive]:
runtime.park(0x409e70, 0xc200339d10, 0x60682a)
	/build/go.tip/go/src/pkg/runtime/proc.c:1167 +0x64
runtime.chanrecv(0x4e8260, 0xc200339cc0, 0x7f52d1884ce0, 0x0, 0x0, ...)
	/build/go.tip/go/src/pkg/runtime/chan.c:366 +0x566
runtime.chanrecv1()
	/build/go.tip/go/src/pkg/runtime/chan.c:458 +0x38
testing.RunTests(0x56e1e8, 0x608b80, 0x33, 0x33, 0x1, ...)
	/build/go.tip/go/src/pkg/testing/testing.go:430 +0x88e
testing.Main(0x56e1e8, 0x608b80, 0x33, 0x33, 0x608700, ...)
	/build/go.tip/go/src/pkg/testing/testing.go:361 +0x8a
main.main()
	bytes/_test/_testmain.go:245 +0x9a
runtime.main()
	/build/go.tip/go/src/pkg/runtime/proc.c:182 +0x92
runtime.goexit()
	/build/go.tip/go/src/pkg/runtime/proc.c:1214

goroutine 2 [syscall]:
runtime.entersyscallblock()
	/build/go.tip/go/src/pkg/runtime/proc.c:1324 +0x16e
runtime.MHeap_Scavenger()
	/build/go.tip/go/src/pkg/runtime/mheap.c:435 +0xee
runtime.goexit()
	/build/go.tip/go/src/pkg/runtime/proc.c:1214
created by runtime.main
	/build/go.tip/go/src/pkg/runtime/proc.c:165
FAIL	bytes	0.158s

[dfc, 2013-04-03 05:47:04]

Go 1.1 will be fast enough[1] without this change. If it's unstable
then my vote would be to revert this change so we have a chance of
publishing a release candidate. Thoughts ?


[1]. https://github.com/davecheney/autobench

On Wed, Apr 3, 2013 at 4:43 PM,  <fullung@gmail.com> wrote:
> Since this change has gone in, I'm seeing the following crash on
> linux/amd64:
>
> go test -v -short -cpu 1,2,4,8,16,256 std
>
> === RUN TestLargeStringWrites-2
> unexpected fault address 0xc2003da008
> fatal error: fault
> [signal 0xb code=0x2 addr=0xc2003da008 pc=0x40e6fb]
>
> goroutine 57 [running]:
> [fp=0xc20042a140] runtime.throw(0x605737)
>         /build/go.tip/go/src/pkg/runtime/panic.c:473 +0x67
> [fp=0xc20042a158] runtime.sigpanic()
>         /build/go.tip/go/src/pkg/runtime/os_linux.c:239 +0xe7
> [fp=0xc20042a210] sweepspan(0xc20008e480, 0xf8)
>         /build/go.tip/go/src/pkg/runtime/mgc0.c:1625 +0x33b
> [fp=0xc20042a288] runtime.parfordo(0xc20008e480)
>         /build/go.tip/go/src/pkg/runtime/parfor.c:120 +0x125
> [fp=0xc20042a3b8] gc(0x7f52d1892c1c)
>         /build/go.tip/go/src/pkg/runtime/mgc0.c:1940 +0x2e3
> ----- stack segment boundary -----
> [fp=0x7f52d1892c30] runtime.gc(0xc200000000)
>         /build/go.tip/go/src/pkg/runtime/mgc0.c:1857 +0x11b
> [fp=0x7f52d1892c88] runtime.mallocgc(0x12000, 0x100000001,
> 0x7f5200000001)
>         /build/go.tip/go/src/pkg/runtime/zmalloc_linux_amd64.c:101 +0x1e4
> [fp=0x7f52d1892cc0] makeslice1(0x4e76e0, 0x11170, 0x11170,
> 0x7f52d1892d08)
>         /build/go.tip/go/src/pkg/runtime/slice.c:61 +0x89
> [fp=0x7f52d1892cf0] runtime.makeslice(0x4e76e0, 0x11170, 0x11170, 0xc,
> 0x11170, ...)
>         /build/go.tip/go/src/pkg/runtime/slice.c:34 +0x9a
> [fp=0x7f52d1892d28] bytes.makeSlice(0x11170, 0x0, 0x0, 0x0)
>         /build/go.tip/go/src/pkg/bytes/buffer.go:191 +0x61
> [fp=0x7f52d1892de0] bytes.(*Buffer).grow(0xc20045a310, 0x2710, 0x2)
>         /build/go.tip/go/src/pkg/bytes/buffer.go:99 +0x20b
> [fp=0x7f52d1892e30] bytes.(*Buffer).WriteString(0xc20045a310,
> 0xc2001c4000, 0x2710, 0xc20045a310, 0x0, ...)
>         /build/go.tip/go/src/pkg/bytes/buffer.go:136 +0x49
> [fp=0x7f52d1892ef0] bytes_test.fillString(0xc200481120, 0x54e990, 0x13,
> 0xc20045a310, 0xc2004cf000, ...)
>         /build/go.tip/go/src/pkg/bytes/buffer_test.go:55 +0xe8
> [fp=0x7f52d1892f78] bytes_test.TestLargeStringWrites(0xc200481120)
>         /build/go.tip/go/src/pkg/bytes/buffer_test.go:178 +0xd5
> [fp=0x7f52d1892fa8] testing.tRunner(0xc200481120, 0x608bc8)
>         /build/go.tip/go/src/pkg/testing/testing.go:349 +0x8a
> [fp=0x7f52d1892fb0] runtime.goexit()
>         /build/go.tip/go/src/pkg/runtime/proc.c:1214
> created by testing.RunTests
>         /build/go.tip/go/src/pkg/testing/testing.go:429 +0x86b
>
> goroutine 1 [chan receive]:
> runtime.park(0x409e70, 0xc200339d10, 0x60682a)
>         /build/go.tip/go/src/pkg/runtime/proc.c:1167 +0x64
> runtime.chanrecv(0x4e8260, 0xc200339cc0, 0x7f52d1884ce0, 0x0, 0x0, ...)
>         /build/go.tip/go/src/pkg/runtime/chan.c:366 +0x566
> runtime.chanrecv1()
>         /build/go.tip/go/src/pkg/runtime/chan.c:458 +0x38
> testing.RunTests(0x56e1e8, 0x608b80, 0x33, 0x33, 0x1, ...)
>         /build/go.tip/go/src/pkg/testing/testing.go:430 +0x88e
> testing.Main(0x56e1e8, 0x608b80, 0x33, 0x33, 0x608700, ...)
>         /build/go.tip/go/src/pkg/testing/testing.go:361 +0x8a
> main.main()
>         bytes/_test/_testmain.go:245 +0x9a
> runtime.main()
>         /build/go.tip/go/src/pkg/runtime/proc.c:182 +0x92
> runtime.goexit()
>         /build/go.tip/go/src/pkg/runtime/proc.c:1214
>
> goroutine 2 [syscall]:
> runtime.entersyscallblock()
>         /build/go.tip/go/src/pkg/runtime/proc.c:1324 +0x16e
> runtime.MHeap_Scavenger()
>         /build/go.tip/go/src/pkg/runtime/mheap.c:435 +0xee
> runtime.goexit()
>         /build/go.tip/go/src/pkg/runtime/proc.c:1214
> created by runtime.main
>         /build/go.tip/go/src/pkg/runtime/proc.c:165
> FAIL    bytes   0.158s
>
> https://codereview.appspot.com/8056043/

[albert.strasheim, 2013-04-03 05:51:22]

On 2013/04/03 05:47:04, dfc wrote:
> Go 1.1 will be fast enough[1] without this change. If it's unstable
> then my vote would be to revert this change so we have a chance of
> publishing a release candidate. Thoughts ?

Without looking past the stack trace, it seems like it might be a GC issue
though, in which case reverting this isn't going to fix the underlying
problem...

Always forward, never back! :-)

Cheers

Albert

[remyoudompheng, 2013-04-03 05:53:53]

On 3 Apr 2013 07:47, "Dave Cheney" <dave@cheney.net> wrote:
>
> Go 1.1 will be fast enough[1] without this change. If it's unstable
> then my vote would be to revert this change so we have a chance of
> publishing a release candidate. Thoughts ?
>
>

It could be wise but since it's absolutely unclear why the GC is affected a
proper investigation is needed.

If in the end the change is reverted I would like to keep the new test code.

Rémy

[remyoudompheng, 2013-04-03 06:33:05]

On 2013/4/3  <fullung@gmail.com> wrote:
> On 2013/04/03 05:47:04, dfc wrote:
>>
>> Go 1.1 will be fast enough[1] without this change. If it's unstable
>> then my vote would be to revert this change so we have a chance of
>> publishing a release candidate. Thoughts ?
>
>
> Without looking past the stack trace, it seems like it might be a GC
> issue though, in which case reverting this isn't going to fix the
> underlying problem...

Please try https://codereview.appspot.com/8300044
I'm surprised the test could possibly work for anyone.
[maybe the better fix is to check the error returned by syscall.Mprotect]

Rémy.

[dfc, 2013-04-03 07:35:51]

Wow, great catch!

On Wed, Apr 3, 2013 at 5:33 PM, Rémy Oudompheng
<remyoudompheng@gmail.com> wrote:
> On 2013/4/3  <fullung@gmail.com> wrote:
>> On 2013/04/03 05:47:04, dfc wrote:
>>>
>>> Go 1.1 will be fast enough[1] without this change. If it's unstable
>>> then my vote would be to revert this change so we have a chance of
>>> publishing a release candidate. Thoughts ?
>>
>>
>> Without looking past the stack trace, it seems like it might be a GC
>> issue though, in which case reverting this isn't going to fix the
>> underlying problem...
>
> Please try https://codereview.appspot.com/8300044
> I'm surprised the test could possibly work for anyone.
> [maybe the better fix is to check the error returned by syscall.Mprotect]
>
> Rémy.