1. 09 May, 2019 19 commits
    • Russ Cox's avatar
      runtime: fix vet complaints for linux/amd64 · fe67ce2e
      Russ Cox authored
      Working toward making the tree vet-safe instead of having
      so many exceptions in cmd/vet/all/whitelist.
      
      This CL makes "GOOS=linux GOARCH=amd64 go vet -unsafeptr=false runtime" happy,
      while keeping "GO_BUILDER_NAME=misc-vetall go tool dist test" happy too.
      
      For #31916.
      
      Change-Id: I4ca1acb02f4666b102d25fcc55fac96b8f80379a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176100
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      fe67ce2e
    • Russ Cox's avatar
      runtime: fix vet complaints for linux/386 · 6ed2ec4a
      Russ Cox authored
      Working toward making the tree vet-safe instead of having
      so many exceptions in cmd/vet/all/whitelist.
      
      This CL makes "GOOS=linux GOARCH=386 go vet -unsafeptr=false runtime" happy,
      while keeping "GO_BUILDER_NAME=misc-vetall go tool dist test" happy too.
      
      For #31916.
      
      Change-Id: I3e5586a7ff6e359357350d0602c2259493280ded
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176099
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      6ed2ec4a
    • Michael Anthony Knyszek's avatar
      runtime: fix js/wasm lock implementation · cd03664f
      Michael Anthony Knyszek authored
      Trybots started failing on js/wasm after golang.org/cl/175797 landed,
      but it seemed completely unrelated. It would fail very consistently on
      the heapsampling.go test.
      
      Digging deeper it was very difficult to ascertain what was going wrong,
      but clearly m.locks for some m was non-zero when calling into the
      scheduler.
      
      The failure comes from the fact that lock calls into gosched, but it's
      unclear how exactly we got there in the first place; there should be no
      preemption in this single-threaded context.
      
      Furthermore, lock shouldn't be calling gosched_m at all because in a
      single-threaded context, the thread shouldn't be preempted until it
      actually unlocks.
      
      But, digging further it turns out the implementation in lock_js.go never
      incremented or decremented m.locks. This is definitely wrong because
      many parts of the runtime depend on that value being set correctly.
      
      So, this change removes the loop which calls into gosched_m (which
      should be unnecessary) and increments and decrements m.locks
      appropriately. This appears to fix the aforementioned failure.
      
      Change-Id: Id214c0762c3fb2b405ff55543d7e2a78c17443c4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176297
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      cd03664f
    • Russ Cox's avatar
      cmd/link: use standard-syntax struct tags in large-tag test · 0a2f72b8
      Russ Cox authored
      These ridiculous tags are testing what happens with very long tags.
      There is no need for them to be malformed as well.
      Fix them to make vet happier.
      
      Working toward making the tree vet-safe instead of having
      so many exceptions in cmd/vet/all/whitelist.
      
      For #31916.
      
      Change-Id: I100aed5230fcde41676c79c7074c69c16ea4b96d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176111
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      0a2f72b8
    • Russ Cox's avatar
      encoding/gob: rename encBuffer.WriteByte to writeByte · c5140719
      Russ Cox authored
      Renaming the method makes clear, both to readers and to vet,
      that this method is not the implementation of io.ByteWriter.
      
      Working toward making the tree vet-safe instead of having
      so many exceptions in cmd/vet/all/whitelist.
      
      For #31916.
      
      Change-Id: I5b509eb7f0118d5f2d3c6e352ff2849cd5a3071e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176110
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      c5140719
    • Russ Cox's avatar
      fmt: rename buffer.WriteByte to writeByte · 50a1d89a
      Russ Cox authored
      Renaming the method makes clear, both to readers and to vet,
      that this method is not the implementation of io.ByteWriter.
      
      Working toward making the tree vet-safe instead of having
      so many exceptions in cmd/vet/all/whitelist.
      
      For #31916.
      
      Change-Id: I79da062ca6469b62a6b9e284c6cf2413c7425249
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176109
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      50a1d89a
    • Jay Conrod's avatar
      all: document vendoring in the standard library · a44c3edb
      Jay Conrod authored
      Added documentation that explains special cases for vendored packages
      in the standard library and provides instructions for updating vendor
      directories.
      
      Fixes #31806
      
      Change-Id: Ib697ed18eae28023ab0bfb9f4d250992c393571d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/174999Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      a44c3edb
    • Russ Cox's avatar
      cmd/internal/bio: rename Reader.Seek to MustSeek · ffd7eba2
      Russ Cox authored
      Renaming the method makes clear, both to readers and to vet,
      that this method is not the implementation of io.Seeker:
      it cannot fail.
      
      Working toward making the tree vet-safe instead of having
      so many exceptions in cmd/vet/all/whitelist.
      
      For #31916.
      
      Change-Id: I3e6ad7264cb0121b4b76935450cccb71d533e96b
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176108
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      ffd7eba2
    • Michael Anthony Knyszek's avatar
      runtime: split spans when scavenging if it's more than we need · ff70494b
      Michael Anthony Knyszek authored
      This change makes it so that during scavenging we split spans when the
      span we have next for scavenging is larger than the amount of work we
      have left to do.
      
      The purpose of this change is to improve the worst-case behavior of the
      scavenger: currently, if the scavenger only has a little bit of work to
      do but sees a very large free span, it will scavenge the whole thing,
      spending a lot of time to get way ahead of the scavenge pacing for no
      reason.
      
      With this change the scavenger should follow the pacing more closely,
      but may still over-scavenge by up to a physical huge page since the
      splitting behavior avoids breaking up huge pages in free spans.
      
      This change is also the culmination of the scavenging improvements, so
      we also include benchmark results for this series (starting from
      "runtime: merge all treaps into one implementation" until this patch).
      
      This patch stack results in average and peak RSS reductions (up to 11%
      and 7% respectively) for some benchmarks, with mostly minimal
      performance degredation (3-4% for some benchmarks, ~0% geomean). Each of
      these benchmarks was executed with GODEBUG=madvdontneed=1 on Linux; the
      performance degredation is even smaller when MADV_FREE may be used, but
      the impact on RSS is much harder to measure. Applications that generally
      maintain a steady heap size for the most part show no change in
      application performance.
      
      These benchmarks are taken from an experimental benchmarking suite
      representing a variety of open-source Go packages, the raw results may
      be found here:
      
      https://perf.golang.org/search?q=upload:20190509.1
      
      For #30333.
      
      Change-Id: I618a48534d2d6ce5f656bb66825e3c383ab1ffba
      Reviewed-on: https://go-review.googlesource.com/c/go/+/175797
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      ff70494b
    • Michael Anthony Knyszek's avatar
      runtime: add background scavenger · fe67ea32
      Michael Anthony Knyszek authored
      This change adds a background scavenging goroutine whose pacing is
      determined when the heap goal changes. The scavenger is paced to use
      at most 1% of the mutator's time for most systems. Furthermore, the
      scavenger's pacing is computed based on the estimated number of
      scavengable huge pages to take advantage of optimizations provided by
      the OS.
      
      The purpose of this scavenger is to deal with a shrinking heap: if the
      heap goal is falling over time, the scavenger should kick in and start
      returning free pages from the heap to the OS.
      
      Also, now that we have a pacing system, the credit system used by
      scavengeLocked has become redundant. Replace it with a mechanism which
      only scavenges on the allocation path if it makes sense to do so with
      respect to the new pacing system.
      
      Fixes #30333.
      
      Change-Id: I6203f8dc84affb26c3ab04528889dd9663530edc
      Reviewed-on: https://go-review.googlesource.com/c/go/+/142960
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      fe67ea32
    • Michael Anthony Knyszek's avatar
      runtime: remove periodic scavenging · eaa1c87b
      Michael Anthony Knyszek authored
      This change removes the periodic scavenger which goes over every span
      in the heap and scavenges it if it hasn't been used for 5 minutes. It
      should no longer be necessary if we have background scavenging
      (follow-up).
      
      For #30333.
      
      Change-Id: Ic3a1a4e85409dc25719ba4593a3b60273a4c71e0
      Reviewed-on: https://go-review.googlesource.com/c/go/+/143157
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      eaa1c87b
    • Russ Cox's avatar
      cmd/vendor: import vet fixes from x/tools · d56199d6
      Russ Cox authored
      Fixes build - I did not understand that vetall was
      effectively pinned to a vet version by cmd/go.mod.
      
      Change-Id: I56bfd8f62eadacc97cad0ed48e41a178bbc18b8f
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176179
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      d56199d6
    • Than McIntosh's avatar
      cmd/compile: emit DWARF call_line attrs with data4 form on iOS · b56d1bad
      Than McIntosh authored
      When targeting iOS, change the format (DWARF "form") of the call line
      attribute for inlined subroutine DIEs, to work around an apparent bug
      in /usr/bin/symbols on Darwin.
      
      [Just for posterity: there is nothing wrong with using DW_FORM_udata
      for the call_line attribute form; this is perfectly legal DWARF (and
      is in fact recommended by the standard relative to data4, which is
      less descriptive and often takes more space). The form switch is there
      just to make the Apple tools happy.]
      
      Updates #31459.
      
      Change-Id: Iaf362788a8c6684eea4cde8956c0661b694cecc1
      Reviewed-on: https://go-review.googlesource.com/c/go/+/174538
      Run-TryBot: Than McIntosh <thanm@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      b56d1bad
    • Russ Cox's avatar
      cmd/vet/all: update whitelist for vet fixes · b6f59cbc
      Russ Cox authored
      The vetall builder runs vet straight out of golang.org/x/tools,
      so submiting CL 176097 in that repo will break the builder
      by making all these whitelist entries stale.
      Submiting this CL will fix it, by removing them.
      
      The addition of the gcWriteBarrier declaration in runtime/stubs.go
      is necessary because the diagnostic is no longer emitted on arm,
      so it must be removed from all.txt. Adding it to runtime is better
      than adding it to every-other-goarch.txt.
      
      For #31916.
      
      Change-Id: I432f6049cd3ee5a467add5066c440be8616d9d54
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176177
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      b6f59cbc
    • David Chase's avatar
      cmd/compile: test delve instead of gdb in ssa/debug_test.go · 5286b2ad
      David Chase authored
      This seems to deflake the test, and also allows testing
      on macOS.
      
      Fixes #31786.
      
      Change-Id: I10bfba46dd4b8e64cb09fdd4dd9d175c1ce1f022
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176058
      Run-TryBot: David Chase <drchase@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      5286b2ad
    • Than McIntosh's avatar
      cmd/link: fix link time regression in object file reading · 0793c810
      Than McIntosh authored
      In CL 173938, the linker's object file reader was switched over to
      selectively create strings backed with read-only mmap'd memory.
      In the process a call to r.rd.Offset() was added to readSymName(),
      which greatly increased the number of system calls (Offset does a
      seek system call).
      
      This patch changes the object file reader so that all reads are done
      directly from the mmap'd data if it is present, and adds logic to keep
      track of the offset within the rodata consumed so far. Doing this gets
      rid of the calls to r.rd.Offset() and the corresponding seek system
      calls.
      
      Also as part of this change, hoist the calls to objabi.PathToPrefix
      up into the initial setup code for object reading, and store the
      result in the reader (since objabi.PathToPrefix was also coming up
      as hot in the profile).
      
      Numbers for this change from compilebench:
      
      benchmark                 old ns/op       new ns/op       delta
      BenchmarkTemplate         172053975       170357597       -0.99%
      BenchmarkUnicode          64564850        64333653        -0.36%
      BenchmarkGoTypes          627931042       628043673       +0.02%
      BenchmarkCompiler         2982468893      2924575043      -1.94%
      BenchmarkSSA              9701681721      9799342557      +1.01%
      BenchmarkFlate            106847240       107509414       +0.62%
      BenchmarkGoParser         132082319       130734905       -1.02%
      BenchmarkReflect          386810586       383036621       -0.98%
      BenchmarkTar              154360072       152670594       -1.09%
      BenchmarkXML              217725693       216858727       -0.40%
      BenchmarkLinkCompiler     908813802       734363234       -19.20%
      BenchmarkStdCmd           32378532486     31222542974     -3.57%
      
      Fixes #31898.
      
      Change-Id: Ibf253a52ce9213325f42b1c2b20d0410f5c88c3b
      Reviewed-on: https://go-review.googlesource.com/c/go/+/176039Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      0793c810
    • Elias Naur's avatar
      misc/android: silence adb output unless an error occurs · fb63ed2b
      Elias Naur authored
      Fixes #31917
      
      Change-Id: I794e457b2245d355e2df5077078c67aa09e00ff9
      Reviewed-on: https://go-review.googlesource.com/c/go/+/175920
      Run-TryBot: Elias Naur <mail@eliasnaur.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      fb63ed2b
    • Russ Cox's avatar
      cmd/asm: accept TEXT f+0(SB) in -gensymabis mode · f766b680
      Russ Cox authored
      f+0(SB) is a non-standard but acceptable alias for f(SB).
      
      Fixes #30968.
      
      Change-Id: I499ccee4d3ff3ab4e47f75d99407aace858e59aa
      Reviewed-on: https://go-review.googlesource.com/c/go/+/174537Reviewed-by: default avatarAustin Clements <austin@google.com>
      f766b680
    • Russ Cox's avatar
      cmd/go: diagnose go.mod and vendor out of sync in std and cmd · 856b57e0
      Russ Cox authored
      The most common failure mode of the current std/cmd setup is
      going to be people running "go get m@latest" and then not running
      "go mod vendor" and being confused about getting the old m.
      Diagnose and report what to do.
      
      Also, having done the check, when in the standard library,
      switch the go command to -mod=vendor mode.
      This avoids some network accesses I saw when running
      'go clean -modcache' before doing some work in cmd.
      
      Change-Id: I0ba4a66637b67225a9b97a1c89f26f9015b41673
      Reviewed-on: https://go-review.googlesource.com/c/go/+/174528
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      856b57e0
  2. 08 May, 2019 16 commits
  3. 07 May, 2019 5 commits