1. 17 Mar, 2016 5 commits
    • Martin Möhrmann's avatar
      fmt: separate unicode and integer formatting · d38275c7
      Martin Möhrmann authored
      Separate unicode formatting into its own fmt_unicode function.
      Remove the fmtUnicode wrapper and the f.unicode and f.uniQuote
      flags that are not needed anymore. Remove mangling and restoring
      of the precision and sharp flags.
      
      Removes the buffer copy needed for %#U by moving
      the character encoding before the number encoding.
      
      Changes the behavior of plus and space flag to have
      no effect instead of printing a plus or space before "U+".
      
      Always print at least four digits after "U+"
      even if precision is set to less than 4.
      
      Change-Id: If9a0ee79e9eca2c76f06a4e0fdd75d98393899ac
      Reviewed-on: https://go-review.googlesource.com/20574
      Run-TryBot: Rob Pike <r@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      d38275c7
    • Keith Randall's avatar
      cmd/compile: keep value use counts in SSA · 56e0ecc5
      Keith Randall authored
      Keep track of how many uses each Value has.  Each appearance in
      Value.Args and in Block.Control counts once.
      
      The number of uses of a value is generically useful to
      constrain rewrite rules.  For instance, we might want to
      prevent merging index operations into loads if the same
      index expression is used lots of times.
      
      But I have one use in particular for which the use count is required.
      We must make sure we don't combine ops with loads if the load has
      more than one use.  Otherwise, we may split a single load
      into multiple loads and that breaks perceived behavior in
      the presence of races.  In particular, the load of m.state
      in sync/mutex.go:Lock can't be done twice.  (I have a separate
      CL which triggers the mutex failure.  This CL has a test which
      demonstrates a similar failure.)
      
      Change-Id: Icaafa479239f48632a069d0c3f624e6ebc6b1f0e
      Reviewed-on: https://go-review.googlesource.com/20790
      Run-TryBot: Keith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarTodd Neal <todd@tneal.org>
      56e0ecc5
    • Dave Cheney's avatar
      cmd/compile/internal/gc: disable logProgs debug flag · cb1f2afc
      Dave Cheney authored
      Spotted while splunking in the compiler with GOGC=off.
      
      name       old time/op     new time/op     delta
      Template       407ms ± 5%      402ms ± 6%     ~           (p=0.301 n=20+20)
      GoTypes        1.33s ± 2%      1.29s ± 1%   -3.47%        (p=0.000 n=20+20)
      Compiler       6.21s ± 1%      5.91s ± 2%   -4.83%        (p=0.000 n=20+20)
      
      name       old alloc/op    new alloc/op    delta
      Template      66.8MB ± 0%     63.9MB ± 0%   -4.46%        (p=0.000 n=19+20)
      GoTypes        232MB ± 0%      220MB ± 0%   -5.16%        (p=0.000 n=19+17)
      Compiler      1.02GB ± 0%     0.97GB ± 0%   -5.81%        (p=0.000 n=20+20)
      
      name       old allocs/op   new allocs/op   delta
      Template        789k ± 0%       708k ± 0%  -10.28%        (p=0.000 n=19+20)
      GoTypes        2.49M ± 0%      2.20M ± 0%  -11.57%        (p=0.000 n=20+20)
      Compiler       10.8M ± 0%       9.4M ± 0%  -12.82%        (p=0.000 n=20+20)
      
      Change-Id: I76615cab912dde10595ca6ab9979ff6c5f1aec49
      Reviewed-on: https://go-review.googlesource.com/20782Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      cb1f2afc
    • Michael Hudson-Doyle's avatar
      cmd/link: do not add duplicate symbols to Allsym · 956e9e6c
      Michael Hudson-Doyle authored
      When building shared libraries, all symbols on Allsym are marked reachable.
      What I didn't realize was that this includes the ".dup" symbols created when
      "dupok" symbols are read from multiple package files. This breaks now because
      deadcode makes some assumptions that fail for these ".dup" symbols, but in any
      case was a bad idea -- I suspect this change makes libstd.so a bunch smaller,
      but creating it was broken before this CL so I can't be sure.
      
      This change simply stops adding these symbols to Allsym, which might make some
      of the many iterations over Allsym the linker does a touch quicker, although
      that's not the motivation here.
      
      Add a test that no symbols called ".dup" makes it into the runtime shared
      library.
      
      Fixes #14841
      
      Change-Id: I65dd6e88d150a770db2d01b75cfe5db5fd4f8d25
      Reviewed-on: https://go-review.googlesource.com/20780
      Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      956e9e6c
    • Matthew Dempsky's avatar
      cmd/compile: ignore receiver parameters in Eqtype · b2b5e779
      Matthew Dempsky authored
      Receiver parameters generally aren't relevant to the function
      signature type. In particular:
      
        1. When checking whether a type's method implements an interface's
           method, we specifically want to ignore the receiver parameters,
           because they'll be different.
      
        2. When checking interface type equality, interface methods always
           use the same "fakethis" *struct{} type as their receiver.
      
        3. Finally, method expressions and method values degenerate into
           receiver-less function types.
      
      The only case where we care about receiver types matching is in
      addmethod, which is easily handled by adding an extra Eqtype check of
      the receiver parameters. Also, added a test for this, since
      (surprisingly) there weren't any.
      
      As precedence, go/types.Identical ignores receiver parameters when
      comparing go/types.Signature values.
      
      Notably, this allows us to slightly simplify the "implements"
      function, which is used for checking whether type/interface t
      implements interface iface. Currently, cmd/compile actually works
      around Eqtype's receiver parameter checking by creating new throwaway
      TFUNC Types without the receiver parameter.
      
      (Worse, the compiler currently only provides APIs to build TFUNC Types
      from Nod syntax trees, so building those throwaway types also involves
      first building throwaway syntax trees.)
      
      Passes toolstash -cmp.
      
      Change-Id: Ib07289c66feacee284e016bc312e8c5ff674714f
      Reviewed-on: https://go-review.googlesource.com/20602Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      b2b5e779
  2. 16 Mar, 2016 35 commits
    • Josh Bleecher Snyder's avatar
      cmd/compile: further sinit.go cleanup · d33e37a7
      Josh Bleecher Snyder authored
      Follow-up to CL 20674.
      
      Passes toolstash -cmp.
      
      Change-Id: I065fd4cd80d996c1e6566773189401ca4630c1ca
      Reviewed-on: https://go-review.googlesource.com/20692
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDave Cheney <dave@cheney.net>
      d33e37a7
    • Josh Bleecher Snyder's avatar
      cmd/internal/obj/x86: estimate text size · 82625649
      Josh Bleecher Snyder authored
      We can’t perfectly predict how large the function
      will be, but we can make a safe overestimate.
      No significant CPU time changes.
      
      name       old alloc/op    new alloc/op    delta
      Template      67.7MB ± 0%     67.5MB ± 0%   -0.24%          (p=0.029 n=4+4)
      Unicode       43.9MB ± 0%     43.8MB ± 0%   -0.13%          (p=0.029 n=4+4)
      GoTypes        244MB ± 0%      244MB ± 0%   -0.28%          (p=0.029 n=4+4)
      Compiler      1.05GB ± 0%     1.05GB ± 0%   -0.38%          (p=0.029 n=4+4)
      
      name       old allocs/op   new allocs/op   delta
      Template        795k ± 0%       794k ± 0%   -0.14%          (p=0.029 n=4+4)
      Unicode         569k ± 0%       569k ± 0%     ~             (p=0.114 n=4+4)
      GoTypes        2.59M ± 0%      2.58M ± 0%   -0.11%          (p=0.029 n=4+4)
      Compiler       11.0M ± 0%      11.0M ± 0%   -0.09%          (p=0.029 n=4+4)
      
      Passes toolstash -cmp.
      
      Change-Id: I0a92ab04cba7520540ec58fe7189666d0e771454
      Reviewed-on: https://go-review.googlesource.com/20771Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      82625649
    • Josh Bleecher Snyder's avatar
      cmd/internal/obj: convert Symgrow to a method · fb950cd7
      Josh Bleecher Snyder authored
      Passes toolstash -cmp.
      
      Change-Id: I77a415a4e5d8de7eb902fb0866aaf8783259485a
      Reviewed-on: https://go-review.googlesource.com/20770Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      fb950cd7
    • James Bardin's avatar
      cmd/cgo: add C.CBytes · 5a34472d
      James Bardin authored
      Add a C.CBytes function to copy a Go byte slice into C memory. This
      returns an unsafe.Pointer, since that is what needs to be passed to
      C.free, and the data is often opaque bytes anyway.
      
      Fixes #14838
      
      Change-Id: Ic7bc29637eb6f1f5ee409b3898c702a59833a85a
      Reviewed-on: https://go-review.googlesource.com/20762Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      5a34472d
    • Austin Clements's avatar
      cmd/compile: omit write barrier when assigning global function · 3e54ca9a
      Austin Clements authored
      Currently we generate write barriers when the right side of an
      assignment is a global function. This doesn't fall into the existing
      case of storing an address of a global because we haven't lowered the
      function to a pointer yet.
      
      This write barrier is unnecessary, so eliminate it.
      
      Fixes #13901.
      
      Change-Id: Ibc10e00a8803db0fd75224b66ab94c3737842a79
      Reviewed-on: https://go-review.googlesource.com/20772
      Run-TryBot: Austin Clements <austin@google.com>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      3e54ca9a
    • Josh Bleecher Snyder's avatar
      cmd/compile: make sinit consts Go-ish · 4e75932c
      Josh Bleecher Snyder authored
      Passes toolstash -cmp.
      
      Change-Id: Ie11912a16d2cd54500e2f6e84316519b80e7c304
      Reviewed-on: https://go-review.googlesource.com/20672Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      4e75932c
    • Josh Bleecher Snyder's avatar
      cmd/internal/obj/x86: clean up asm buffer · e6ed3e8a
      Josh Bleecher Snyder authored
      c2go translated writing and advancing a pointer using slices.
      Switch to something more idiomatic.
      It is also more efficient, but not enough to matter.
      
      Change-Id: I67709632ac53253615a35365824ae97bbe5458d5
      Reviewed-on: https://go-review.googlesource.com/20767
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      e6ed3e8a
    • Josh Bleecher Snyder's avatar
      cmd/internal/obj/x86: clean up part of span6 · e248b96d
      Josh Bleecher Snyder authored
      Passes toolstash -cmp.
      
      Change-Id: I38eb507de2e9dc2cf01822e420bf31a91fb1b720
      Reviewed-on: https://go-review.googlesource.com/20766
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      e248b96d
    • Robert Griesemer's avatar
      cmd/compile: remove dead code handling '~' operator · c1a4fe8d
      Robert Griesemer authored
      The parser code was not reachable ever since some of the lexer cleanups.
      We could recognize '~' in the lexer, complain, and return a '^' instead,
      but it's been a few years since Go was new and this may have been a use-
      ful error. The lexer complains with "illegal character U+007E '~'" which
      is good enough.
      
      For #13244.
      
      Change-Id: Ie3283738486eb6f8462d594f2728ac98333c0520
      Reviewed-on: https://go-review.googlesource.com/20768Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      c1a4fe8d
    • Brad Fitzpatrick's avatar
      net/http: remove init func reference to ServeMux · 8540a1c4
      Brad Fitzpatrick authored
      Shrinks cmd/go by 30KB.
      
      Change-Id: Ied31192e85af76ebac743f8cc12bd9ef6ec5048f
      Reviewed-on: https://go-review.googlesource.com/20765Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      8540a1c4
    • Josh Bleecher Snyder's avatar
      cmd/compile: move LSym.RefIdx for better packing · 826831ac
      Josh Bleecher Snyder authored
      Change-Id: I0516d49ee8381c5e022d77c2fb41515c01c8a631
      Reviewed-on: https://go-review.googlesource.com/20764Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarDavid Crawshaw <crawshaw@golang.org>
      826831ac
    • Josh Bleecher Snyder's avatar
      cmd/internal/obj: remove LSym.Etext · 31a9e505
      Josh Bleecher Snyder authored
      Use a local variable instead.
      
      Passes toolstash -cmp.
      
      Change-Id: I9623a40ff0d568f11afd1279b6aaa1c33eda644c
      Reviewed-on: https://go-review.googlesource.com/20730Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      31a9e505
    • Josh Bleecher Snyder's avatar
      cmd/internal/obj: remove LSym.Next · dd2ba0c7
      Josh Bleecher Snyder authored
      Instead, use a slice.
      
      Passes toolstash -cmp.
      
      Change-Id: I889fdb4ae997416f907522f549b96506be13bec7
      Reviewed-on: https://go-review.googlesource.com/20699Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      dd2ba0c7
    • Josh Bleecher Snyder's avatar
      cmd/internal/obj: remove LSym.Value · 61b9315d
      Josh Bleecher Snyder authored
      It is unused.
      
      Passes toolstash -cmp.
      
      Change-Id: I22ae2bb432ce6be377dea43cf018ffccb6e95f37
      Reviewed-on: https://go-review.googlesource.com/20698Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      61b9315d
    • Austin Clements's avatar
      runtime: shrink stacks during concurrent mark · f11e4eb5
      Austin Clements authored
      Currently we shrink stacks during STW mark termination because it used
      to be unsafe to shrink them concurrently. For some programs, this
      significantly increases pause time: stack shrinking costs ~5ms/MB
      copied plus 2µs/shrink.
      
      Now that we've made it safe to shrink a stack without the world being
      stopped, shrink them during the concurrent mark phase.
      
      This reduces the STW time in the program from issue #12967 by an order
      of magnitude and brings it from over the 10ms goal to well under:
      
      name           old 95%ile-markTerm-time  new 95%ile-markTerm-time  delta
      Stackshrink-4               23.8ms ±60%               1.80ms ±39%  -92.44%  (p=0.008 n=5+5)
      
      Fixes #12967.
      
      This slows down the go1 and garbage benchmarks overall by < 0.5%.
      
      name              old time/op  new time/op  delta
      XBenchGarbage-12  2.48ms ± 1%  2.49ms ± 1%  +0.45%  (p=0.005 n=25+21)
      
      name                      old time/op    new time/op    delta
      BinaryTree17-12              2.93s ± 2%     2.97s ± 2%  +1.34%  (p=0.002 n=19+20)
      Fannkuch11-12                2.51s ± 1%     2.59s ± 0%  +3.09%  (p=0.000 n=18+18)
      FmtFprintfEmpty-12          51.1ns ± 2%    51.5ns ± 1%    ~     (p=0.280 n=20+17)
      FmtFprintfString-12          175ns ± 1%     169ns ± 1%  -3.01%  (p=0.000 n=20+20)
      FmtFprintfInt-12             160ns ± 1%     160ns ± 0%  +0.53%  (p=0.000 n=20+20)
      FmtFprintfIntInt-12          265ns ± 0%     266ns ± 1%  +0.59%  (p=0.000 n=20+20)
      FmtFprintfPrefixedInt-12     237ns ± 1%     238ns ± 1%  +0.44%  (p=0.000 n=20+20)
      FmtFprintfFloat-12           326ns ± 1%     341ns ± 1%  +4.55%  (p=0.000 n=20+19)
      FmtManyArgs-12              1.01µs ± 0%    1.02µs ± 0%  +0.43%  (p=0.000 n=20+19)
      GobDecode-12                8.41ms ± 1%    8.30ms ± 2%  -1.22%  (p=0.000 n=20+19)
      GobEncode-12                6.66ms ± 1%    6.68ms ± 0%  +0.30%  (p=0.000 n=18+19)
      Gzip-12                      322ms ± 1%     322ms ± 1%    ~     (p=1.000 n=20+20)
      Gunzip-12                   42.8ms ± 0%    42.9ms ± 0%    ~     (p=0.174 n=20+20)
      HTTPClientServer-12         69.7µs ± 1%    70.6µs ± 1%  +1.20%  (p=0.000 n=20+20)
      JSONEncode-12               16.8ms ± 0%    16.8ms ± 1%    ~     (p=0.154 n=19+19)
      JSONDecode-12               65.1ms ± 0%    65.3ms ± 1%  +0.34%  (p=0.003 n=20+20)
      Mandelbrot200-12            3.93ms ± 0%    3.92ms ± 0%    ~     (p=0.396 n=19+20)
      GoParse-12                  3.66ms ± 1%    3.65ms ± 1%    ~     (p=0.117 n=16+18)
      RegexpMatchEasy0_32-12      85.0ns ± 2%    85.5ns ± 2%    ~     (p=0.143 n=20+20)
      RegexpMatchEasy0_1K-12       267ns ± 1%     267ns ± 1%    ~     (p=0.867 n=20+17)
      RegexpMatchEasy1_32-12      83.3ns ± 2%    83.8ns ± 1%    ~     (p=0.068 n=20+20)
      RegexpMatchEasy1_1K-12       432ns ± 1%     432ns ± 1%    ~     (p=0.804 n=20+19)
      RegexpMatchMedium_32-12      133ns ± 0%     133ns ± 0%    ~     (p=1.000 n=20+20)
      RegexpMatchMedium_1K-12     40.3µs ± 1%    40.4µs ± 1%    ~     (p=0.319 n=20+19)
      RegexpMatchHard_32-12       2.10µs ± 1%    2.10µs ± 1%    ~     (p=0.723 n=20+18)
      RegexpMatchHard_1K-12       63.0µs ± 0%    63.0µs ± 0%    ~     (p=0.158 n=19+17)
      Revcomp-12                   461ms ± 1%     476ms ± 8%  +3.29%  (p=0.002 n=20+20)
      Template-12                 80.1ms ± 1%    79.3ms ± 1%  -1.00%  (p=0.000 n=20+20)
      TimeParse-12                 360ns ± 0%     360ns ± 0%    ~     (p=0.802 n=18+19)
      TimeFormat-12                374ns ± 1%     372ns ± 0%  -0.77%  (p=0.000 n=20+19)
      [Geo mean]                  61.8µs         62.0µs       +0.40%
      
      Change-Id: Ib60cd46b7a4987e07670eb271d22f6cee5802842
      Reviewed-on: https://go-review.googlesource.com/20044Reviewed-by: default avatarKeith Randall <khr@golang.org>
      f11e4eb5
    • Austin Clements's avatar
      runtime: generalize work.finalizersDone to work.markrootDone · c14d25c6
      Austin Clements authored
      We're about to add another root marking job that needs to happen only
      during the first markroot pass (whether that's concurrent or STW),
      just like finalizer scanning. Rather than introducing another flag
      that has the same value as finalizersDone, just rename finalizersDone
      to markrootDone.
      
      Change-Id: I535356c6ea1f3734cb5b6add264cb7bf48de95e8
      Reviewed-on: https://go-review.googlesource.com/20043Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      c14d25c6
    • Austin Clements's avatar
      runtime: make shrinkstack concurrent-safe · 276b1777
      Austin Clements authored
      Currently shinkstack is only safe during STW because it adjusts
      channel-related stack pointers and moves send/receive stack slots
      without synchronizing with the channel code. Make it safe to use when
      the world isn't stopped by:
      
      1) Locking all channels the G is blocked on while adjusting the sudogs
         and copying the area of the stack that may contain send/receive
         slots.
      
      2) For any stack frames that may contain send/receive slot, using an
         atomic CAS to adjust pointers to prevent races between adjusting a
         pointer in a receive slot and a concurrent send writing to that
         receive slot.
      
      In principle, the synchronization could be finer-grained. For example,
      we considered synchronizing around the sudogs, which would allow
      channel operations involving other Gs to continue if the G being
      shrunk was far enough down the send/receive queue. However, using the
      channel lock means no additional locks are necessary in the channel
      code. Furthermore, the stack shrinking code holds the channel lock for
      a very short time (much less than the time required to shrink the
      stack).
      
      This does not yet make stack shrinking concurrent; it merely makes
      doing so safe.
      
      This has negligible effect on the go1 and garbage benchmarks.
      
      For #12967.
      
      Change-Id: Ia49df3a8a7be4b36e365aac4155a2416b94b988c
      Reviewed-on: https://go-review.googlesource.com/20042Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      276b1777
    • Austin Clements's avatar
      runtime: define lock order between G status and channel lock · d45bf722
      Austin Clements authored
      Currently, locking a G's stack by setting its status to _Gcopystack or
      _Gscan is unordered with respect to channel locks. However, when we
      make stack shrinking concurrent, stack shrinking will need to lock the
      G and then acquire channel locks, which imposes an order on these.
      
      Document this lock ordering and fix closechan to respect it.
      Everything else already happens to respect it.
      
      For #12967.
      
      Change-Id: I4dd02675efffb3e7daa5285cf75bf24f987d90d4
      Reviewed-on: https://go-review.googlesource.com/20041Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      d45bf722
    • Austin Clements's avatar
      runtime: protect sudog.elem with hchan.lock · db72b41b
      Austin Clements authored
      Currently sudog.elem is never accessed concurrently, so in several
      cases we drop the channel lock just before reading/writing the
      sent/received value from/to sudog.elem. However, concurrent stack
      shrinking is going to have to adjust sudog.elem to point to the new
      stack, which means it needs a way to synchronize with accesses to
      sudog.elem. Hence, add sudog.elem to the fields protected by
      hchan.lock and scoot the unlocks down past the uses of sudog.elem.
      
      While we're here, better document the channel synchronization rules.
      
      For #12967.
      
      Change-Id: I3ad0ca71f0a74b0716c261aef21b2f7f13f74917
      Reviewed-on: https://go-review.googlesource.com/20040Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      db72b41b
    • Austin Clements's avatar
      runtime: fix transient _Gwaiting states in newstack · 3c2a21ff
      Austin Clements authored
      With concurrent stack shrinking, the stack can move the instant after
      a G enters _Gwaiting. There are only two places that put a G into
      _Gwaiting: gopark and newstack. We fixed uses of gopark. This commit
      fixes newstack by simplifying its G transitions and, in particular,
      eliminating or narrowing the transient _Gwaiting states it passes
      through so it's clear nothing in the G is accessed while in _Gwaiting.
      
      For #12967.
      
      Change-Id: I2440ead411d2bc61beb1e2ab020ebe3cb3481af9
      Reviewed-on: https://go-review.googlesource.com/20039Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      3c2a21ff
    • Austin Clements's avatar
      runtime: never pass stack pointers to gopark · 8fb182d0
      Austin Clements authored
      gopark calls the unlock function after setting the G to _Gwaiting.
      This means it's generally unsafe to access the G's stack from the
      unlock function because the G may start running on another P. Once we
      start shrinking stacks concurrently, a stack shrink could also move
      the stack the moment after it enters _Gwaiting and before the unlock
      function is called.
      
      Document this restriction and fix the two places where we currently
      violate it.
      
      This is unlikely to be a problem in practice for these two places
      right now, but they're already skating on thin ice. For example, the
      following sequence could in principle cause corruption, deadlock, or a
      panic in the select code:
      
      On M1/P1:
      1. G1 selects on channels A and B.
      2. selectgoImpl calls gopark.
      3. gopark puts G1 in _Gwaiting.
      4. gopark calls selparkcommit.
      5. selparkcommit releases the lock on channel A.
      
      On M2/P2:
      6. G2 sends to channel A.
      7. The send puts G1 in _Grunnable and puts it on P2's run queue.
      8. The scheduler runs, selects G1, puts it in _Grunning, and resumes G1.
      9. On G1, the sellock immediately following the gopark gets called.
      10. sellock grows and moves the stack.
      
      On M1/P1:
      11. selparkcommit continues to scan the lock order for the next
      channel to unlock, but it's now reading from a freed (and possibly
      reused) stack.
      
      This shouldn't happen in practice because step 10 isn't the first call
      to sellock, so the stack should already be big enough. However, once
      we start shrinking stacks concurrently, this reasoning won't work any
      more.
      
      For #12967.
      
      Change-Id: I3660c5be37e5be9f87433cb8141bdfdf37fadc4c
      Reviewed-on: https://go-review.googlesource.com/20038Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      8fb182d0
    • Austin Clements's avatar
      runtime: put g.waiting list in lock order · 005140a7
      Austin Clements authored
      Currently the g.waiting list created by a select is in poll order.
      However, nothing depends on this, and we're going to need access to
      the channel lock order in other places shortly, so modify select to
      put the waiting list in channel lock order.
      
      For #12967.
      
      Change-Id: If0d38816216ecbb37a36624d9b25dd96e0a775ec
      Reviewed-on: https://go-review.googlesource.com/20037Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      005140a7
    • Austin Clements's avatar
      runtime: use indexes for select lock order · 26594c3d
      Austin Clements authored
      Currently the select lock order is a []*hchan. We're going to need to
      refer to things other than the channel itself in lock order shortly,
      so switch this to a []uint16 of indexes into the select cases. This
      parallels the existing representation for the poll order.
      
      Change-Id: I89262223fe20b4ddf5321592655ba9eac489cda1
      Reviewed-on: https://go-review.googlesource.com/20036Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      26594c3d
    • Austin Clements's avatar
      runtime: record channel in sudog · e4a95b63
      Austin Clements authored
      Given a G, there's currently no way to find the channel it's blocking
      on. We'll need this information to fix a (probably theoretical) bug in
      select and to implement concurrent stack shrinking, so record the
      channel in the sudog.
      
      For #12967.
      
      Change-Id: If8fb63a140f1d07175818824d08c0ebeec2bdf66
      Reviewed-on: https://go-review.googlesource.com/20035Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      e4a95b63
    • Austin Clements's avatar
      runtime: perform gcMarkRootCheck during STW in checkmark mode · d7cedc4b
      Austin Clements authored
      gcMarkRootCheck is too expensive to do during mark termination.
      However, since it's a useful check and it complements checkmark mode
      nicely, enable it during mark termination is checkmark is enabled.
      
      Change-Id: Icd9039e85e6e9d22747454441b50f1cdd1412202
      Reviewed-on: https://go-review.googlesource.com/20663Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      d7cedc4b
    • Brad Fitzpatrick's avatar
      net/http: use dynamic type assertion to remove HTTP server code from cmd/go · a96884cf
      Brad Fitzpatrick authored
      I was wondering why cmd/go includes the HTTP server implementations.
      
      Dumping the linker's deadcode dependency graph into a file and doing
      some graph analysis, I found that the only reason cmd/go included an
      HTTP server was because the maxBytesReader type (used by both the HTTP
      transport & HTTP server) did a static type assertion to an HTTP server
      type.
      
      Changing it to a interface type assertion reduces the size of cmd/go
      by 533KB (5.2%)
      
      On linux/amd64, cmd/go goes from 10549200 to 10002624 bytes.
      
      Add a test too so this doesn't regress. The test uses cmd/go as the
      binary to test (a binary which needs the HTTP client but not the HTTP
      server), but this change and test are equally applicable to any such
      program.
      
      Change-Id: I93865f43ec03b06d09241fbd9ea381817c2909c5
      Reviewed-on: https://go-review.googlesource.com/20763Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      Reviewed-by: default avatarJosh Bleecher Snyder <josharian@gmail.com>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarDavid Crawshaw <crawshaw@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      a96884cf
    • Robert Griesemer's avatar
      cmd/compile: faster parameter parsing with no OKEY nodes · bb3b1021
      Robert Griesemer authored
      Step 2 of stream-lining parameter parsing
      
      - do parameter validity checks in parser
      - two passes instead of multiple (and theoretically quadratic) passes
        when checking parameters
      - removes the need for OKEY and some ONONAME nodes in those passes
      
      This removes allocation of ~123K OKEY (incl. some ONONAME) nodes
      out of a total of ~10M allocated nodes when running make.bash, or
      a reduction of the number of alloacted nodes by ~1.2%.
      
      Change-Id: I4a8ec578d0ee2a7b99892ac6b92e56f8e0415f03
      Reviewed-on: https://go-review.googlesource.com/20748Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      Run-TryBot: Robert Griesemer <gri@golang.org>
      bb3b1021
    • Alan Donovan's avatar
      runtime/debug: clarify WriteHeapDump STW behavior · ed73efbb
      Alan Donovan authored
      Change-Id: I049d2596fe8ce0e93391599f5c224779fd8e316f
      Reviewed-on: https://go-review.googlesource.com/20761Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      ed73efbb
    • Robert Griesemer's avatar
      cmd/compile: factor parameter parsing · 9f301643
      Robert Griesemer authored
      Step 1 of streamlining parameter parsing.
      
      Change-Id: If9fd38295ccc08aafc7f1d26188d0926dd73058b
      Reviewed-on: https://go-review.googlesource.com/20747Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      Run-TryBot: Robert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      9f301643
    • Todd Neal's avatar
      cmd/compile: fold constants from lsh/rsh/lsh and rsh/lsh/rsh · c8b148e7
      Todd Neal authored
      Fixes #14825
      
      Change-Id: Ib44d80579a55c15d75ea2ad1ef54efa6ca66a9a6
      Reviewed-on: https://go-review.googlesource.com/20745
      Run-TryBot: Todd Neal <todd@tneal.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      c8b148e7
    • Martin Möhrmann's avatar
      fmt: reuse buffer and add range checks for %c and %q · 42cd69f5
      Martin Möhrmann authored
      Use The fmt internal buffer for character formatting instead of
      the pp Printer rune decoding buffer.
      
      Uses an uint64 instead of int64 argument to fmt_c and fmt_qc for easier
      range checks since no valid runes are represented by negative numbers or
      are above 0x10ffff.
      
      Add range checks to fmt_c and fmt_qc to guarantee that a RuneError
      character is returned by the functions for any invalid code point
      in range uint64. For invalid code points in range utf8.MaxRune
      the used utf8 and strconv functions already return a RuneError.
      
      Change-Id: I9772f804dfcd79c3826fa7f6c5ebfbf4b5304a51
      Reviewed-on: https://go-review.googlesource.com/20373
      Run-TryBot: Rob Pike <r@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      42cd69f5
    • Martin Möhrmann's avatar
      fmt: cleanup %p and %T code paths · b8ddcc0a
      Martin Möhrmann authored
      Remove check for %p and %T in printValue.
      These verbs are not recursive and are handled already in
      printArg which is called on any argument before printValue.
      
      Format the type string for %T directly instead of invoking
      the more complex printArg with %s on the type string.
      
      Decouple the %T tests from variables declared in scan_test.go.
      
      Change-Id: Ibd51566bd4cc1a260ce6d052f36382ed05020b48
      Reviewed-on: https://go-review.googlesource.com/20622
      Run-TryBot: Rob Pike <r@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      b8ddcc0a
    • Aliaksandr Valialkin's avatar
      cmd/vet: added some missing copylock checks · fee86e4a
      Aliaksandr Valialkin authored
      Fixes #14664
      
      Change-Id: I8bda2435857772f590859808904c48d768b87d46
      Reviewed-on: https://go-review.googlesource.com/20254
      Run-TryBot: Rob Pike <r@golang.org>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      fee86e4a
    • Rob Pike's avatar
      path: fix up bizarre test · 55567d37
      Rob Pike authored
      The Join test was doing something remarkable and unnecessary instead of
      just using ... on a slice. Maybe it was an editing relic.
      
      Fix it by deleting the monstrosity.
      
      Change-Id: I5b90c6d539d334a9c27e57d26dacd831721cfcfe
      Reviewed-on: https://go-review.googlesource.com/20727Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      55567d37
    • Martin Möhrmann's avatar
      fmt: clear flags before printing extra argument errors · cf08eadf
      Martin Möhrmann authored
      Do a reset of the fmt flags before printing the extra argument
      error message to prevent a malformed printing of extra arguments.
      
      Regroup tests for extra argument error strings.
      
      Change-Id: Ifd97f5ca36f6c97ed5a380d975cf154d17997d3f
      Reviewed-on: https://go-review.googlesource.com/20571
      Run-TryBot: Rob Pike <r@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      cf08eadf