gmane.comp.lang.go.devel

code review 6255052: time: speed up Format by making formatting machines (issue 6255052)

2012-05-26T23:14:50

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
time: speed up Format by making formatting machines

FOR DISCUSSION.

Before:
BenchmarkFormat     1000000   2348 ns/op
8.3% of HTTP server benchmark.

After:
BenchmarkFormat     1000000   1577 ns/op
BenchmarkFormatter  1000000   1383 ns/op
5.6% of HTTP server benchmark.

I could imagine exposing the *time.Formatter type as being
offensive.  I could also see the automatic caching of
Formatters in Time.Format being offensive, but without that,
BenchmarkFormat gets twice as slow (building the machine is
slower than the old Format code)

Please review this at http://codereview.appspot.com/6255052/

Affected files:
   M src/pkg/net/http/server.go
   M src/pkg/time/format.go
   M src/pkg/time/time_test.go

code review 6254055: runtime: make go work on netbsd/386 (issue 6254055)

2012-05-26T13:19:48

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev< at >googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
runtime: make go work on netbsd/386

Please review this at http://codereview.appspot.com/6254055/

Affected files:
   M src/pkg/runtime/signal_netbsd_386.c
   M src/pkg/runtime/sys_netbsd_386.s
   M src/pkg/runtime/sys_netbsd_amd64.s


Index: src/pkg/runtime/signal_netbsd_386.c
===================================================================
--- a/src/pkg/runtime/signal_netbsd_386.c
+++ b/src/pkg/runtime/signal_netbsd_386.c
< at >< at > -7,6 +7,7 < at >< at >
  #include "signals_GOOS.h"
  #include "os_GOOS.h"

+extern void runtime·lwp_tramp(void);
  extern void runtime·sigtramp(void);

  typedef struct sigaction {
< at >< at > -136,3 +137,13 < at >< at >
  sa._sa_u._sa_sigaction = (void*)fn;
  runtime·sigaction(i, &sa, nil);
  }
+
+void
+runtime·lwp_mcontext_init(McontextT *mc, void *stack, M *m, G *g, void  
(*fn)(void))
+{
+mc->_

code review 6243059: runtime: optimize the symbol table (issue 6243059)

2012-05-26T11:35:33

Reviewers: rsc,

Message:
Hello rsc< at >golang.org (cc: golang-dev< at >googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
runtime: optimize the symbol table

Please review this at http://codereview.appspot.com/6243059/

Affected files:
   M src/pkg/runtime/symtab.c


Index: src/pkg/runtime/symtab.c
===================================================================
--- a/src/pkg/runtime/symtab.c
+++ b/src/pkg/runtime/symtab.c
< at >< at > -16,6 +16,7 < at >< at >
  #include "defs_GOOS_GOARCH.h"
  #include "os_GOOS.h"
  #include "arch_GOARCH.h"
+#include "malloc.h"

  extern byte pclntab[], epclntab[], symtab[], esymtab[];

< at >< at > -28,6 +29,11 < at >< at >
  //byte *gotype;
  };

+// A dynamically allocated string containing multiple substrings.
+// Individual strings are slices of hugestring.
+static String hugestring;
+static int32 hugestring_len;
+
  // Walk over symtab, calling fn(&s) for each symbol.
  static void
  walksymtab(void (*fn)(Sym*))
< at >< at > -135

code review 6257059: path/filepath: Implement documented SkipDir behavior (issue 6257059)

2012-05-26T11:25:20

Reviewers: rsc,

Message:
Hello rsc-iFWiy5xATs8dnm+yROfE0A< at >public.gmane.org (cc: golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

I'd like you to review this change to
https://code.google.com/p/go


Description:
path/filepath: Implement documented SkipDir behavior

Currently walk() doesn't check for err == SkipDir when iterating
a directory list, but such promise is made in the docs for WalkFunc.

Fixes issue 3486.

Please review this at http://codereview.appspot.com/6257059/

Affected files:
   M src/pkg/path/filepath/path.go


Index: src/pkg/path/filepath/path.go
===================================================================
--- a/src/pkg/path/filepath/path.go
+++ b/src/pkg/path/filepath/path.go
< at >< at > -320,7 +320,7 < at >< at >
  }

  for _, fileInfo := range list {
-if err = walk(Join(path, fileInfo.Name()), fileInfo, walkFn); err != nil  
{
+if err = walk(Join(path, fileInfo.Name()), fileInfo, walkFn); err != nil  
&& err != SkipDir {
  return err
  }
  }

code review 6242061: runtime: update field types in preparation for GC changes (issue 6242061)

2012-05-26T08:30:30

Reviewers: rsc,

Message:
Hello rsc-iFWiy5xATs8dnm+yROfE0A< at >public.gmane.org (cc: golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
runtime: update field types in preparation for GC changes

Please review this at http://codereview.appspot.com/6242061/

Affected files:
   M src/pkg/runtime/cgocall.c
   M src/pkg/runtime/mgc0.c
   M src/pkg/runtime/mprof.goc
   M src/pkg/runtime/proc.c
   M src/pkg/runtime/runtime.h
   M src/pkg/runtime/thread_linux.c
   M src/pkg/runtime/traceback_x86.c

code review 6258050: cmd/cgo: give error if imported "C" but not used (issue 6258050)

2012-05-25T17:55:04

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org (cc: golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

I'd like you to review this change to
https://code.google.com/p/go/


Description:
cmd/cgo: give error if imported "C" but not used
     runtime/cgo is a special case, so it uses a workaround.

Please review this at http://codereview.appspot.com/6258050/

Affected files:
   M src/cmd/cgo/ast.go
   M src/pkg/runtime/cgo/cgo.go


Index: src/cmd/cgo/ast.go
===================================================================
--- a/src/cmd/cgo/ast.go
+++ b/src/cmd/cgo/ast.go
< at >< at > -58,6 +58,7 < at >< at >

  // In ast1, find the import "C" line and get any extra C preamble.
  sawC := false
+var importCPos token.Pos
  for _, decl := range ast1.Decls {
  d, ok := decl.(*ast.GenDecl)
  if !ok {
< at >< at > -69,6 +70,7 < at >< at >
  continue
  }
  sawC = true
+importCPos = s.Path.Pos()
  if s.Name != nil {
  error_(s.Path.Pos(), `cannot rename import "C"`)

wanna atomic func..

まにらふ — 2012-05-25T12:42:55

4 exmple

a:=int{1,2,3,4,5}
b:=int{3,2,4}

for _,i:=range a|b {
  fmt.printf(%d)            // stdout is 1 2 3 4 5
}

for _,i:=range a&b{
  fmt.printf(%d)            // stdout is 2 3 4
}

if its logical array function, very useful..

wanna atomic func..

まにらふ — 2012-05-25T12:45:47

4 exmple

a:=[]int{1,2,3,4,5}
b:=[]int{3,2,4}

for _,i:=range a|b {
  fmt.printf(%d)            // stdout is 1 2 3 4 5
}

for _,i:=range a&b{
  fmt.printf(%d)            // stdout is 2 3 4
}

if its logical array function, very useful..

Showstoppers with cross-compilation to TI C6x architecture

Timon ter Braak — 2012-05-25T09:53:38

I have a cross-compilation toolchain (derived from codesourcery) towards 
the C6x architecture from TI ( a HOST to C6x-uclibc chain).
The toolchain is able to produce C,C++ compilers.

I started this effort to attempt to run Go on this multi-core DSP.
I know that C6x is a specialized architecture and freshly supported. But 
I would like to use Go on it for it's robustness and the convenience of 
Go operating in a heterogeneous platform.

Starting with GCC 4.7, both Go and C6x are supported. However, the 
combination of these two is not. I will outline my findings, and would 
like some feedback whether this combo is a nogo ;) or that I should 
invest some porting effort to do so.

Currently, Go builds upon two pillars that are currently prohibiting the 
build:

* libffi:
   the C6x architecture is not available within libffi
   the runtime can be build without libffi support, at the cost of 
broken reflection support; we could port libffi to the C6x architecture 
to fix this. So, for an initial attempt this i

code review 6248049: cmd/6g, cmd/8g: move panicindex calls out of line (issue 6248049)

2012-05-25T06:44:59

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
cmd/6g, cmd/8g: move panicindex calls out of line

The old code generated for a bounds check was
                 CMP
                 JLT ok
                 CALL panicindex
         ok:
                 ...

The new code is (once the linker finishes with it):
                 CMP
                 JGE panic
                 ...
         panic:
                 CALL panicindex

which moves the calls out of line, putting more useful
code in each cache line.  This matters especially in tight
loops, such as in Fannkuch.  The benefit is more modest
elsewhere, but real.

 From test/bench/go1, amd64:

benchmark                old ns/op    new ns/op    delta
BenchmarkBinaryTree17   6096092000   6088808000   -0.12%
BenchmarkFannkuch11     6151404000   4020463000  -34.64%
BenchmarkGobDecode        28990050     28894630   -0.33%

code review 6248048: net/rpc: replace else if with an else (issue 6248048)

2012-05-25T05:03:30

Reviewers: r,

Message:
Hello r< at >golang.org (cc: golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
net/rpc: replace else if with an else

CL 5956051 used an else if condition, which is unnecessary.
If the top condition didn't hit, then we already know that
response.Error == "".

Please review this at http://codereview.appspot.com/6248048/

Affected files:
   M src/pkg/net/rpc/client.go


Index: src/pkg/net/rpc/client.go
===================================================================
--- a/src/pkg/net/rpc/client.go
+++ b/src/pkg/net/rpc/client.go
< at >< at > -127,7 +127,7 < at >< at >
  if err != nil {
  err = errors.New("reading error body: " + err.Error())
  }
-} else if response.Error == "" {
+} else {
  err = client.codec.ReadResponseBody(call.Reply)
  if err != nil {
  call.Error = errors.New("reading body " + err.Error())

ab is uh, weird

Brad Fitzpatrick — 2012-05-25T04:26:09

Playing with ab against Go's http server, I wasted some time debugging some
failures like this (on OS X 10.7):

$ ab -c 5 -n 100 -v 0 http://localhost:8080/hello
This is ApacheBench, Version 2.3 <$Revision: 655654 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking localhost (be patient)...apr_socket_recv: Connection reset by
peer (54)


Look what ab is sending, though!

In one TCP connection (in one packet, even) it's sending multiple pipelined
(!!) HTTP/1.0 (!!) requests, without Connection: keep-alive, even. (true, I
didn't specify -k to ab)

So Go's doing the right thing and closing the connection after the first
one, but still.... this is stupid.  People use this piece of crap software
to benchmark webservers and compare webservers against each other?

Or maybe the ab version that comes with OS X just sucks?

*sigh*

code review 6259043: crypto: amd64 assembly for hashes (issue 6259043)

2012-05-25T04:15:58

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
crypto: amd64 assembly for hashes

Tuning the Go versions was a good source of things to fix
in the compiler, but ultimately the assembly is going to win.
Can't beat 'em, so join 'em.

The good news is that the compiled code size goes down.

benchmark                  old MB/s     new MB/s  speedup
md5.BenchmarkHash8K          359.22       599.04    1.67x
sha1.BenchmarkHash8K          87.31       405.65    4.65x
sha256.BenchmarkHash8K        51.87       185.23    3.57x
sha512.BenchmarkHash8K        79.28       288.15    3.63x

Please review this at http://codereview.appspot.com/6259043/

Affected files:
   A src/pkg/crypto/gen.sh
   A src/pkg/crypto/md5/asm.go
   A src/pkg/crypto/md5/block_amd64.s
   M src/pkg/crypto/md5/gen.go
   M src/pkg/crypto/md5/md5.go
   M src/pkg/crypto/md5/md5block.go
   A src/pkg/crypto/sha1/a

code review 6258049: cmd/8c: better fix for 64-bit register smash (issue 6258049)

2012-05-25T03:36:24

Reviewers: ken2,

Message:
Hello ken2 (cc: golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
cmd/8c: better fix for 64-bit register smash

Ken pointed out that CL 5998043 was ugly code.
This should be better.

Fixes issue 3501.

Please review this at http://codereview.appspot.com/6258049/

Affected files:
   M src/cmd/8c/cgen.c
   M src/cmd/8c/cgen64.c


Index: src/cmd/8c/cgen.c
===================================================================
--- a/src/cmd/8c/cgen.c
+++ b/src/cmd/8c/cgen.c
< at >< at > -1703,6 +1703,7 < at >< at >
  }
  }

+v = w == 8;
  if(n->complex >= FNX && nn != nil && nn->complex >= FNX) {
  t = nn->type;
  nn->type = types[TLONG];
< at >< at > -1728,8 +1729,28 < at >< at >
  }

  x = 0;
-v = w == 8;
  if(v) {
+if(nn != nil && nn->complex >= FNX) {
+t = nn->type;
+nn->type = types[TLONG];
+regialloc(&nod2, nn, Z);
+lcgen(nn, &nod2);
+nn->type = t;
+
+nod2.type = typ(TIND, t);
+
+nod1 = nod2;
+n

code review 6242056: image/png: optimize the paeth filter implementation. (issue 6242056)

2012-05-25T03:09:25

Reviewers: rsc,

Message:
Hello rsc-iFWiy5xATs8dnm+yROfE0A< at >public.gmane.org (cc: golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
image/png: optimize the paeth filter implementation.

image/png benchmarks:
benchmark                       old ns/op    new ns/op    delta
BenchmarkPaeth                         10            7  -29.21%
BenchmarkDecodeGray               2381745      2241620   -5.88%
BenchmarkDecodeNRGBAGradient      9535555      8835100   -7.35%
BenchmarkDecodeNRGBAOpaque        8189590      7611865   -7.05%
BenchmarkDecodePaletted           1300688      1301940   +0.10%
BenchmarkDecodeRGB                6760146      6317082   -6.55%
BenchmarkEncodePaletted           6048596      6122666   +1.22%
BenchmarkEncodeRGBOpaque         18891140     19474230   +3.09%
BenchmarkEncodeRGBA              78945350     78552600   -0.50%

Wall time for Denis Cheremisov's PNG-decoding program given in
https://groups.google.com

code review 6252048: cmd/gc: fix parallel assignment in range (issue 6252048)

2012-05-25T03:05:34

Reviewers: ken2,

Message:
Hello ken2 (cc: golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
cmd/gc: fix parallel assignment in range

for expr1, expr2 = range slice
was assigning to expr1 and expr2 in sequence
instead of in parallel.  Now it assigns in parallel,
as it should.  This matters for things like
for i, x[i] = range slice.

Fixes issue 3464.

Please review this at http://codereview.appspot.com/6252048/

Affected files:
   M src/cmd/gc/range.c
   M src/cmd/gc/subr.c
   M test/range.go


Index: src/cmd/gc/range.c
===================================================================
--- a/src/cmd/gc/range.c
+++ b/src/cmd/gc/range.c
< at >< at > -152,9 +152,14 < at >< at >
  n->ntest = nod(OLT, hv1, hn);
  n->nincr = nod(OASOP, hv1, nodintconst(1));
  n->nincr->etype = OADD;
-body = list1(nod(OAS, v1, hv1));
-if(v2) {
-body = list(body, nod(OAS, v2, nod(OIND, hp, N)));
+if(v2 == N)
+body = list1(nod(OAS, v1, hv1));
+el

code review 6250056: misc/dist: use archive/tar.FileInfoHeader (issue 6250056)

2012-05-25T00:17:30

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

I'd like you to review this change to
https://go.googlecode.com/hg


Description:
misc/dist: use archive/tar.FileInfoHeader

Fixes issue 3299

Please review this at http://codereview.appspot.com/6250056/

Affected files:
   M misc/dist/bindist.go
   R misc/dist/stat_darwin.go
   R misc/dist/stat_linux.go

code review 6257050: C: Bill Thiede (Google CLA) (issue 6257050)

2012-05-24T23:15:41

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
C: Bill Thiede (Google CLA)

Please review this at http://codereview.appspot.com/6257050/

Affected files:
   M CONTRIBUTORS


Index: CONTRIBUTORS
===================================================================
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
< at >< at > -68,6 +68,7 < at >< at >
  Benny Siegert <bsiegert-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org>
  Berengar Lehr <Berengar.Lehr-Mmb7MZpHnFY< at >public.gmane.org>
  Bill Neubauer <wcn-iFWiy5xATs8dnm+yROfE0A< at >public.gmane.org> <wcn-hpIqsD4AKlfQT0dZR+AlfA< at >public.gmane.org>
+Bill Thiede <couchmoney-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org>
  Bjorn Tillenius <bjorn-dSjf3tBqmUzwXD0AkxfrAg< at >public.gmane.org>
  Bjorn Tipling <bjorn.tipling-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org>
  Blake Mizerany <blake.mizerany-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org>

code review 6244051: syscall: regenerate z-files for FreeBSD 9.0-STABLE (issue 6244051)

2012-05-24T22:54:48

Reviewers: mikioh, dho,

Message:
Hello mikioh.mikioh-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org, devon.odell-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org (cc:
golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

I'd like you to review this change to
https://code.google.com/p/go


Description:
syscall: regenerate z-files for FreeBSD 9.0-STABLE

There are changes in archive/tar and os due to the changes in the
struct syscall.Stat_t.

Please review this at http://codereview.appspot.com/6244051/

Affected files:
   M src/pkg/archive/tar/stat_atim.go
   M src/pkg/archive/tar/stat_atimespec.go
   M src/pkg/os/stat_freebsd.go
   M src/pkg/syscall/mkerrors.sh
   M src/pkg/syscall/zerrors_freebsd_386.go
   M src/pkg/syscall/zerrors_freebsd_amd64.go
   M src/pkg/syscall/ztypes_freebsd_386.go
   M src/pkg/syscall/ztypes_freebsd_amd64.go

code review 6252047: strconv: better documentation for FormatInt, FormatUint. (issue 6252047)

2012-05-24T22:54:03

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

I'd like you to review this change to
https://code.google.com/p/go


Description:
strconv: better documentation for FormatInt, FormatUint.

Fixes issue 3580.

Please review this at http://codereview.appspot.com/6252047/

Affected files:
   M src/pkg/strconv/itoa.go


Index: src/pkg/strconv/itoa.go
===================================================================
--- a/src/pkg/strconv/itoa.go
+++ b/src/pkg/strconv/itoa.go
< at >< at > -4,13 +4,17 < at >< at >

  package strconv

-// FormatUint returns the string representation of i in the given base.
+// FormatUint returns the string representation of i in the given base,
+// for 2 <= base <= 36. The result uses the lowercase letters 'a' to 'z'
+// for digit values >= 10.
  func FormatUint(i uint64, base int) string {
  _, s := formatBits(nil, i, base, false, false)
  return s
  }

-// FormatInt returns the string representation of i in the given base.
+// FormatInt r

code review 6215078: runtime: handle and test large map values (issue 6215078)

2012-05-24T21:54:42

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev< at >googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
runtime: handle and test large map values

This is from CL 5451105 but was dropped from that CL.
See also CL 6137051.

The only change compared to 5451105 is to check for
h != nil in reflect·mapiterinit; allowing use of nil maps
must have happened after that original CL.

Fixes issue 3573.

Please review this at http://codereview.appspot.com/6215078/

Affected files:
   M src/pkg/runtime/hashmap.c
   M test/bigmap.go