1. 21 Apr, 2017 27 commits
    • Damien Lespiau's avatar
      cmd/compile: provide a way to auto-discover -d debug keys · f9be63b9
      Damien Lespiau authored
      Currently one needs to refer to the sources to have a list of accepted
      debug keys. We can copy what 'ssa/help' does and introspect the list of
      debug keys to print a more detailed help:
      
          $ go tool compile -d help
          usage: -d arg[,arg]* and arg is <key>[=<value>]
      
          <key> is one of:
      
          	append    	print information about append compilation
          	closure   	print information about closure compilation
          	disablenil	disable nil checks
          	dclstack  	run internal dclstack check
          	gcprog    	print dump of GC programs
          	nil       	print information about nil checks
          	panic     	do not hide any compiler panic
          	slice     	print information about slice compilation
          	typeassert	print information about type assertion inlining
          	wb        	print information about write barriers
          	export    	print export data
          	pctab     	print named pc-value table
          	ssa/help  	print help about SSA debugging
      
          <value> is key-specific.
      
          Key "pctab" supports values:
          	"pctospadj", "pctofile", "pctoline", "pctoinline", "pctopcdata"
      
      For '-d help' to be discoverable, a hint is given in the -d flag
      description.
      
      A last thing, today at least one go file needs to be provided to get to
      the code printing ssa/help.
      
        $ go tool compile -d ssa/help foo.go
      
      Add a check so one can just do '-d help' or '-d ssa/help'
      
      Caught by trybot: I needed to update fmt_test.go as I'm introducing the
      usage of %-*s in a format string.
      
      Fixes #20041
      
      Change-Id: Ib2858b038c1bcbe644aa3b1a371009710c6d957d
      Reviewed-on: https://go-review.googlesource.com/41091
      Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      f9be63b9
    • Keith Randall's avatar
      cmd/compile: experiment which clobbers all dead pointer fields · 1e72bf62
      Keith Randall authored
      The experiment "clobberdead" clobbers all pointer fields that the
      compiler thinks are dead, just before and after every safepoint.
      Useful for debugging the generation of live pointer bitmaps.
      
      Helped find the following issues:
      Update #15936
      Update #16026
      Update #16095
      Update #18860
      
      Change-Id: Id1d12f86845e3d93bae903d968b1eac61fc461f9
      Reviewed-on: https://go-review.googlesource.com/23924
      Run-TryBot: Keith Randall <khr@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      1e72bf62
    • Austin Clements's avatar
      runtime/debug: increase threshold on TestSetGCPercent · e5162275
      Austin Clements authored
      Currently TestSetGCPercent checks that NextGC is within 10 MB of the
      expected value. For some reason it's much noisier on some of the
      builders. To get these passing again, raise the threshold to 20 MB.
      
      Change-Id: I14e64025660d782d81ff0421c1eb898f416e11fe
      Reviewed-on: https://go-review.googlesource.com/41374
      Run-TryBot: Austin Clements <austin@google.com>
      Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      e5162275
    • Robert Griesemer's avatar
      cmd/compile/internal/types: remove unused lineno arguments for PushDcl/MarkDcl · eaf02e1f
      Robert Griesemer authored
      More steps towards simpler symbol handling:
      
      - Pushdcl's incoming pos argument, saved in a newly pushed *Sym, was always
        immediately overwritten by the Lastlineno value of the saved *Sym.
      
      - Markdcl's incoming pos argument, saved in the stack mark *Sym, was not
        restored when the stack mark was popped.
      
      - Popdcl always maintained the most recent Lastlineno for a *Sym given
        by package and name, making it unnecessary to save Lastlineno in the
        first place. Removed Lastlineno from the set of fields that need saving,
        and simplified Popdcl.
      
      Change-Id: Ie93da1fbd780dcafc2703044e781c0c6298df569
      Reviewed-on: https://go-review.googlesource.com/41390
      Run-TryBot: Robert Griesemer <gri@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      eaf02e1f
    • Austin Clements's avatar
      runtime/debug: don't trigger a GC on SetGCPercent · 227fff2e
      Austin Clements authored
      Currently SetGCPercent forces a GC in order to recompute GC pacing.
      Since we can now recompute pacing on the fly using gcSetTriggerRatio,
      change SetGCPercent (really runtime.setGCPercent) to go through
      gcSetTriggerRatio and not trigger a GC.
      
      Fixes #19076.
      
      Change-Id: Ib30d7ab1bb3b55219535b9f238108f3d45a1b522
      Reviewed-on: https://go-review.googlesource.com/39835
      Run-TryBot: Austin Clements <austin@google.com>
      Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      227fff2e
    • Austin Clements's avatar
      runtime/debug: expand SetGCPercent test · d9308cbb
      Austin Clements authored
      The current SetGCPercent test is, shall we say, minimal.
      
      Expand it to check that the GC target is actually computed and updated
      correctly.
      
      For #19076.
      
      Change-Id: I6e9b2ee0ef369f22f72e43b58d89e9f1e1b73b1b
      Reviewed-on: https://go-review.googlesource.com/39834
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      d9308cbb
    • Austin Clements's avatar
      runtime: make gcSetTriggerRatio work at any time · 1c4f3c5e
      Austin Clements authored
      This changes gcSetTriggerRatio so it can be called even during
      concurrent mark or sweep. In this case, it will adjust the pacing of
      the current phase, accounting for progress that has already been made.
      
      To make this work for concurrent sweep, this introduces a "basis" for
      the pagesSwept count, much like the basis we just introduced for
      heap_live. This lets gcSetTriggerRatio shift the basis to the current
      heap_live and pagesSwept and compute a slope from there to completion.
      This avoids creating a discontinuity where, if the ratio has
      increased, there has to be a flurry of sweep activity to catch up.
      Instead, this creates a continuous, piece-wise linear function as
      adjustments are made.
      
      For #19076.
      
      Change-Id: Ibcd76aeeb81ff4814b00be7cbd3530b73bbdbba9
      Reviewed-on: https://go-review.googlesource.com/39833
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      1c4f3c5e
    • Austin Clements's avatar
      runtime: drive proportional sweep directly off heap_live · a5eb3dce
      Austin Clements authored
      Currently, proportional sweep maintains its own count of how many
      bytes have been allocated since the beginning of the sweep cycle so it
      can compute how many pages need to be swept for a given allocation.
      
      However, this requires a somewhat complex reimbursement scheme since
      proportional sweep must be done before a span is allocated, but we
      don't know how many bytes to charge until we've allocated a span. This
      means that the allocated byte count used by proportional sweep can go
      up and down, which has led to underflow bugs in the past (#18043) and
      is going to interfere with adjusting sweep pacing on-the-fly (for #19076).
      
      This approach also means we're maintaining a statistic that is very
      closely related to heap_live, but has a different 0 value. This is
      particularly confusing because the sweep ratio is computed based on
      heap_live, so you have to understand that these two statistics are
      very closely related.
      
      Replace all of this and compute the sweep debt directly from the
      current value of heap_live. To make this work, we simply save the
      value of heap_live when the sweep ratio is computed to use as a
      "basis" for later computing the sweep debt.
      
      This eliminates the need for reimbursement as well as the code for
      maintaining the sweeper's version of the live heap size.
      
      For #19076.
      
      Coincidentally fixes #18043, since this eliminates sweep reimbursement
      entirely.
      
      Change-Id: I1f931ddd6e90c901a3972c7506874c899251dc2a
      Reviewed-on: https://go-review.googlesource.com/39832
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      a5eb3dce
    • Austin Clements's avatar
      runtime: consolidate all trigger-derived computations · ee175afa
      Austin Clements authored
      Currently, the computations that derive controls from the GC trigger
      are spread across several parts of the mark termination code.
      Consolidate computing the absolute trigger, the heap goal, and sweep
      pacing into a single function called at the end of mark termination.
      
      Unlike the code being consolidated, this has to be more careful about
      negative gcpercent. Many of the consolidated code paths simply didn't
      execute if GC was off.
      
      This is a step toward being able to change the GC trigger ratio in the
      middle of concurrent sweeping and marking. For this commit, we try to
      stick close to the original structure of the code that's being
      consolidated, so it doesn't yet support mid-cycle adjustments.
      
      For #19076.
      
      Change-Id: Ic5335be04b96ad20e70d53d67913a86bd6b31456
      Reviewed-on: https://go-review.googlesource.com/39831
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      ee175afa
    • Austin Clements's avatar
      runtime: rationalize triggerRatio · 49a412a5
      Austin Clements authored
      gcController.triggerRatio is the only field in gcController that
      persists across cycles. As global mutable state, the places where it
      written and read are spread out, making it difficult to see that
      updates and downstream calculations are done correctly.
      
      Improve this situation by doing two things:
      
      1) Move triggerRatio to memstats so it lives with the other
      trigger-related fields and makes gcController entirely transient
      state.
      
      2) Commit the new trigger ratio during mark termination when we
      compute other next-cycle controls, including the absolute trigger.
      This forces us to explicitly thread the new trigger ratio from
      gcController.endCycle to mark termination, so we're not just pulling
      it out of global state.
      
      Change-Id: I6669932f8039a8c0ef46a3f2a8c537db72e578aa
      Reviewed-on: https://go-review.googlesource.com/39830
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      49a412a5
    • Austin Clements's avatar
      runtime: consistently use atomic loads for heap_live · 9d36163c
      Austin Clements authored
      heap_live is updated atomically without locking, so we should also use
      atomic loads to read it. Fix the reads of heap_live that happen
      outside of STW to be atomic.
      
      Change-Id: Idca9451c348168c2a792a9499af349833a3c333f
      Reviewed-on: https://go-review.googlesource.com/41371
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      9d36163c
    • Hiroshi Ioka's avatar
      cmd/cgo: remove duplicate mangle definition · 44fe0820
      Hiroshi Ioka authored
      Change-Id: I0f8c695146b39cff72ca2374f861f3e9f72b0f77
      Reviewed-on: https://go-review.googlesource.com/41314Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      44fe0820
    • Austin Clements's avatar
      cmd/compile: convert ishairy into a visitor · e52d317d
      Austin Clements authored
      The inliner's ishairy passes a budget and a reason down through the
      walk. Lift these into a visitor object and turn ishairy and its
      helpers into methods.
      
      This will make it easy to add more state.
      
      Change-Id: Ic6ae246e1affd67ed283c3205f9595ae33e22215
      Reviewed-on: https://go-review.googlesource.com/41151
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      e52d317d
    • Josh Bleecher Snyder's avatar
      cmd/compile: move typepkg to package types · 24c52ee5
      Josh Bleecher Snyder authored
      Response to code review feedback on CL 40693.
      
      It is now only accessible by types.TypePkgLookup.
      
      Passes toolstash-check.
      
      Change-Id: I0c422c1a271f97467ae38de53af9dc33f4b31bdb
      Reviewed-on: https://go-review.googlesource.com/41304
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      24c52ee5
    • Josh Bleecher Snyder's avatar
      cmd/compile: unexport types.Sym.LSym · 0d50a49f
      Josh Bleecher Snyder authored
      Response to code review feedback on CL 40693.
      
      Remove the final reference to it from package gc,
      and manually unexport.
      
      Passes toolstash-check.
      
      Change-Id: I7fc48edd43263d8f7c56b47aeb7573408463dc22
      Reviewed-on: https://go-review.googlesource.com/41303
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      0d50a49f
    • Josh Bleecher Snyder's avatar
      cmd/compile: move Linksym, linksymname, and isblanksym to types package · 30940e2c
      Josh Bleecher Snyder authored
      Response to code review feedback on CL 40693.
      
      This CL was prepared by:
      
      (1) manually adding new implementations and the Ctxt var to package types
      
      (2) running eg with template:
      
      func before(s *types.Sym) *obj.LSym { return gc.Linksym(s) }
      func after(s *types.Sym) *obj.LSym  { return s.Linksym() }
      
      (3) running gofmt -r:
      
      gofmt -r 'isblanksym(a) -> a.IsBlank()'
      
      (4) manually removing old implementations from package gc
      
      Passes toolstash-check.
      
      Change-Id: I39c35def7cae5bcbcc7c77253e5d2b066b981dea
      Reviewed-on: https://go-review.googlesource.com/41302
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      30940e2c
    • Josh Bleecher Snyder's avatar
      cmd/compile: simplify sharedProgArray init · 5aebeaac
      Josh Bleecher Snyder authored
      Per code review feedback on CL 40693.
      
      Change-Id: I38c522022a3c2f3e61ea90181391edb5c178916e
      Reviewed-on: https://go-review.googlesource.com/41300
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      5aebeaac
    • David Lazar's avatar
      testing: use function names to identify helpers · 318812e7
      David Lazar authored
      Previously, helpers were identified by entry PC, but this breaks if the
      helper is inlined (as in notHelperCallingHelper). Instead, identify
      helpers by function name (with package path). Now TestTBHelper and
      TestTBHelperParallel pass with -l=4.
      
      To keep the code unified, this change makes it so that the runner
      is also identified by function name instead of entry PC.
      
      Change-Id: I1b1987fc49d114e69d075fab56aeeacd5294982b
      Reviewed-on: https://go-review.googlesource.com/41257
      Run-TryBot: David Lazar <lazard@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJosh Bleecher Snyder <josharian@gmail.com>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      318812e7
    • David Lazar's avatar
      cmd/compile: don't inline functions that call runtime.getcaller{pc,sp} · 2397cd0f
      David Lazar authored
      runtime.getcaller{pc,sp} expect their argument to be a pointer to the
      caller's first function argument. This assumption breaks when the caller
      is inlined. For example, with -l=4, calls to runtime.entersyscall (which
      calls getcallerpc) are inlined and that breaks multiple cgo tests.
      
      This change modifies the compiler to refuse to inline functions that
      call runtime.getcaller{pc,sp}. Alternatively, we could mark these
      functions //go:noinline but that limits optimization opportunities if
      the calls to getcaller{pc,sp} are eliminated as dead code.
      
      Previously TestCgoPprofPIE, TestCgoPprof, and TestCgoCallbackGC failed
      with -l=4. Now all of the runtime tests pass with -l=4.
      
      Change-Id: I258bca9025e20fc451e673a18f862b5da1e07ae7
      Reviewed-on: https://go-review.googlesource.com/40998
      Run-TryBot: David Lazar <lazard@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      2397cd0f
    • Austin Clements's avatar
      runtime: inform arena placement using sbrk(0) · bb6309cd
      Austin Clements authored
      On 32-bit architectures (or if we fail to map a 64-bit-style arena),
      we try to map the heap arena just above the end of the process image.
      While we can accept any address, using lower addresses is preferable
      because lower addresses cause us to map less of the heap bitmap.
      
      However, if a program is linked against C code that has global
      constructors, those constructors may call brk/sbrk to allocate memory
      (e.g., many C malloc implementations do this for small allocations).
      The brk also starts just above the process image, so this may adjust
      the brk past the beginning of where we want to put the heap arena. In
      this case, the kernel will pick a different address for the arena and
      it will usually be very high (at least, as these things go in a 32-bit
      address space).
      
      Fix this by consulting the current value of the brk and using this in
      addition to the end of the process image to compute the initial arena
      placement.
      
      This is implemented only on Linux currently, since we have no evidence
      that it's an issue on any other OSes.
      
      Fixes #19831.
      
      Change-Id: Id64b45d08d8c91e4f50d92d0339146250b04f2f8
      Reviewed-on: https://go-review.googlesource.com/39810
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      bb6309cd
    • Josh Bleecher Snyder's avatar
      cmd/compile: break up large value rewrite functions · fc7b83d1
      Josh Bleecher Snyder authored
      This makes the cmd/compile/internal/ssa package
      compile much faster, and has no impact
      on the speed of the compiler.
      
      The chunk size was selected empirically,
      in that at chunk size 10, the object
      file was smaller than at chunk size 5 or 20.
      
      name  old time/op       new time/op       delta
      SSA         7.33s ± 5%        5.64s ± 1%  -23.10%  (p=0.000 n=10+10)
      
      name  old user-time/op  new user-time/op  delta
      SSA         9.70s ± 1%        8.04s ± 2%  -17.17%  (p=0.000 n=9+10)
      
      name  old obj-bytes     new obj-bytes     delta
      SSA         9.82M ± 0%        8.28M ± 0%  -15.67%  (p=0.000 n=10+10)
      
      Change-Id: Iab472905da3f0e82f3db2c93d06e2759abc9dd44
      Reviewed-on: https://go-review.googlesource.com/41296Reviewed-by: default avatarKeith Randall <khr@golang.org>
      fc7b83d1
    • Josh Bleecher Snyder's avatar
      cmd/compile: stop generating block successor vars in rewrite rules · eaa198f3
      Josh Bleecher Snyder authored
      They are left over from the days before
      we had BlockKindFirst and swapSuccessors.
      
      Change-Id: I9259d53ac2821ca4d5de5dd520ca4b78f52ecad4
      Reviewed-on: https://go-review.googlesource.com/41206
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      eaa198f3
    • xufei_Alex's avatar
      crypto/x509: use native compare in test instead of strings.Compare · db8437eb
      xufei_Alex authored
      Change-Id: I24c824edd8af6311a4eff44ef4bb28d73a91c68e
      Reviewed-on: https://go-review.googlesource.com/41295Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      db8437eb
    • Mostyn Bramley-Moore's avatar
      cmd/go/internal/work: fix TestRespectGroupSticky on FreeBSD · e4852aaa
      Mostyn Bramley-Moore authored
      FreeBSD doesn't allow non-root users to enable the SetGID bit on
      files or directories in /tmp, however it does allow this in
      subdirectories, so create the test directory one level deeper.
      
      Followup to golang/go#19596.
      
      Change-Id: I30e71c6d6a156badc863e8068df10ef6ed817e26
      Reviewed-on: https://go-review.googlesource.com/41216Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      e4852aaa
    • Damien Lespiau's avatar
      net/http: use bullet characters for godoc lists · be66d174
      Damien Lespiau authored
      Brad noticed a bullet list was rendered as preformatted text because of
      the indentation. One can use a unicode bullet as an ersatz for bullet
      lists.
      
      Fixes #20043
      
      Change-Id: Iaed3582d14bd05920455669039a900d7155960d9
      Reviewed-on: https://go-review.googlesource.com/41212Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      be66d174
    • Ian Lance Taylor's avatar
      bufio: clarify that Flush returns a cached write error · 20948079
      Ian Lance Taylor authored
      Change-Id: I377403fc0981d58aec5d84a1dd0d4e08532a575c
      Reviewed-on: https://go-review.googlesource.com/41291Reviewed-by: default avatarDan Peterson <dpiddy@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      20948079
    • Carlisia Campos's avatar
      doc: add Go Time podcast to the help page · 963b6d1d
      Carlisia Campos authored
      Adding the Go Time podcast under the `Stay informed` section of the help
      page on the website.
      
      Change-Id: Ifb1c6bb20cbf640a91572d47f14a432f58439261
      Reviewed-on: https://go-review.googlesource.com/41146Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      963b6d1d
  2. 20 Apr, 2017 13 commits