1. 31 Mar, 2016 4 commits
    • Dave Cheney's avatar
      cmd/compile/internal/gc: avoid append when building Type fields · 1a9373bc
      Dave Cheney authored
      As a followup to CL 21296, avoid append operations when constructing the
      fields of a Type if the length is known beforehand
      
      This also includes some small scoping driveby cleanups, and a change to
      tointerface0 to avoid iterating over the field list twice.
      
      compilebench shows a very small reduction in allocations.
      
       name      old time/op    new time/op    delta
      Template     364ms ± 5%     363ms ± 4%    ~     (p=0.945 n=20+19)
      Unicode      182ms ±11%     185ms ±12%    ~     (p=0.445 n=20+20)
      GoTypes      1.14s ± 2%     1.14s ± 3%    ~     (p=0.221 n=20+20)
      Compiler     5.85s ± 2%     5.84s ± 2%    ~     (p=0.369 n=20+20)
      
      name      old alloc/op   new alloc/op   delta
      Template    56.7MB ± 0%    56.7MB ± 0%  -0.04%  (p=0.000 n=20+20)
      Unicode     38.3MB ± 0%    38.3MB ± 0%    ~     (p=0.728 n=20+19)
      GoTypes      180MB ± 0%     180MB ± 0%  -0.02%  (p=0.000 n=20+20)
      Compiler     812MB ± 0%     812MB ± 0%  -0.02%  (p=0.000 n=19+20)
      
      name      old allocs/op  new allocs/op  delta
      Template      482k ± 0%      480k ± 0%  -0.34%  (p=0.000 n=20+20)
      Unicode       377k ± 0%      377k ± 0%  -0.04%  (p=0.010 n=20+20)
      GoTypes      1.36M ± 0%     1.35M ± 0%  -0.24%  (p=0.000 n=20+20)
      Compiler     5.47M ± 0%     5.46M ± 0%  -0.11%  (p=0.000 n=20+18)
      
      Change-Id: Ibb4c40229fa3816acd8de98ba41d1571a2aabacf
      Reviewed-on: https://go-review.googlesource.com/21352Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Dave Cheney <dave@cheney.net>
      1a9373bc
    • Dave Cheney's avatar
      cmd/compile/internal/gc: don't let the argument to Fields.Set escape · ea5091fc
      Dave Cheney authored
      Apply Robert's optimisation from CL 21241 to Type.Fields. The results
      are less impressive, possibly because of the makeup of the test data.
      
      name      old time/op    new time/op    delta
      Template     365ms ± 5%     365ms ± 3%    ~     (p=0.888 n=20+16)
      Unicode      182ms ±10%     180ms ± 9%    ~     (p=0.883 n=20+20)
      GoTypes      1.14s ± 2%     1.13s ± 3%    ~     (p=0.096 n=20+20)
      Compiler     5.74s ± 1%     5.76s ± 2%    ~     (p=0.369 n=20+20)
      
      name      old alloc/op   new alloc/op   delta
      Template    56.8MB ± 0%    56.7MB ± 0%  -0.15%  (p=0.000 n=19+20)
      Unicode     38.3MB ± 0%    38.3MB ± 0%  -0.02%  (p=0.006 n=20+19)
      GoTypes      180MB ± 0%     180MB ± 0%  -0.13%  (p=0.000 n=20+20)
      Compiler     805MB ± 0%     804MB ± 0%  -0.05%  (p=0.000 n=20+20)
      
      name      old allocs/op  new allocs/op  delta
      Template      485k ± 0%      482k ± 0%  -0.54%  (p=0.000 n=19+20)
      Unicode       377k ± 0%      377k ± 0%  -0.05%  (p=0.005 n=20+20)
      GoTypes      1.37M ± 0%     1.36M ± 0%  -0.53%  (p=0.000 n=20+19)
      Compiler     5.42M ± 0%     5.41M ± 0%  -0.21%  (p=0.000 n=20+20)
      
      Change-Id: I6782659fadd605ce9931bf5c737c7058b96a29eb
      Reviewed-on: https://go-review.googlesource.com/21296Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      ea5091fc
    • Nigel Tao's avatar
      image/jpeg: reconstruct progressive images even if incomplete. · 225b223e
      Nigel Tao authored
      Fixes #14522.
      
      As I said on that issue:
      
      ----
      This is a progressive JPEG image. There are two dimensions of
      progressivity: spectral selection (variables zs and ze in scan.go,
      ranging in [0, 63]) and successive approximation (variables ah and al in
      scan.go, ranging in [0, 8), from LSB to MSB, although ah=0 implicitly
      means ah=8).
      
      For this particular image, there are three components, and the SOS
      markers contain this progression:
      
      zs, ze, ah, al:  0  0 0 0	components: 0, 1, 2
      zs, ze, ah, al:  1 63 0 0	components: 1
      zs, ze, ah, al:  1 63 0 0	components: 2
      zs, ze, ah, al:  1 63 0 2	components: 0
      zs, ze, ah, al:  1 10 2 1	components: 0
      zs, ze, ah, al: 11 63 2 1	components: 0
      zs, ze, ah, al:  1 10 1 0	components: 0
      
      The combination of all of these is complete (i.e. spectra 0 to 63 and
      bits 8 exclusive to 0) for components 1 and 2, but it is incomplete for
      component 0 (the luma component). In particular, there is no data for
      component 0, spectra 11 to 63 and bits 1 exclusive to 0.
      
      The image/jpeg code, as of Go 1.6, waits until both dimensions are
      complete before performing the de-quantization, IDCT and copy to an
      *image.YCbCr. This is the "if zigEnd != blockSize-1 || al != 0 { ...
      continue }" code and associated commentary in scan.go.
      
      Almost all progressive JPEG images end up complete in both dimensions
      for all components, but this particular image is incomplete for
      component 0, so the Go code never writes anything to the Y values of the
      resultant *image.YCbCr, which is why the broken output is so dark (but
      still looks recognizable in terms of red and blue hues).
      
      My reading of the ITU T.81 JPEG specification (Annex G) doesn't
      explicitly say that this is a valid image, but it also doesn't rule it
      out.
      
      In any case, the fix is, for progressive JPEG images, to always
      reconstruct the decoded blocks (by performing the de-quantization, IDCT
      and copy to an *image.YCbCr), regardless of whether or not they end up
      complete. Note that, in Go, the jpeg.Decode function does not return
      until the entire image is decoded, so we still only want to reconstruct
      each block once, not once per SOS (Start Of Scan) marker.
      ----
      
      A test image was also added, based on video-001.progressive.jpeg. When
      decoding that image, inserting a
      
      println("nComp, zs, ze, ah, al:", nComp, zigStart, zigEnd, ah, al)
      
      into decoder.processSOS in scan.go prints:
      
      nComp, zs, ze, ah, al: 3 0 0 0 1
      nComp, zs, ze, ah, al: 1 1 5 0 2
      nComp, zs, ze, ah, al: 1 1 63 0 1
      nComp, zs, ze, ah, al: 1 1 63 0 1
      nComp, zs, ze, ah, al: 1 6 63 0 2
      nComp, zs, ze, ah, al: 1 1 63 2 1
      nComp, zs, ze, ah, al: 3 0 0 1 0
      nComp, zs, ze, ah, al: 1 1 63 1 0
      nComp, zs, ze, ah, al: 1 1 63 1 0
      nComp, zs, ze, ah, al: 1 1 63 1 0
      
      In other words, video-001.progressive.jpeg contains 10 different scans.
      This little program below drops half of them (remembering to keep the
      "\xff\xd9" End of Image marker):
      
      ----
      package main
      
      import (
      	"bytes"
      	"io/ioutil"
      	"log"
      )
      
      func main() {
      	sos := []byte{0xff, 0xda}
      	eoi := []byte{0xff, 0xd9}
      
      	src, err := ioutil.ReadFile("video-001.progressive.jpeg")
      	if err != nil {
      		log.Fatal(err)
      	}
      	b := bytes.Split(src, sos)
      	println(len(b)) // Prints 11.
      	dst := bytes.Join(b[:5], sos)
      	dst = append(dst, eoi...)
      	if err := ioutil.WriteFile("video-001.progressive.truncated.jpeg", dst, 0666); err != nil {
      		log.Fatal(err)
      	}
      }
      ----
      
      The video-001.progressive.truncated.jpeg was converted to png via
      libjpeg and ImageMagick:
      
      djpeg -nosmooth video-001.progressive.truncated.jpeg > tmp.tga
      convert tmp.tga video-001.progressive.truncated.png
      rm tmp.tga
      
      Change-Id: I72b20cd4fb6746d36d8d4d587f891fb3bc641f84
      Reviewed-on: https://go-review.googlesource.com/21062Reviewed-by: default avatarRob Pike <r@golang.org>
      225b223e
    • Dave Cheney's avatar
      cmd/compile/internal/gc: don't iterate over field list twice · 03731283
      Dave Cheney authored
      In tostruct0 and tofunargs we take a list of nodes, transform them into
      a slice of Fields, set the fields on a type, then use the IterFields
      iterator to iterate over the list again to see if any of them are
      broken.
      
      As we know the slice of fielde-we just created it-we can combine these two
      interations into one pass over the fields.
      
      Change-Id: I8b04c90fb32fd6c3b1752cfc607128a634ee06c5
      Reviewed-on: https://go-review.googlesource.com/21350Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      03731283
  2. 30 Mar, 2016 32 commits
  3. 29 Mar, 2016 4 commits
    • Josh Bleecher Snyder's avatar
      cmd/compile: add typArray, typSlice, and typDDDArray · eb98e515
      Josh Bleecher Snyder authored
      These are the first of several convenience
      constructors for types.
      
      They are part of type field encapsulation.
      This removes most external writes to TARRAY Type and Bound fields.
      
      substAny still directly fiddles with the .Type field.
      substAny generally needs access to Type internals.
      It will be moved to type.go in a future CL.
      
      bimport still directly writes the .Type field.
      This is hard to change.
      
      Also of note:
      
      * inl.go contains an (apparently irrelevant) bug fix:
        as.Right was given the wrong type.
        vararrtype was previously unused.
      * I believe that aindex (subr.go) never creates slices,
        but it is safer to keep existing behavior.
        The removal of -1 as a constant there is part
        of hiding that implementation detail.
        Future CLs will finish that job.
      
      Passes toolstash -cmp.
      
      Change-Id: If09bf001a874d7dba08e9ad0bcd6722860af4b91
      Reviewed-on: https://go-review.googlesource.com/21249Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      eb98e515
    • Josh Bleecher Snyder's avatar
      cmd/compile: make only one new Node in defaultlit · f32161da
      Josh Bleecher Snyder authored
      defaultlit and friends sometimes create a new
      OLITERAL node, only to have replace it.
      Thread hints when that is unnecessary.
      
      name       old time/op     new time/op     delta
      Template       318ms ± 6%      322ms ± 4%     ~           (p=0.154 n=24+25)
      Unicode        162ms ± 6%      151ms ± 7%   -6.94%        (p=0.000 n=22+23)
      GoTypes        1.04s ± 1%      1.04s ± 3%     ~           (p=0.136 n=20+25)
      Compiler       5.08s ± 2%      5.10s ± 4%     ~           (p=0.788 n=25+25)
      MakeBash       41.4s ± 1%      41.5s ± 1%     ~           (p=0.084 n=25+25)
      
      name       old user-ns/op  new user-ns/op  delta
      Template        438M ±10%       441M ± 9%     ~           (p=0.418 n=25+25)
      Unicode         272M ± 5%       219M ± 5%  -19.33%        (p=0.000 n=24+21)
      GoTypes        1.51G ± 3%      1.51G ± 3%     ~           (p=0.500 n=25+25)
      Compiler       7.31G ± 3%      7.32G ± 3%     ~           (p=0.572 n=25+24)
      
      name       old alloc/op    new alloc/op    delta
      Template      57.3MB ± 0%     57.2MB ± 0%   -0.16%        (p=0.000 n=25+25)
      Unicode       41.1MB ± 0%     38.7MB ± 0%   -5.81%        (p=0.000 n=25+25)
      GoTypes        191MB ± 0%      191MB ± 0%   -0.06%        (p=0.000 n=25+25)
      Compiler       840MB ± 0%      839MB ± 0%   -0.12%        (p=0.000 n=25+25)
      
      name       old allocs/op   new allocs/op   delta
      Template        500k ± 0%       500k ± 0%   -0.12%        (p=0.000 n=24+25)
      Unicode         400k ± 0%       384k ± 0%   -4.16%        (p=0.000 n=25+25)
      GoTypes        1.50M ± 0%      1.49M ± 0%   -0.05%        (p=0.000 n=25+25)
      Compiler       6.04M ± 0%      6.03M ± 0%   -0.11%        (p=0.000 n=25+25)
      
      Change-Id: I2fda5e072db67ba239848bde827c7deb2ad4abae
      Reviewed-on: https://go-review.googlesource.com/20813Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      f32161da
    • Aliaksandr Valialkin's avatar
      cmd/vet: improve detecting printf-like format argument · ee1b90ad
      Aliaksandr Valialkin authored
      Previously format argument was detected via scanning func type args.
      This didn't work when func type couldn't be determined if the func
      is declared in the external package. Fall back to scanning for
      the first string call argument in this case.
      
      Fixes #14754
      
      Change-Id: I571cc29684cc641bc87882002ef474cf1481e9e2
      Reviewed-on: https://go-review.googlesource.com/21023
      Run-TryBot: Rob Pike <r@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      ee1b90ad
    • Michael Munday's avatar
      debug/elf: add s390x relocations · 12fb62a5
      Michael Munday authored
      Change-Id: I8440f69c7f99d65b2f69035c26b4a62104f22bd3
      Reviewed-on: https://go-review.googlesource.com/20874Reviewed-by: default avatarMinux Ma <minux@golang.org>
      12fb62a5