1. 27 Aug, 2019 31 commits
    • Filippo Valsorda's avatar
      crypto/tls: remove SSLv3 support · ffcb678f
      Filippo Valsorda authored
      SSLv3 has been irreparably broken since the POODLE attack 5 years ago
      and RFC 7568 (f.k.a. draft-ietf-tls-sslv3-diediedie) prohibits its use
      in no uncertain terms.
      
      As announced in the Go 1.13 release notes, remove support for it
      entirely in Go 1.14.
      
      Updates #32716
      
      Change-Id: Id653557961d8f75f484a01e6afd2e104a4ccceaf
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191976
      Run-TryBot: Filippo Valsorda <filippo@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      ffcb678f
    • Josh Bleecher Snyder's avatar
      cmd/compile: improve shortcircuit pass · 52ae04fd
      Josh Bleecher Snyder authored
      While working on #30645, I noticed that many instances
      in which the walkinrange optimization could apply
      were not even being considered.
      
      This was because of extraneous blocks in the CFG,
      of the type that shortcircuit normally removes.
      
      The change improves the shortcircuit pass to handle
      most of those cases. (There are a few that can only be
      reasonably detected later in compilation, after other
      optimizations have been run, but not enough to be worth chasing.)
      
      Notable changes:
      
      * Instead of calculating live-across-blocks values, use v.Uses == 1.
        This is cheaper and more straightforward.
        v.Uses did not exist when this pass was initially written.
      * Incorporate a fusePlain and loop until stable.
        This is necessary to find many of the instances.
      * Allow Copy and Not wrappers around Phi values.
        This significantly increases effectiveness.
      * Allow removal of all preds, creating a dead block.
        The previous pass stopped unnecessarily at one pred.
      * Use phielimValue during cleanup instead of manually
        setting the op to OpCopy.
      
      The result is marginally faster compilation and smaller code.
      
      name        old time/op       new time/op       delta
      Template          213ms ± 2%        212ms ± 2%  -0.63%  (p=0.002 n=49+48)
      Unicode          90.0ms ± 2%       89.8ms ± 2%    ~     (p=0.122 n=48+48)
      GoTypes           710ms ± 3%        711ms ± 2%    ~     (p=0.433 n=45+49)
      Compiler          3.23s ± 2%        3.22s ± 2%    ~     (p=0.124 n=47+49)
      SSA               10.0s ± 1%        10.0s ± 1%  -0.43%  (p=0.000 n=48+50)
      Flate             135ms ± 3%        135ms ± 2%    ~     (p=0.311 n=49+49)
      GoParser          158ms ± 2%        158ms ± 2%    ~     (p=0.757 n=48+48)
      Reflect           447ms ± 2%        447ms ± 2%    ~     (p=0.815 n=49+48)
      Tar               189ms ± 2%        189ms ± 3%    ~     (p=0.530 n=47+49)
      XML               251ms ± 3%        250ms ± 1%  -0.75%  (p=0.002 n=49+48)
      [Geo mean]        427ms             426ms       -0.25%
      
      name        old user-time/op  new user-time/op  delta
      Template          265ms ± 2%        265ms ± 2%    ~     (p=0.969 n=48+50)
      Unicode           119ms ± 6%        119ms ± 6%    ~     (p=0.738 n=50+50)
      GoTypes           923ms ± 2%        925ms ± 2%    ~     (p=0.057 n=43+47)
      Compiler          4.37s ± 2%        4.37s ± 2%    ~     (p=0.691 n=50+46)
      SSA               13.4s ± 1%        13.4s ± 1%    ~     (p=0.282 n=42+49)
      Flate             162ms ± 2%        162ms ± 2%    ~     (p=0.774 n=48+50)
      GoParser          186ms ± 2%        186ms ± 3%    ~     (p=0.213 n=47+47)
      Reflect           572ms ± 2%        573ms ± 3%    ~     (p=0.303 n=50+49)
      Tar               240ms ± 3%        240ms ± 2%    ~     (p=0.939 n=46+44)
      XML               302ms ± 2%        302ms ± 2%    ~     (p=0.399 n=47+47)
      [Geo mean]        540ms             541ms       +0.07%
      
      name        old alloc/op      new alloc/op      delta
      Template         36.8MB ± 0%       36.7MB ± 0%  -0.42%  (p=0.008 n=5+5)
      Unicode          28.1MB ± 0%       28.1MB ± 0%    ~     (p=0.151 n=5+5)
      GoTypes           124MB ± 0%        124MB ± 0%  -0.26%  (p=0.008 n=5+5)
      Compiler          571MB ± 0%        566MB ± 0%  -0.84%  (p=0.008 n=5+5)
      SSA              1.86GB ± 0%       1.85GB ± 0%  -0.58%  (p=0.008 n=5+5)
      Flate            22.8MB ± 0%       22.8MB ± 0%  -0.17%  (p=0.008 n=5+5)
      GoParser         27.3MB ± 0%       27.3MB ± 0%  -0.20%  (p=0.008 n=5+5)
      Reflect          79.5MB ± 0%       79.3MB ± 0%  -0.20%  (p=0.008 n=5+5)
      Tar              34.7MB ± 0%       34.6MB ± 0%  -0.42%  (p=0.008 n=5+5)
      XML              45.4MB ± 0%       45.3MB ± 0%  -0.29%  (p=0.008 n=5+5)
      [Geo mean]       80.0MB            79.7MB       -0.34%
      
      name        old allocs/op     new allocs/op     delta
      Template           378k ± 0%         377k ± 0%  -0.22%  (p=0.008 n=5+5)
      Unicode            339k ± 0%         339k ± 0%    ~     (p=0.643 n=5+5)
      GoTypes           1.36M ± 0%        1.36M ± 0%  -0.10%  (p=0.008 n=5+5)
      Compiler          5.51M ± 0%        5.50M ± 0%  -0.13%  (p=0.008 n=5+5)
      SSA               17.5M ± 0%        17.5M ± 0%  -0.14%  (p=0.008 n=5+5)
      Flate              234k ± 0%         234k ± 0%  -0.04%  (p=0.008 n=5+5)
      GoParser           299k ± 0%         299k ± 0%  -0.05%  (p=0.008 n=5+5)
      Reflect            978k ± 0%         979k ± 0%  +0.02%  (p=0.016 n=5+5)
      Tar                351k ± 0%         351k ± 0%  -0.04%  (p=0.008 n=5+5)
      XML                435k ± 0%         435k ± 0%  -0.11%  (p=0.008 n=5+5)
      [Geo mean]         840k              840k       -0.08%
      
      file      before    after     Δ       %
      go        14794788  14770212  -24576  -0.166%
      addr2line 4203688   4199592   -4096   -0.097%
      api       5954056   5941768   -12288  -0.206%
      asm       4862704   4846320   -16384  -0.337%
      cgo       4778920   4770728   -8192   -0.171%
      compile   24001568  23923792  -77776  -0.324%
      cover     5198440   5190248   -8192   -0.158%
      dist      3595248   3587056   -8192   -0.228%
      doc       4618504   4610312   -8192   -0.177%
      fix       3337416   3333320   -4096   -0.123%
      link      6120408   6116312   -4096   -0.067%
      nm        4149064   4140872   -8192   -0.197%
      objdump   4555608   4547416   -8192   -0.180%
      pprof     14616324  14595844  -20480  -0.140%
      test2json 2766328   2762232   -4096   -0.148%
      trace     11638844  11622460  -16384  -0.141%
      vet       8274936   8258552   -16384  -0.198%
      total     132520780 132270972 -249808 -0.189%
      
      Change-Id: Ifcd235a2a6e5f13ed5c93e62523e2ef61321fccf
      Reviewed-on: https://go-review.googlesource.com/c/go/+/178197
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      52ae04fd
    • Bryan C. Mills's avatar
      cmd/go/internal/get: remove '--' separator from 'git ls-remote' command · 0a778cf5
      Bryan C. Mills authored
      'git ls-remote' started recognizing the '--' separator at some point
      after 2.7.4, but git defaults to version 2.7.4 on Ubuntu 16.04 LTS,
      which remains supported by Ubuntu until April 2021.
      
      We added '--' tokens to most VCS commands as a defensive measure in
      CL 181237, but it isn't strictly necessary here because the 'scheme'
      argument to our template is chosen from a predefined list: we can
      safely drop it to retain compatibility.
      
      Fixes #33836
      Updates #26746
      
      Change-Id: Ibb53366b95f8029b587e0b7646a439330d759ac7
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191978
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDmitri Shuralyov <dmitshur@golang.org>
      0a778cf5
    • Josh Bleecher Snyder's avatar
      cmd/compile: run deadcode before lowered CSE · 260e3d08
      Josh Bleecher Snyder authored
      CSE can make dead values live again.
      Running deadcode first avoids that;
      it also makes CSE more efficient.
      
      file    before    after     Δ       %       
      api     5970616   5966520   -4096   -0.069% 
      asm     4867088   4846608   -20480  -0.421% 
      compile 23988320  23935072  -53248  -0.222% 
      link    6084376   6080280   -4096   -0.067% 
      nm      4165736   4161640   -4096   -0.098% 
      objdump 4572216   4568120   -4096   -0.090% 
      pprof   14452996  14457092  +4096   +0.028% 
      trace   11467292  11471388  +4096   +0.036% 
      total   132181100 132099180 -81920  -0.062% 
      
      Compiler performance impact is negligible:
      
      name        old alloc/op      new alloc/op      delta
      Template         38.8MB ± 0%       38.8MB ± 0%  -0.04%  (p=0.008 n=5+5)
      Unicode          28.2MB ± 0%       28.2MB ± 0%    ~     (p=1.000 n=5+5)
      GoTypes           131MB ± 0%        131MB ± 0%  -0.14%  (p=0.008 n=5+5)
      Compiler          606MB ± 0%        606MB ± 0%  -0.05%  (p=0.008 n=5+5)
      SSA              2.14GB ± 0%       2.13GB ± 0%  -0.26%  (p=0.008 n=5+5)
      Flate            24.0MB ± 0%       24.0MB ± 0%  -0.18%  (p=0.008 n=5+5)
      GoParser         28.8MB ± 0%       28.8MB ± 0%  -0.15%  (p=0.008 n=5+5)
      Reflect          83.8MB ± 0%       83.7MB ± 0%  -0.11%  (p=0.008 n=5+5)
      Tar              36.4MB ± 0%       36.4MB ± 0%  -0.09%  (p=0.008 n=5+5)
      XML              47.9MB ± 0%       47.8MB ± 0%  -0.15%  (p=0.008 n=5+5)
      [Geo mean]       84.6MB            84.5MB       -0.12%
      
      name        old allocs/op     new allocs/op     delta
      Template           379k ± 0%         380k ± 0%  +0.15%  (p=0.008 n=5+5)
      Unicode            340k ± 0%         340k ± 0%    ~     (p=0.738 n=5+5)
      GoTypes           1.36M ± 0%        1.36M ± 0%  +0.05%  (p=0.008 n=5+5)
      Compiler          5.49M ± 0%        5.49M ± 0%  +0.12%  (p=0.008 n=5+5)
      SSA               17.5M ± 0%        17.5M ± 0%  -0.18%  (p=0.008 n=5+5)
      Flate              235k ± 0%         235k ± 0%    ~     (p=0.079 n=5+5)
      GoParser           302k ± 0%         302k ± 0%    ~     (p=0.310 n=5+5)
      Reflect            976k ± 0%         977k ± 0%  +0.08%  (p=0.008 n=5+5)
      Tar                352k ± 0%         352k ± 0%  +0.12%  (p=0.008 n=5+5)
      XML                436k ± 0%         436k ± 0%  -0.05%  (p=0.008 n=5+5)
      [Geo mean]         842k              842k       +0.03%
      
      
      Change-Id: I53e8faed1859885ca5c4a5d45067a50984f3eff1
      Reviewed-on: https://go-review.googlesource.com/c/go/+/175879
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      260e3d08
    • Stefan Baebler's avatar
      net/url: fail TestParseErrors test when getting an unwanted error · cc6feab3
      Stefan Baebler authored
      The TestParseErrors test function was not strict with unwanted errors
      received from url.Parse(). It was not failing in such cases, now it does
      
      Fixes #33646
      Updates #29098
      
      Change-Id: I069521093e2bff8b1fcd41ffd3f9799f3108bc61
      GitHub-Last-Rev: e6844c57f979ddb8418643d9c5244a5d1b4578ba
      GitHub-Pull-Request: golang/go#33876
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191966
      Run-TryBot: Filippo Valsorda <filippo@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarFilippo Valsorda <filippo@golang.org>
      cc6feab3
    • Anderson Queiroz's avatar
      net/http: enhance documentation for Server.Addr · 2b598944
      Anderson Queiroz authored
      Fixes golang/go#31249
      
      Change-Id: I3280f8ab170ed31d4efb71106533e016d430d44c
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191557Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      2b598944
    • Filippo Valsorda's avatar
      crypto/tls: make SSLv3 again disabled by default · 2ebc3d81
      Filippo Valsorda authored
      It was mistakenly re-enabled in CL 146217.
      
      Fixes #33837
      
      Change-Id: I8c0e1787114c6232df5888e51e355906622295bc
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191877
      Run-TryBot: Filippo Valsorda <filippo@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDmitri Shuralyov <dmitshur@golang.org>
      2ebc3d81
    • Bryan C. Mills's avatar
      cmd/go/internal/modload: fix swapped paths in error message · 8f5353fd
      Bryan C. Mills authored
      Updates #33879
      
      Change-Id: Ifc91490b1cb791fdf5ffe69ef81c0ec0e6cbecc3
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191997
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      Reviewed-by: default avatarAlexander Rakoczy <alex@golang.org>
      8f5353fd
    • Matthew Dempsky's avatar
      cmd/compile: fix "previous" position info for duplicate switch cases · c302785d
      Matthew Dempsky authored
      Because the Node AST represents references to declared objects (e.g.,
      variables, packages, types, constants) by directly pointing to the
      referred object, we don't have use-position info for these objects.
      
      For switch statements with duplicate cases, we report back where the
      first duplicate value appeared. However, due to the AST
      representation, if the value was a declared constant, we mistakenly
      reported the constant declaration position as the previous case
      position.
      
      This CL reports back against the 'case' keyword's position instead, if
      there's no more precise information available to us.
      
      It also refactors code to emit the same "previous at" error message
      for duplicate values in map literals.
      
      Thanks to Emmanuel Odeke for the test case.
      
      Fixes #33460.
      
      Change-Id: Iec69542ccd4aad594dde8df02d1b880a422c5622
      Reviewed-on: https://go-review.googlesource.com/c/go/+/188901Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      c302785d
    • Matthew Dempsky's avatar
      runtime: simplify some pointer conversions · c1df5187
      Matthew Dempsky authored
      Use efaceOf to safely convert from *interface{} to *_eface, and to
      make it clearer what the pointer arithmetic is computing.
      
      Incidentally, remove a spurious unsafe.Pointer->*uint8->unsafe.Pointer
      round trip conversion in newproc.
      
      No behavior change.
      
      Change-Id: I2ad9d791d35d8bd008ef43b03dad1589713c5fd4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/190457
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      c1df5187
    • Bryan C. Mills's avatar
      net/http: fix wantConnQueue memory leaks in Transport · 94bf9a8d
      Bryan C. Mills authored
      I'm trying to keep the code changes minimal for backporting to Go 1.13,
      so it is still possible for a handful of entries to leak,
      but the leaks are now O(1) instead of O(N) in the steady state.
      
      Longer-term, I think it would be a good idea to coalesce idleMu with
      connsPerHostMu and clear entries out of both queues as soon as their
      goroutines are done waiting.
      
      Fixes #33849
      Fixes #33850
      
      Change-Id: Ia66bc64671eb1014369f2d3a01debfc023b44281
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191964
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      94bf9a8d
    • LE Manh Cuong's avatar
      cmd/compile: optimize bitset tests · c5f142fa
      LE Manh Cuong authored
      The assembly output for x & c == c, where c is power of 2:
      
      	MOVQ	"".set+8(SP), AX
      	ANDQ	$8, AX
      	CMPQ	AX, $8
      	SETEQ	"".~r2+24(SP)
      
      With optimization using bitset:
      
      	MOVQ	"".set+8(SP), AX
      	BTL	$3, AX
      	SETCS	"".~r2+24(SP)
      
      output less than 1 instruction.
      
      However, there is no speed improvement:
      
      name         old time/op  new time/op  delta
      AllBitSet-8  0.35ns ± 0%  0.35ns ± 0%   ~     (all equal)
      
      Fixes #31904
      
      Change-Id: I5dca4e410bf45716ed2145e3473979ec997e35d4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/175957
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      c5f142fa
    • Daniel Martí's avatar
      encoding/json: remove unnecessary isValidNumber call · ae68a912
      Daniel Martí authored
      The decoder called this function to check numbers being decoded into a
      json.Number. However, these can't be quoted as strings, so the tokenizer
      has already verified they are valid JSON numbers.
      
      Verified this by adding a test with such an input. As expected, it
      produces a syntax error, not the fmt.Errorf - that line could never
      execute.
      
      Since the only remaining non-test caller of isvalidnumber is in
      encode.go, move the function there.
      
      This change should slightly reduce the amount of work when decoding into
      json.Number, though that isn't very common nor part of any current
      benchmarks.
      
      Change-Id: I67a1723deb3d18d5b542d6dd35f3ae56a43f23eb
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184817
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      ae68a912
    • Dong-hee Na's avatar
      html/template: micro optimization for isJSType · b9bf2f5d
      Dong-hee Na authored
      There is an unnecessary lower operation in isJSType.
      Simple logic fix can improve tiny performance.
      
      name        old time/op    new time/op    delta
      isJSType-8     152ns ± 0%      58ns ± 7%   -61.82%  (p=0.001 n=6+8)
      
      name        old alloc/op   new alloc/op   delta
      isJSType-8     32.0B ± 0%      0.0B       -100.00%  (p=0.000 n=8+8)
      
      name        old allocs/op  new allocs/op  delta
      isJSType-8      1.00 ± 0%      0.00       -100.00%  (p=0.000 n=8+8)
      
      Change-Id: I281aadf1677d4377920c9649af206381189a27e6
      Reviewed-on: https://go-review.googlesource.com/c/go/+/177118
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      b9bf2f5d
    • Daniel Martí's avatar
      Revert "net/url: fail TestParseErrors test when getting an unwanted error" · 72e71b90
      Daniel Martí authored
      This reverts https://golang.org/cl/185080.
      
      Reason for revert: some new changes are erroring again, so this broke the builders.
      
      Change-Id: I28da16da98b90cefbb47173d31bbbb56e43062d5
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191781
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      72e71b90
    • LE Manh Cuong's avatar
      cmd/compile: eliminate usage of global Fatalf in ssa.go · 1a432f27
      LE Manh Cuong authored
      state and ssafn both have their own Fatalf, so use them instead of
      global Fatalf.
      
      Updates #19683
      
      Change-Id: Ie02a961d4285ab0a3f3b8d889a5b498d926ed567
      Reviewed-on: https://go-review.googlesource.com/c/go/+/188539
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      1a432f27
    • Daniel Martí's avatar
      cmd/compile: teach rulegen to remove unused decls · 79dee788
      Daniel Martí authored
      First, add cpu and memory profiling flags, as these are useful to see
      where rulegen is spending its time. It now takes many seconds to run on
      a recent laptop, so we have to keep an eye on what it's doing.
      
      Second, stop writing '_ = var' lines to keep imports and variables used
      at all times. Now that rulegen removes all such unused names, they're
      unnecessary.
      
      To perform the removal, lean on go/types to first detect what names are
      unused. We can configure it to give us all the type-checking errors in a
      file, so we can collect all "declared but not used" errors in a single
      pass.
      
      We then use astutil.Apply to remove the relevant nodes based on the line
      information from each unused error. This allows us to apply the changes
      without having to do extra parser+printer roundtrips to plaintext, which
      are far too expensive.
      
      We need to do multiple such passes, as removing an unused variable
      declaration might then make another declaration unused. Two passes are
      enough to clean every file at the moment, so add a limit of three passes
      for now to avoid eating cpu uncontrollably by accident.
      
      The resulting performance of the changes above is a ~30% loss across the
      table, since go/types is fairly expensive. The numbers were obtained
      with 'benchcmd Rulegen go run *.go', which involves compiling rulegen
      itself, but that seems reflective of how the program is used.
      
      	name     old time/op         new time/op         delta
      	Rulegen          5.61s ± 0%          7.36s ± 0%  +31.17%  (p=0.016 n=5+4)
      
      	name     old user-time/op    new user-time/op    delta
      	Rulegen          7.20s ± 1%          9.92s ± 1%  +37.76%  (p=0.016 n=5+4)
      
      	name     old sys-time/op     new sys-time/op     delta
      	Rulegen          135ms ±19%          169ms ±17%  +25.66%  (p=0.032 n=5+5)
      
      	name     old peak-RSS-bytes  new peak-RSS-bytes  delta
      	Rulegen         71.0MB ± 2%         85.6MB ± 2%  +20.56%  (p=0.008 n=5+5)
      
      We can live with a bit more resource usage, but the time/op getting
      close to 10s isn't good. To win that back, introduce concurrency in
      main.go. This further increases resource usage a bit, but the real time
      on this quad-core laptop is greatly reduced. The final benchstat is as
      follows:
      
      	name     old time/op         new time/op         delta
      	Rulegen          5.61s ± 0%          3.97s ± 1%   -29.26%  (p=0.008 n=5+5)
      
      	name     old user-time/op    new user-time/op    delta
      	Rulegen          7.20s ± 1%         13.91s ± 1%   +93.09%  (p=0.008 n=5+5)
      
      	name     old sys-time/op     new sys-time/op     delta
      	Rulegen          135ms ±19%          269ms ± 9%   +99.17%  (p=0.008 n=5+5)
      
      	name     old peak-RSS-bytes  new peak-RSS-bytes  delta
      	Rulegen         71.0MB ± 2%        226.3MB ± 1%  +218.72%  (p=0.008 n=5+5)
      
      It might be possible to reduce the cpu or memory usage in the future,
      such as configuring go/types to do less work, or taking shortcuts to
      avoid having to run it many times. For now, ~2x cpu and ~4x memory usage
      seems like a fair trade for a faster and better rulegen.
      
      Finally, we can remove the old code that tried to remove some unused
      variables in a hacky and unmaintainable way.
      
      Change-Id: Iff9e83e3f253babf5a1bd48cc993033b8550cee6
      Reviewed-on: https://go-review.googlesource.com/c/go/+/189798
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      79dee788
    • Andrew Gerrand's avatar
      archive/zip: remove unused special case · 483d6d99
      Andrew Gerrand authored
      This removes a special case that was added to fix issue #10956, but that
      was never actually effective. The code in the test case still fails to
      read, so perhaps the zip64 support added in CL 6463050 inadvertently
      caught this particular case.
      
      It's possible that the original theorized bug still exists, but I'm not
      convinced it was ever fixed.
      
      Update #28700
      
      Change-Id: I4854de616364510f64a6def30b308686563f8dbb
      Reviewed-on: https://go-review.googlesource.com/c/go/+/179757
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      483d6d99
    • tnclong's avatar
      text/template: avoid allocating a new common in copy · 4a4f752c
      tnclong authored
      Template.New calls t.init, which allocates several items that
      are immediately rewritten by copy, so avoid the call to New
      
      Change-Id: I16c7cb001bbcd14cf547c1a2db2734a2f8214e7e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/182757
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      4a4f752c
    • Stefan Baebler's avatar
      net/url: fail TestParseErrors test when getting an unwanted error · 32b9e568
      Stefan Baebler authored
      The TestParseErrors test function was not strict with unwanted errors
      received from url.Parse(). It was not failing in such cases, now it does.
      
      Change-Id: I18a26a68c1136f5c762989a76e04b47e33dd35f1
      GitHub-Last-Rev: c33f9842f7908f27012859e25caa79388cc2785a
      GitHub-Pull-Request: golang/go#32954
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185080Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      32b9e568
    • Alex Brainman's avatar
      net: do not call Windows TransmitFile for large files · b963149d
      Alex Brainman authored
      TransmitFile does not allow for number of bytes that can be
      transmitted to be larger than 2147483646. See
      
      https://docs.microsoft.com/en-us/windows/win32/api/mswsock/nf-mswsock-transmitfile
      
      for details. So adjust sendFile accordingly.
      
      No test added, because this would require creating large file
      (more than 2GB file).
      
      Fixes #33193.
      
      Change-Id: I82e0cb104d112264e4ea363bb20b6d02ac30b38e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/187037
      Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      b963149d
    • Dong-hee Na's avatar
      text/template: replace bytes.Buffer with strings.Builder · 997086b7
      Dong-hee Na authored
      After Go.1.10+ strings.Builder is known as more efficient in
      concatenating and building strings than bytes.Buffer.
      
      In this CL,
      there is a minor logic fix for getting advantage of strings.builder.
      
      name               old time/op    new time/op    delta
      DefinedTemplate-8     543ns ± 3%     512ns ± 2%   -5.73%  (p=0.000 n=8+8)
      
      name               old alloc/op   new alloc/op   delta
      DefinedTemplate-8      192B ± 0%      160B ± 0%  -16.67%  (p=0.000 n=8+8)
      
      name               old allocs/op  new allocs/op  delta
      DefinedTemplate-8      5.00 ± 0%      5.00 ± 0%     ~     (all equal)
      
      Change-Id: Icda0054d146e6c5e32ed8a4d13221bb6850d31b4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/175261
      Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      997086b7
    • Jonathan Amsterdam's avatar
      errors: document Is and As methods · fc4663d5
      Jonathan Amsterdam authored
      Add brief descriptions of why one might implement
      an Is or As method.
      
      Fixes #33364.
      
      Change-Id: I81a091bf564c654ddb9ef3997e780451a01efb7a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191338Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Reviewed-by: default avatarAndrew Bonventre <andybons@golang.org>
      Run-TryBot: Jonathan Amsterdam <jba@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      fc4663d5
    • Daniel Martí's avatar
      encoding/json: fix the broken "overwriting of data" tests · 95c3c430
      Daniel Martí authored
      Because TestUnmarshal actually allocates a new value to decode into
      using ptr's pointer type, any existing data is thrown away. This was
      harmless in alomst all of the test cases, minus the "overwriting of
      data" ones added in 2015 in CL 12209.
      
      I spotted that nothing covered decoding a JSON array with few elements
      into a slice which already had many elements. I initially assumed that
      the code was buggy or that some code could be removed, when in fact
      there simply wasn't any code covering the edge case.
      
      Move those two tests to TestPrefilled, which already served a very
      similar purpose. Remove the map case, as TestPrefilled already has
      plenty of prefilled map cases. Moreover, we no longer reset an entire
      map when decoding, as per the godoc:
      
      	To unmarshal a JSON object into a map, Unmarshal first
      	establishes a map to use. If the map is nil, Unmarshal allocates
      	a new map. Otherwise Unmarshal reuses the existing map, keeping
      	existing entries.
      
      Finally, to ensure that ptr is used correctly in the future, make
      TestUnmarshal error if it's anything other than a pointer to a zero
      value. That is, the only correct use should be new(type). Don't rename
      the ptr field, as that would be extremely noisy and cause unwanted merge
      conflicts.
      
      Change-Id: I41e3ecfeae42d877ac5443a6bd622ac3d6c8120c
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185738
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      95c3c430
    • Daniel Martí's avatar
      cmd/compile: initial rulegen rewrite · 1a53915c
      Daniel Martí authored
      rulegen.go produces plaintext Go code directly, which was fine for a
      while. However, that's started being a bottleneck for making code
      generation more complex, as we can only generate code directly one line
      at a time.
      
      Some workarounds were used, like multiple layers of buffers to generate
      chunks of code, to then use strings.Contains to see whether variables
      need to be defined or not. However, that's error-prone, verbose, and
      difficult to work with.
      
      A better approach is to generate an intermediate syntax tree in memory,
      which we can inspect and modify easily. For example, we could run a
      number of "passes" on the syntax tree before writing to disk, such as
      removing unused variables, simplifying logic, or moving declarations
      closer to their uses.
      
      This is the first step in that direction, without changing any of the
      generated code. We didn't use go/ast directly, as it's too complex for
      our needs. In particular, we only need a few kinds of simple statements,
      but we do want to support arbitrary expressions. As such, define a
      simple set of statement structs, and add thin layers for printer.Fprint
      and ast.Inspect.
      
      A nice side effect of this change, besides removing some buffers and
      string handling, is that we can now avoid passing so many parameters
      around. And, while we add over a hundred lines of code, the tricky
      pieces of code are now a bit simpler to follow.
      
      While at it, apply some cleanups, such as replacing isVariable with
      token.IsIdentifier, and consistently using log.Fatalf.
      
      Follow-up CLs will start improving the generated code, also simplifying
      the rulegen code itself. I've added some TODOs for the low-hanging fruit
      that I intend to work on right after.
      
      Updates #30810.
      
      Change-Id: Ic371c192b29c85dfc4a001be7fbcbeec85facc9d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/177539
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      1a53915c
    • Javier Revillas's avatar
      net/http: fix a typo in comments · cd33d271
      Javier Revillas authored
      HTTP is an initialism, not an acronym, where you pronounce each letter as a
      word. It's "an H", not "a H".
      
      Running `find src/net/http -type f | xargs grep -n 'an HTTP' | wc -l` shows
      that the "an HTTP" form is used 67 times across the `net/http` package.
      Furthermore, `find src/net/http -type f | xargs grep -n 'a HTTP' | wc -l`
      yields only 4 results.
      
      Change-Id: I219c292a9e2c9bf7a009dbfe82ea8b15874685e9
      GitHub-Last-Rev: 6ebd095023af47444b6b0fc5b6d7b26d85f4c7b7
      GitHub-Pull-Request: golang/go#33810
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191700Reviewed-by: default avatarToshihiro Shiino <shiino.toshihiro@gmail.com>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      cd33d271
    • LE Manh Cuong's avatar
      cmd/compile: remove adjustctx from inline test · 324cf21f
      LE Manh Cuong authored
      After golang.org/cl/33895, function adjustctx can not be inlined,
      cost 82 exceeds budget 80
      
      Change-Id: Ie559ed80ea2c251add940a99f11b2983f6cbddbc
      Reviewed-on: https://go-review.googlesource.com/c/go/+/187977
      Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      324cf21f
    • zdjones's avatar
      cmd/compile: handle sign/zero extensions in prove, via update method · 69ff0ba7
      zdjones authored
      Array accesses with index types smaller than the machine word size may
      involve a sign or zero extension of the index value before bounds
      checking. Currently, this defeats prove because the facts about the
      original index value don't flow through the sign/zero extension.
      
      This CL fixes this by looking back through value-preserving sign/zero
      extensions when adding facts via Update and, where appropriate, applying
      the same facts using the pre-extension value. This fix is enhanced by
      also looking back through value-preserving extensions within
      ft.isNonNegative to infer whether the extended value is known to be
      non-negative. Without this additional isNonNegative enhancement, this
      logic is rendered significantly less effective by the limitation
      discussed in the next paragraph.
      
      In Update, the application of facts to pre-extension values is limited
      to cases where the domain of the new fact is consistent with the type of
      the pre-extension value. There may be cases where this cross-domain
      passing of facts is valid, but distinguishing them from the invalid
      cases is difficult for me to reason about and to implement.
      Assessing which cases to allow requires details about the context and
      inferences behind the fact being applied which are not available
      within Update. Additional difficulty arises from the fact that the SSA
      does not curently differentiate extensions added by the compiler for
      indexing operations, extensions added by the compiler for implicit
      conversions, or explicit extensions from the source.
      
      Examples of some cases that would need to be filtered correctly for
      cross-domain facts:
      
      (1) A uint8 is zero-extended to int for indexing (a value-preserving
      zeroExt). When, if ever, can signed domain facts learned about the int be
      applied to the uint8?
      
      (2) An int8 is sign-extended to int16 (value-preserving) for an equality
      comparison. Equality comparison facts are currently always learned in both
      the signed and unsigned domains. When, if ever, can the unsigned facts
      learned about the int16, from the int16 != int16 comparison, be applied
      to the original int8?
      
      This is an alternative to CL 122695 and CL 174309. Compared to CL 122695,
      this CL differs in that the facts added about the pre-extension value will
      pass through the Update method, where additional inferences are processed
      (e.g. fence-post implications, see #29964). CL 174309 is limited to bounds
      checks, so is narrower in application, and makes the code harder to read.
      
      Fixes #26292.
      Fixes #29964.
      Fixes #15074
      
      Removes 238 bounds checks from std/cmd.
      
      Change-Id: I1f87c32ee672bfb8be397b27eab7a4c2f304893f
      Reviewed-on: https://go-review.googlesource.com/c/go/+/174704
      Run-TryBot: Zach Jones <zachj1@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarGiovanni Bajo <rasky@develer.com>
      69ff0ba7
    • Sergei Zagurskii's avatar
      reflect: optimize directlyAssignable to avoid rtype.Name call · 8057c088
      Sergei Zagurskii authored
      directlyAssignable invoked rtype.Name() just to compare its result
      to empty string. We really only need to check whether rtype has
      name. It can be done much cheaper, by checking tflagNamed.
      
      Benchmark: https://play.golang.org/p/V2BzESPuf2w
      name                   old time/op  new time/op  delta
      DirectlyAssignable-12  32.7ns ± 6%   6.6ns ± 6%  -79.80%  (p=0.008 n=5+5)
      
      Fixes #32186
      
      Change-Id: I1a2a167dbfddf319fba3015cb6a011bf010f99a8
      Reviewed-on: https://go-review.googlesource.com/c/go/+/178518
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      8057c088
    • Robert Griesemer's avatar
      cmd/compile/internal/syntax: better error recovery after missing expression · dca0d03b
      Robert Griesemer authored
      Don't skip closing parentheses of any kind after a missing
      expression. They are likely part of the lexical construct
      enclosing the expression.
      
      Fixes #33386.
      
      Change-Id: Ic0abc2037ec339a345ec357ccc724b7ad2a64c00
      Reviewed-on: https://go-review.googlesource.com/c/go/+/188502Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      dca0d03b
    • Jason A. Donenfeld's avatar
      ld: fix up header copy and paste error · 1a423bec
      Jason A. Donenfeld authored
      Some constants were added above the initial copyright blurb, and then
      later a new copyright blurb was added on top of that. So we wound up
      with two header sections, one of which contained a useful comment that
      became obscured. This commit fixes up that mistake.
      
      Change-Id: I8b9b8c34495cdceae959e151e8ccdee3137f6ca4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191841
      Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      1a423bec
  2. 26 Aug, 2019 9 commits