1. 24 Oct, 2014 3 commits
    • Russ Cox's avatar
      cmd/gc: synthesize zeroed value for non-assignment context · 5225854b
      Russ Cox authored
      CL 157910047 introduced code to turn a node representing
      a zeroed composite literal into N, the nil Node* pointer
      (which represents any zero, not the Go literal nil).
      
      That's great for assignments like x = T{}, but it doesn't work
      when T{} is used in a value context like T{}.v or x == T{}.
      Fix those.
      
      Should have no effect on performance; confirmed.
      The deltas below are noise (compare ns/op):
      
      benchmark                          old ns/op      new ns/op      delta
      BenchmarkBinaryTree17              2902919192     2915228424     +0.42%
      BenchmarkFannkuch11                2597417605     2630363685     +1.27%
      BenchmarkFmtFprintfEmpty           73.7           74.8           +1.49%
      BenchmarkFmtFprintfString          196            199            +1.53%
      BenchmarkFmtFprintfInt             213            217            +1.88%
      BenchmarkFmtFprintfIntInt          336            356            +5.95%
      BenchmarkFmtFprintfPrefixedInt     289            294            +1.73%
      BenchmarkFmtFprintfFloat           415            416            +0.24%
      BenchmarkFmtManyArgs               1281           1271           -0.78%
      BenchmarkGobDecode                 10271734       10307978       +0.35%
      BenchmarkGobEncode                 8985021        9079442        +1.05%
      BenchmarkGzip                      410233227      412266944      +0.50%
      BenchmarkGunzip                    102114554      103272443      +1.13%
      BenchmarkHTTPClientServer          45297          44993          -0.67%
      BenchmarkJSONEncode                19499741       19498489       -0.01%
      BenchmarkJSONDecode                76436733       74247497       -2.86%
      BenchmarkMandelbrot200             4273814        4307292        +0.78%
      BenchmarkGoParse                   4024594        4028937        +0.11%
      BenchmarkRegexpMatchEasy0_32       131            135            +3.05%
      BenchmarkRegexpMatchEasy0_1K       328            333            +1.52%
      BenchmarkRegexpMatchEasy1_32       115            117            +1.74%
      BenchmarkRegexpMatchEasy1_1K       931            948            +1.83%
      BenchmarkRegexpMatchMedium_32      216            217            +0.46%
      BenchmarkRegexpMatchMedium_1K      72669          72857          +0.26%
      BenchmarkRegexpMatchHard_32        3818           3809           -0.24%
      BenchmarkRegexpMatchHard_1K        121398         121945         +0.45%
      BenchmarkRevcomp                   613996550      615145436      +0.19%
      BenchmarkTemplate                  93678525       93267391       -0.44%
      BenchmarkTimeParse                 414            411            -0.72%
      BenchmarkTimeFormat                396            399            +0.76%
      
      Fixes #8947.
      
      LGTM=r
      R=r, dave
      CC=golang-codereviews
      https://golang.org/cl/162130043
      5225854b
    • Russ Cox's avatar
      doc/go1.4: encoding/csv · 737a9e0d
      Russ Cox authored
      CC=golang-codereviews
      https://golang.org/cl/162140043
      737a9e0d
    • Russ Cox's avatar
      encoding/csv: for Postgres, unquote empty strings, quote \. · 6ad2749d
      Russ Cox authored
      In theory both of these lines encode the same three fields:
      
              a,,c
              a,"",c
      
      However, Postgres defines that when importing CSV, the unquoted
      version is treated as NULL (missing), while the quoted version is
      treated as a string value (empty string). If the middle field is supposed to
      be an integer value, the first line can be imported (NULL is okay), but
      the second line cannot (empty string is not).
      
      Postgres's import command (COPY FROM) has an option to force
      the unquoted empty to be interpreted as a string but it does not
      have an option to force the quoted empty to be interpreted as a NULL.
      
      From http://www.postgresql.org/docs/9.0/static/sql-copy.html:
      
              The CSV format has no standard way to distinguish a NULL
              value from an empty string. PostgreSQL's COPY handles this
              by quoting. A NULL is output as the NULL parameter string
              and is not quoted, while a non-NULL value matching the NULL
              parameter string is quoted. For example, with the default
              settings, a NULL is written as an unquoted empty string,
              while an empty string data value is written with double
              quotes (""). Reading values follows similar rules. You can
              use FORCE_NOT_NULL to prevent NULL input comparisons for
              specific columns.
      
      Therefore printing the unquoted empty is more flexible for
      imports into Postgres than printing the quoted empty.
      
      In addition to making the output more useful with Postgres, not
      quoting empty strings makes the output smaller and easier to read.
      It also matches the behavior of Microsoft Excel and Google Drive.
      
      Since we are here and making concessions for Postgres, handle this
      case too (again quoting the Postgres docs):
      
              Because backslash is not a special character in the CSV
              format, \., the end-of-data marker, could also appear as a
              data value. To avoid any misinterpretation, a \. data value
              appearing as a lone entry on a line is automatically quoted
              on output, and on input, if quoted, is not interpreted as
              the end-of-data marker. If you are loading a file created by
              another application that has a single unquoted column and
              might have a value of \., you might need to quote that value
              in the input file.
      
      Fixes #7586.
      
      LGTM=bradfitz
      R=bradfitz
      CC=golang-codereviews
      https://golang.org/cl/164760043
      6ad2749d
  2. 23 Oct, 2014 2 commits
  3. 22 Oct, 2014 4 commits
    • Dmitriy Vyukov's avatar
      sync: release Pool memory during second and later GCs · af3868f1
      Dmitriy Vyukov authored
      Pool memory was only being released during the first GC after the first Put.
      
      Put assumes that p.local != nil means p is on the allPools list.
      poolCleanup (called during each GC) removed each pool from allPools
      but did not clear p.local, so each pool was cleared by exactly one GC
      and then never cleared again.
      
      This bug was introduced late in the Go 1.3 release cycle.
      
      Fixes #8979.
      
      LGTM=rsc
      R=golang-codereviews, bradfitz, r, rsc
      CC=golang-codereviews, khr
      https://golang.org/cl/162980043
      af3868f1
    • Ian Lance Taylor's avatar
      test: add more cases to recover.go · 18051c08
      Ian Lance Taylor authored
      test16 used to fail with gccgo.  The withoutRecoverRecursive
      test would have failed in some possible implementations.
      
      LGTM=bradfitz
      R=golang-codereviews, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/151630043
      18051c08
    • Russ Cox's avatar
      CONTRIBUTORS: add Austin Clements's google.com email (Google CLA) · ecd1cc17
      Russ Cox authored
      LGTM=bradfitz, austin
      R=austin
      CC=bradfitz, golang-codereviews
      https://golang.org/cl/158330045
      ecd1cc17
    • Dave Cheney's avatar
      runtime/cgo: encode BLX directly, fixes one clang build error on arm · d1b29137
      Dave Cheney authored
      Fixes #8348.
      
      Trying to work around clang's dodgy support for .arch by reverting to the external assembler didn't work out so well. Minux had a much better solution to encode the instructions we need as .word directives which avoids .arch altogether.
      
      I've confirmed with gdb that this form produces the expected machine code
      
      Dump of assembler code for function crosscall_arm1:
         0x00000000 <+0>:	push	{r4, r5, r6, r7, r8, r9, r10, r11, r12, lr}
         0x00000004 <+4>:	mov	r4, r0
         0x00000008 <+8>:	mov	r5, r1
         0x0000000c <+12>:	mov	r0, r2
         0x00000010 <+16>:	blx	r5
         0x00000014 <+20>:	blx	r4
         0x00000018 <+24>:	pop	{r4, r5, r6, r7, r8, r9, r10, r11, r12, pc}
      
      There is another compilation failure that blocks building Go with clang on arm
      
      # ../misc/cgo/test
      # _/home/dfc/go/misc/cgo/test
      /tmp/--407b12.s: Assembler messages:
      /tmp/--407b12.s:59: Error: selected processor does not support ARM mode `blx r0'
      clang: error: assembler command failed with exit code 1 (use -v to see invocation)
      FAIL	_/home/dfc/go/misc/cgo/test [build failed]
      
      I'll open a new issue for that
      
      LGTM=iant
      R=iant, minux
      CC=golang-codereviews
      https://golang.org/cl/158180047
      d1b29137
  4. 21 Oct, 2014 7 commits
  5. 20 Oct, 2014 16 commits
    • Dave Cheney's avatar
      cmd/cgo: disable clang's integrated assembler · cf9558c8
      Dave Cheney authored
      Fixes #8348.
      
      Clang's internal assembler (introduced by default in clang 3.4) understands the .arch directive, but doesn't change the default value of -march. This causes the build to fail when we use BLX (armv5 and above) when clang is compiled for the default armv4t architecture (which appears to be the default on all the distros I've used).
      
      This is probably a clang bug, so work around it for the time being by disabling the integrated assembler when compiling the cgo assembly shim.
      
      This CL also includes a small change to ldelf.c which was required as clang 3.4 and above generate more weird symtab entries.
      
      LGTM=iant
      R=golang-codereviews, iant
      CC=golang-codereviews
      https://golang.org/cl/156430044
      cf9558c8
    • Alex Brainman's avatar
      debug/pe: use appropriate type for sizeofOptionalHeader32 · e5383c68
      Alex Brainman authored
      LGTM=rsc
      R=golang-codereviews, rsc
      CC=golang-codereviews
      https://golang.org/cl/157220043
      e5383c68
    • Keith Randall's avatar
      runtime: fix flaky TestBlockProfile test · 3ec8fe45
      Keith Randall authored
      It has been failing periodically on Solaris/x64.
      Change blockevent so it always records an event if we called
      SetBlockProfileRate(1), even if the time delta is negative or zero.
      
      Hopefully this will fix the test on Solaris.
      Caveat: I don't actually know what the Solaris problem is, this
      is just an educated guess.
      
      LGTM=dave
      R=dvyukov, dave
      CC=golang-codereviews
      https://golang.org/cl/159150043
      3ec8fe45
    • David du Colombier's avatar
      runtime: handle non-nil-terminated environment strings on Plan 9 · 9d06cfc8
      David du Colombier authored
      Russ Cox pointed out that environment strings are not
      required to be nil-terminated on Plan 9.
      
      LGTM=rsc
      R=rsc
      CC=golang-codereviews
      https://golang.org/cl/159130044
      9d06cfc8
    • David du Colombier's avatar
      os/exec: fix number of expected file descriptors on Plan 9 · 1946afb6
      David du Colombier authored
      Since CL 104570043 and 112720043, we are using the
      nsec system call instead of /dev/bintime on Plan 9.
      
      LGTM=rsc
      R=rsc
      CC=aram, golang-codereviews
      https://golang.org/cl/155590043
      1946afb6
    • Rob Pike's avatar
      flag: roll back 156390043 (flag setting) · 9070afb3
      Rob Pike authored
      Shell scripts depend on the old behavior too often.
      It's too late to make this change.
      
      LGTM=bradfitz
      R=rsc, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/161890044
      9070afb3
    • Rob Pike's avatar
      cmd/go: set exit status for failing "go generate" run. · c57cb786
      Rob Pike authored
      LGTM=rsc
      R=rsc
      CC=golang-codereviews
      https://golang.org/cl/154360048
      c57cb786
    • Ian Lance Taylor's avatar
      reflect: fix TestAllocations now that interfaces hold only pointers · 82a0188c
      Ian Lance Taylor authored
      This test was failing but did not break the build because it
      was not run when -test.short was used.
      
      LGTM=bradfitz
      R=golang-codereviews, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/157150043
      82a0188c
    • Ian Lance Taylor's avatar
      reflect: allocate correct type in assignTo and cvtT2I · 7b9c5ec2
      Ian Lance Taylor authored
      I came across this while debugging a GC problem in gccgo.
      There is code in assignTo and cvtT2I that handles assignment
      to all interface values.  It allocates an empty interface even
      if the real type is a non-empty interface.  The fields are
      then set for a non-empty interface, but the memory is recorded
      as holding an empty interface.  This means that the GC has
      incorrect information.
      
      This is extremely unlikely to fail, because the code in the GC
      that handles empty interfaces looks like this:
      
      obj = nil;
      typ = eface->type;
      if(typ != nil) {
              if(!(typ->kind&KindDirectIface) || !(typ->kind&KindNoPointers))
                      obj = eface->data;
      
      In the current runtime the condition is always true--if
      KindDirectIface is set, then KindNoPointers is clear--and we
      always want to set obj = eface->data.  So the question is what
      happens when we incorrectly store a non-empty interface value
      in memory marked as an empty interface.  In that case
      eface->type will not be a *rtype as we expect, but will
      instead be a pointer to an Itab.  We are going to use this
      pointer to look at a *rtype kind field.  The *rtype struct
      starts out like this:
      
      type rtype struct {
              size          uintptr
              hash          uint32            // hash of type; avoids computation in hash tables
              _             uint8             // unused/padding
              align         uint8             // alignment of variable with this type
              fieldAlign    uint8             // alignment of struct field with this type
              kind          uint8             // enumeration for C
      
      An Itab always has at least two pointers, so on a
      little-endian 64-bit system the kind field will be the high
      byte of the second pointer.  This will normally be zero, so
      the test of typ->kind will succeed, which is what we want.
      
      On a 32-bit system it might be possible to construct a failing
      case by somehow getting the Itab for an interface with one
      method to be immediately followed by a word that is all ones.
      The effect would be that the test would sometimes fail and the
      GC would not mark obj, leading to an invalid dangling
      pointer.  I have not tried to construct this test.
      
      I noticed this in gccgo, where this error is much more likely
      to cause trouble for a rather random reason: gccgo uses a
      different layout of rtype, and in gccgo the kind field happens
      to be the low byte of a pointer, not the high byte.
      
      LGTM=rsc
      R=rsc
      CC=golang-codereviews
      https://golang.org/cl/155450044
      7b9c5ec2
    • Russ Cox's avatar
      regexp: fix TestOnePassCutoff · 22be4bfd
      Russ Cox authored
      The stack blowout can no longer happen,
      but we can still test that too-complex regexps
      are rejected.
      
      Replacement for CL 162770043.
      
      LGTM=iant, r
      R=r, iant
      CC=bradfitz, golang-codereviews
      https://golang.org/cl/162860043
      22be4bfd
    • Ian Lance Taylor's avatar
      regexp/syntax: fix validity testing of zero repeats · 0f022fdd
      Ian Lance Taylor authored
      This is already tested by TestRE2Exhaustive, but the build has
      not broken because that test is not run when using -test.short.
      
      LGTM=rsc
      R=rsc
      CC=golang-codereviews
      https://golang.org/cl/155580043
      0f022fdd
    • Russ Cox's avatar
      debug/pe: remove use of unsafe · 3811c4d8
      Russ Cox authored
      Helps in environments with restricted support for unsafe.
      
      LGTM=bradfitz
      R=r, bradfitz
      CC=dsymonds, golang-codereviews
      https://golang.org/cl/156410044
      3811c4d8
    • Daniel Morsing's avatar
      cmd/gc: emit code for extern = <N> · 0edafefc
      Daniel Morsing authored
      https://golang.org/cl/152700045/ made it possible for struct literals assigned to globals to use <N> as the RHS. Normally, this is to zero out variables on first use. Because globals are already zero (or their linker initialized value), we just ignored this.
      
      Now that <N> can occur from non-initialization code, we need to emit this code. We don't use <N> for initialization of globals any more, so this shouldn't cause any excessive zeroing.
      
      Fixes #8961.
      
      LGTM=rsc
      R=golang-codereviews, rsc
      CC=bradfitz, golang-codereviews
      https://golang.org/cl/154540044
      0edafefc
    • Rob Pike's avatar
      encoding/gob: add custom decoder buffer for performance · 63acc48f
      Rob Pike authored
      As we did with encoding, provide a trivial byte reader for
      faster decoding. We can also reduce some of the copying
      by doing the allocation all at once using a slightly different
      interface from byte buffers.
      
      benchmark                            old ns/op     new ns/op     delta
      BenchmarkEndToEndPipe                13368         12902         -3.49%
      BenchmarkEndToEndByteBuffer          5969          5642          -5.48%
      BenchmarkEndToEndSliceByteBuffer     479485        470798        -1.81%
      BenchmarkEncodeComplex128Slice       92367         92201         -0.18%
      BenchmarkEncodeFloat64Slice          39990         38960         -2.58%
      BenchmarkEncodeInt32Slice            30510         27938         -8.43%
      BenchmarkEncodeStringSlice           33753         33365         -1.15%
      BenchmarkDecodeComplex128Slice       232278        196704        -15.32%
      BenchmarkDecodeFloat64Slice          150258        128191        -14.69%
      BenchmarkDecodeInt32Slice            133806        115748        -13.50%
      BenchmarkDecodeStringSlice           335117        300534        -10.32%
      
      LGTM=rsc
      R=rsc
      CC=golang-codereviews
      https://golang.org/cl/154360049
      63acc48f
    • Brad Fitzpatrick's avatar
      doc/go1.4.txt: add compress/* Reset note · 8ba47e3d
      Brad Fitzpatrick authored
      CC=golang-codereviews
      https://golang.org/cl/156430043
      8ba47e3d
    • James Robinson's avatar
      compress/flate: add Reset() to allow reusing large buffers to compress multiple buffers · 193d09a6
      James Robinson authored
      This adds a Reset() to compress/flate's decompressor and plumbs that through
      to compress/zlib and compress/gzip's Readers so callers can avoid large
      allocations when performing many inflate operations. In particular this
      preserves the allocation of the decompressor.hist buffer, which is 32kb and
      overwritten as needed while inflating.
      
      On the benchmark described in issue 6317, produces the following speedup on
      my 2.3ghz Intel Core i7 MBP with go version devel +6b696a34e0af Sun Aug 03
      15:14:59 2014 -0700 darwin/amd64:
      
      blocked.text w/out patch vs blocked.text w/ patch:
      benchmark           old ns/op      new ns/op      delta
      BenchmarkGunzip     8371577533     7927917687     -5.30%
      
      benchmark           old allocs     new allocs     delta
      BenchmarkGunzip     176818         148519         -16.00%
      
      benchmark           old bytes     new bytes     delta
      BenchmarkGunzip     292184936     12739528      -95.64%
      
      flat.text vs blocked.text w/patch:
      benchmark           old ns/op      new ns/op      delta
      BenchmarkGunzip     7939447827     7927917687     -0.15%
      
      benchmark           old allocs     new allocs     delta
      BenchmarkGunzip     90702          148519         +63.74%
      
      benchmark           old bytes     new bytes     delta
      BenchmarkGunzip     9959528       12739528      +27.91%
      
      Similar speedups to those bradfitz saw in  https://golang.org/cl/13416045.
      
      Fixes #6317.
      Fixes #7950.
      
      LGTM=nigeltao
      R=golang-codereviews, bradfitz, dan.kortschak, adg, nigeltao, jamesr
      CC=golang-codereviews
      https://golang.org/cl/97140043
      193d09a6
  6. 19 Oct, 2014 5 commits
  7. 18 Oct, 2014 2 commits
    • Rob Pike's avatar
      text/template: fix bug in pipelined variadics · 1cd78eed
      Rob Pike authored
      Simple bug in argument processing: The final arg may
      be the pipeline value, in which case it gets bound to the
      fixed argument section. The code got that wrong. Easy
      to fix.
      
      Fixes #8950.
      
      LGTM=bradfitz
      R=golang-codereviews, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/161750043
      1cd78eed
    • Rob Pike's avatar
      encoding/gob: use simple append-only buffer for encoding · 65dde1ed
      Rob Pike authored
      Bytes buffers have more API and are a little slower. Since appending
      is a key part of the path in encode, using a faster implementation
      speeds things up measurably.
      The couple of positive swings are likely garbage-collection related
      since memory allocation looks different in the benchmark now.
      I am not concerned by them.
      
      benchmark                            old ns/op     new ns/op     delta
      BenchmarkEndToEndPipe                6620          6388          -3.50%
      BenchmarkEndToEndByteBuffer          3548          3600          +1.47%
      BenchmarkEndToEndSliceByteBuffer     336678        367980        +9.30%
      BenchmarkEncodeComplex128Slice       78199         71297         -8.83%
      BenchmarkEncodeFloat64Slice          37731         32258         -14.51%
      BenchmarkEncodeInt32Slice            26780         22977         -14.20%
      BenchmarkEncodeStringSlice           35882         26492         -26.17%
      BenchmarkDecodeComplex128Slice       194819        185126        -4.98%
      BenchmarkDecodeFloat64Slice          120538        120102        -0.36%
      BenchmarkDecodeInt32Slice            106442        107275        +0.78%
      BenchmarkDecodeStringSlice           272902        269866        -1.11%
      
      LGTM=ruiu
      R=golang-codereviews, ruiu
      CC=golang-codereviews
      https://golang.org/cl/160990043
      65dde1ed
  8. 17 Oct, 2014 1 commit
    • Rob Pike's avatar
      encoding/gob: custom array/slice decoders · 9965e402
      Rob Pike authored
      Use go generate to write better loops for decoding arrays,
      just as we did for encoding. It doesn't help as much,
      relatively speaking, but it's still noticeable.
      
      benchmark                          old ns/op     new ns/op     delta
      BenchmarkDecodeComplex128Slice     202348        184529        -8.81%
      BenchmarkDecodeFloat64Slice        135800        120979        -10.91%
      BenchmarkDecodeInt32Slice          121200        105149        -13.24%
      BenchmarkDecodeStringSlice         288129        278214        -3.44%
      
      LGTM=rsc
      R=rsc
      CC=golang-codereviews
      https://golang.org/cl/154420044
      9965e402