1. 17 Feb, 2017 3 commits
    • Dmitry Vyukov's avatar
      sync: make Mutex more fair · 0556e262
      Dmitry Vyukov authored
      Add new starvation mode for Mutex.
      In starvation mode ownership is directly handed off from
      unlocking goroutine to the next waiter. New arriving goroutines
      don't compete for ownership.
      Unfair wait time is now limited to 1ms.
      Also fix a long standing bug that goroutines were requeued
      at the tail of the wait queue. That lead to even more unfair
      acquisition times with multiple waiters.
      
      Performance of normal mode is not considerably affected.
      
      Fixes #13086
      
      On the provided in the issue lockskew program:
      
      done in 1.207853ms
      done in 1.177451ms
      done in 1.184168ms
      done in 1.198633ms
      done in 1.185797ms
      done in 1.182502ms
      done in 1.316485ms
      done in 1.211611ms
      done in 1.182418ms
      
      name                    old time/op  new time/op   delta
      MutexUncontended-48     0.65ns ± 0%   0.65ns ± 1%     ~           (p=0.087 n=10+10)
      Mutex-48                 112ns ± 1%    114ns ± 1%   +1.69%        (p=0.000 n=10+10)
      MutexSlack-48            113ns ± 0%     87ns ± 1%  -22.65%         (p=0.000 n=8+10)
      MutexWork-48             149ns ± 0%    145ns ± 0%   -2.48%         (p=0.000 n=9+10)
      MutexWorkSlack-48        149ns ± 0%    122ns ± 3%  -18.26%         (p=0.000 n=6+10)
      MutexNoSpin-48           103ns ± 4%    105ns ± 3%     ~           (p=0.089 n=10+10)
      MutexSpin-48             490ns ± 4%    515ns ± 6%   +5.08%        (p=0.006 n=10+10)
      Cond32-48               13.4µs ± 6%   13.1µs ± 5%   -2.75%        (p=0.023 n=10+10)
      RWMutexWrite100-48      53.2ns ± 3%   41.2ns ± 3%  -22.57%        (p=0.000 n=10+10)
      RWMutexWrite10-48       45.9ns ± 2%   43.9ns ± 2%   -4.38%        (p=0.000 n=10+10)
      RWMutexWorkWrite100-48   122ns ± 2%    134ns ± 1%   +9.92%        (p=0.000 n=10+10)
      RWMutexWorkWrite10-48    206ns ± 1%    188ns ± 1%   -8.52%         (p=0.000 n=8+10)
      Cond32-24               12.1µs ± 3%   12.4µs ± 3%   +1.98%         (p=0.043 n=10+9)
      MutexUncontended-24     0.74ns ± 1%   0.75ns ± 1%     ~           (p=0.650 n=10+10)
      Mutex-24                 122ns ± 2%    124ns ± 1%   +1.31%        (p=0.007 n=10+10)
      MutexSlack-24           96.9ns ± 2%  102.8ns ± 2%   +6.11%        (p=0.000 n=10+10)
      MutexWork-24             146ns ± 1%    135ns ± 2%   -7.70%         (p=0.000 n=10+9)
      MutexWorkSlack-24        135ns ± 1%    128ns ± 2%   -5.01%         (p=0.000 n=10+9)
      MutexNoSpin-24           114ns ± 3%    110ns ± 4%   -3.84%        (p=0.000 n=10+10)
      MutexSpin-24             482ns ± 4%    475ns ± 8%     ~           (p=0.286 n=10+10)
      RWMutexWrite100-24      43.0ns ± 3%   43.1ns ± 2%     ~           (p=0.956 n=10+10)
      RWMutexWrite10-24       43.4ns ± 1%   43.2ns ± 1%     ~            (p=0.085 n=10+9)
      RWMutexWorkWrite100-24   130ns ± 3%    131ns ± 3%     ~           (p=0.747 n=10+10)
      RWMutexWorkWrite10-24    191ns ± 1%    192ns ± 1%     ~           (p=0.210 n=10+10)
      Cond32-12               11.5µs ± 2%   11.7µs ± 2%   +1.98%        (p=0.002 n=10+10)
      MutexUncontended-12     1.48ns ± 0%   1.50ns ± 1%   +1.08%        (p=0.004 n=10+10)
      Mutex-12                 141ns ± 1%    143ns ± 1%   +1.63%        (p=0.000 n=10+10)
      MutexSlack-12            121ns ± 0%    119ns ± 0%   -1.65%          (p=0.001 n=8+9)
      MutexWork-12             141ns ± 2%    150ns ± 3%   +6.36%         (p=0.000 n=9+10)
      MutexWorkSlack-12        131ns ± 0%    138ns ± 0%   +5.73%         (p=0.000 n=9+10)
      MutexNoSpin-12          87.0ns ± 1%   83.7ns ± 1%   -3.80%        (p=0.000 n=10+10)
      MutexSpin-12             364ns ± 1%    377ns ± 1%   +3.77%        (p=0.000 n=10+10)
      RWMutexWrite100-12      42.8ns ± 1%   43.9ns ± 1%   +2.41%         (p=0.000 n=8+10)
      RWMutexWrite10-12       39.8ns ± 4%   39.3ns ± 1%     ~            (p=0.433 n=10+9)
      RWMutexWorkWrite100-12   131ns ± 1%    131ns ± 0%     ~            (p=0.591 n=10+9)
      RWMutexWorkWrite10-12    173ns ± 1%    174ns ± 0%     ~            (p=0.059 n=10+8)
      Cond32-6                10.9µs ± 2%   10.9µs ± 2%     ~           (p=0.739 n=10+10)
      MutexUncontended-6      2.97ns ± 0%   2.97ns ± 0%     ~     (all samples are equal)
      Mutex-6                  122ns ± 6%    122ns ± 2%     ~           (p=0.668 n=10+10)
      MutexSlack-6             149ns ± 3%    142ns ± 3%   -4.63%        (p=0.000 n=10+10)
      MutexWork-6              136ns ± 3%    140ns ± 5%     ~           (p=0.077 n=10+10)
      MutexWorkSlack-6         152ns ± 0%    138ns ± 2%   -9.21%         (p=0.000 n=6+10)
      MutexNoSpin-6            150ns ± 1%    152ns ± 0%   +1.50%         (p=0.000 n=8+10)
      MutexSpin-6              726ns ± 0%    730ns ± 1%     ~           (p=0.069 n=10+10)
      RWMutexWrite100-6       40.6ns ± 1%   40.9ns ± 1%   +0.91%         (p=0.001 n=8+10)
      RWMutexWrite10-6        37.1ns ± 0%   37.0ns ± 1%     ~            (p=0.386 n=9+10)
      RWMutexWorkWrite100-6    133ns ± 1%    134ns ± 1%   +1.01%         (p=0.005 n=9+10)
      RWMutexWorkWrite10-6     152ns ± 0%    152ns ± 0%     ~     (all samples are equal)
      Cond32-2                7.86µs ± 2%   7.95µs ± 2%   +1.10%        (p=0.023 n=10+10)
      MutexUncontended-2      8.10ns ± 0%   9.11ns ± 4%  +12.44%         (p=0.000 n=9+10)
      Mutex-2                 32.9ns ± 9%   38.4ns ± 6%  +16.58%        (p=0.000 n=10+10)
      MutexSlack-2            93.4ns ± 1%   98.5ns ± 2%   +5.39%         (p=0.000 n=10+9)
      MutexWork-2             40.8ns ± 3%   43.8ns ± 7%   +7.38%         (p=0.000 n=10+9)
      MutexWorkSlack-2        98.6ns ± 5%  108.2ns ± 2%   +9.80%         (p=0.000 n=10+8)
      MutexNoSpin-2            399ns ± 1%    398ns ± 2%     ~             (p=0.463 n=8+9)
      MutexSpin-2             1.99µs ± 3%   1.97µs ± 1%   -0.81%          (p=0.003 n=9+8)
      RWMutexWrite100-2       37.6ns ± 5%   46.0ns ± 4%  +22.17%         (p=0.000 n=10+8)
      RWMutexWrite10-2        50.1ns ± 6%   36.8ns ±12%  -26.46%         (p=0.000 n=9+10)
      RWMutexWorkWrite100-2    136ns ± 0%    134ns ± 2%   -1.80%          (p=0.001 n=7+9)
      RWMutexWorkWrite10-2     140ns ± 1%    138ns ± 1%   -1.50%        (p=0.000 n=10+10)
      Cond32                  5.93µs ± 1%   5.91µs ± 0%     ~            (p=0.411 n=9+10)
      MutexUncontended        15.9ns ± 0%   15.8ns ± 0%   -0.63%          (p=0.000 n=8+8)
      Mutex                   15.9ns ± 0%   15.8ns ± 0%   -0.44%        (p=0.003 n=10+10)
      MutexSlack              26.9ns ± 3%   26.7ns ± 2%     ~           (p=0.084 n=10+10)
      MutexWork               47.8ns ± 0%   47.9ns ± 0%   +0.21%          (p=0.014 n=9+8)
      MutexWorkSlack          54.9ns ± 3%   54.5ns ± 3%     ~           (p=0.254 n=10+10)
      MutexNoSpin              786ns ± 2%    765ns ± 1%   -2.66%        (p=0.000 n=10+10)
      MutexSpin               3.87µs ± 1%   3.83µs ± 0%   -0.85%          (p=0.005 n=9+8)
      RWMutexWrite100         21.2ns ± 2%   21.0ns ± 1%   -0.88%         (p=0.018 n=10+9)
      RWMutexWrite10          22.6ns ± 1%   22.6ns ± 0%     ~             (p=0.471 n=9+9)
      RWMutexWorkWrite100      132ns ± 0%    132ns ± 0%     ~     (all samples are equal)
      RWMutexWorkWrite10       124ns ± 0%    123ns ± 0%     ~           (p=0.656 n=10+10)
      
      Change-Id: I66412a3a0980df1233ad7a5a0cd9723b4274528b
      Reviewed-on: https://go-review.googlesource.com/34310
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      0556e262
    • Wander Lairson Costa's avatar
      syscall: only call setgroups if we need to · 79f6a5c7
      Wander Lairson Costa authored
      If the caller set ups a Credential in os/exec.Command,
      os/exec.Command.Start will end up calling setgroups(2), even if no
      supplementary groups were given.
      
      Only root can call setgroups(2) on BSD kernels, which causes Start to
      fail for non-root users when they try to set uid and gid for the new
      process.
      
      We fix by introducing a new field to syscall.Credential named
      NoSetGroups, and setgroups(2) is only called if it is false.
      We make this field with inverted logic to preserve backward
      compatibility.
      
      RELNOTES=yes
      
      Change-Id: I3cff1f21c117a1430834f640ef21fd4e87e06804
      Reviewed-on: https://go-review.googlesource.com/36697Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      79f6a5c7
    • Keith Randall's avatar
      cmd/compile: move constant divide strength reduction to SSA rules · 708ba22a
      Keith Randall authored
      Currently the conversion from constant divides to multiplies is mostly
      done during the walk pass.  This is suboptimal because SSA can
      determine that the value being divided by is constant more often
      (e.g. after inlining).
      
      Change-Id: If1a9b993edd71be37396b9167f77da271966f85f
      Reviewed-on: https://go-review.googlesource.com/37015
      Run-TryBot: Keith Randall <khr@golang.org>
      Reviewed-by: 's avatarJosh Bleecher Snyder <josharian@gmail.com>
      708ba22a
  2. 16 Feb, 2017 12 commits
  3. 15 Feb, 2017 17 commits
    • Matthew Dempsky's avatar
      cmd/compile/internal/gc: skip useless loads for non-SSA params · a6b33312
      Matthew Dempsky authored
      Change-Id: I78ca43a0f0a6a162a2ade1352e2facb29432d4ac
      Reviewed-on: https://go-review.googlesource.com/37102
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      a6b33312
    • Matthew Dempsky's avatar
      cmd/compile/internal/gc: document (*state).checkgoto · 862fde81
      Matthew Dempsky authored
      No behavior change.
      
      Change-Id: I595c15ee976adf21bdbabdf24edf203c9e446185
      Reviewed-on: https://go-review.googlesource.com/36958Reviewed-by: 's avatarJosh Bleecher Snyder <josharian@gmail.com>
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      862fde81
    • Ian Lance Taylor's avatar
      internal/poll: define PollDescriptor on plan9 · 45a5f79c
      Ian Lance Taylor authored
      Fixes #19114.
      
      Change-Id: I352add53d6ee8bf78792564225099f8537ac6b46
      Reviewed-on: https://go-review.googlesource.com/37106
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      Reviewed-by: 's avatarDavid du Colombier <0intro@gmail.com>
      45a5f79c
    • Sarah Adams's avatar
      doc: update Code of Conduct wording and scope · 025dfb13
      Sarah Adams authored
      This change removes the punitive language and anonymous reporting mechanism
      from the Code of Conduct document. Read on for the rationale.
      
      More than a year has passed since the Go Code of Conduct was introduced.
      In that time, there have been a small number (<30) of reports to the Working Group.
      Some reports we handled well, with positive outcomes for all involved.
      A few reports we handled badly, resulting in hurt feelings and a bad
      experience for all involved.
      
      On reflection, the reports that had positive outcomes were ones where the
      Working Group took the role of advisor/facilitator, listening to complaints and
      providing suggestions and advice to the parties involved.
      The reports that had negative outcomes were ones where the subject of the
      report felt threatened by the Working Group and Code of Conduct.
      
      After some discussion among the Working Group, we saw that we are most
      effective as facilitators, rather than disciplinarians. The various Go spaces
      already have moderators; this change to the CoC acknowledges their authority
      and places the group in a purely advisory role. If an incident is
      reported to the group we may provide information to or make a
      suggestion the moderators, but the Working Group need not (and should not) have
      any authority to take disciplinary action.
      
      In short, we want it to be clear that the Working Group are here to help
      resolve conflict, period.
      
      The second change made here is the removal of the anonymous reporting mechanism.
      To date, the quality of anonymous reports has been low, and with no way to
      reach out to the reporter for more information there is often very little we
      can do in response. Removing this one-way reporting mechanism strengthens the
      message that the Working Group are here to facilitate a constructive dialogue.
      
      Change-Id: Iee52aff5446accd0dae0c937bb3aa89709ad5fb4
      Reviewed-on: https://go-review.googlesource.com/37014Reviewed-by: 's avatarAndrew Gerrand <adg@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      025dfb13
    • Ian Lance Taylor's avatar
      os: skip TestPipeThreads on Solaris · ae1d0598
      Ian Lance Taylor authored
      I don't know why it is not working.  Filed issue 19111 for this.
      
      Fixes build.
      
      Update #19111.
      
      Change-Id: I76f8d6aafba5951da2f3ad7d10960419cca7dd1f
      Reviewed-on: https://go-review.googlesource.com/37092Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      ae1d0598
    • Ian Lance Taylor's avatar
      os: skip TestPipeThreads on Plan 9 · 0fe62e75
      Ian Lance Taylor authored
      It can't work since Plan 9 does not support the runtime poller.
      
      Fixes build.
      
      Change-Id: I9ec33eb66019d9364c6ff6519b61b32e59498559
      Reviewed-on: https://go-review.googlesource.com/37091
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      0fe62e75
    • Russ Cox's avatar
      runtime: do not call wakep from enlistWorker, to avoid possible deadlock · 1f77db94
      Russ Cox authored
      We have seen one instance of a production job suddenly spinning to
      100% CPU and becoming unresponsive. In that one instance, a SIGQUIT
      was sent after 328 minutes of spinning, and the stacks showed a single
      goroutine in "IO wait (scan)" state.
      
      Looking for things that might get stuck if a goroutine got stuck in
      scanning a stack, we found that injectglist does:
      
      	lock(&sched.lock)
      	var n int
      	for n = 0; glist != nil; n++ {
      		gp := glist
      		glist = gp.schedlink.ptr()
      		casgstatus(gp, _Gwaiting, _Grunnable)
      		globrunqput(gp)
      	}
      	unlock(&sched.lock)
      
      and that casgstatus spins on gp.atomicstatus until the _Gscan bit goes
      away. Essentially, this code locks sched.lock and then while holding
      sched.lock, waits to lock gp.atomicstatus.
      
      The code that is doing the scan is:
      
      	if castogscanstatus(gp, s, s|_Gscan) {
      		if !gp.gcscandone {
      			scanstack(gp, gcw)
      			gp.gcscandone = true
      		}
      		restartg(gp)
      		break loop
      	}
      
      More analysis showed that scanstack can, in a rare case, end up
      calling back into code that acquires sched.lock. For example:
      
      	runtime.scanstack at proc.go:866
      	calls runtime.gentraceback at mgcmark.go:842
      	calls runtime.scanstack$1 at traceback.go:378
      	calls runtime.scanframeworker at mgcmark.go:819
      	calls runtime.scanblock at mgcmark.go:904
      	calls runtime.greyobject at mgcmark.go:1221
      	calls (*runtime.gcWork).put at mgcmark.go:1412
      	calls (*runtime.gcControllerState).enlistWorker at mgcwork.go:127
      	calls runtime.wakep at mgc.go:632
      	calls runtime.startm at proc.go:1779
      	acquires runtime.sched.lock at proc.go:1675
      
      This path was found with an automated deadlock-detecting tool.
      There are many such paths but they all go through enlistWorker -> wakep.
      
      The evidence strongly suggests that one of these paths is what caused
      the deadlock we observed. We're running those jobs with
      GOTRACEBACK=crash now to try to get more information if it happens
      again.
      
      Further refinement and analysis shows that if we drop the wakep call
      from enlistWorker, the remaining few deadlock cycles found by the tool
      are all false positives caused by not understanding the effect of calls
      to func variables.
      
      The enlistWorker -> wakep call was intended only as a performance
      optimization, it rarely executes, and if it does execute at just the
      wrong time it can (and plausibly did) cause the deadlock we saw.
      
      Comment it out, to avoid the potential deadlock.
      
      Fixes #19112.
      Unfixes #14179.
      
      Change-Id: I6f7e10b890b991c11e79fab7aeefaf70b5d5a07b
      Reviewed-on: https://go-review.googlesource.com/37093
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: 's avatarAustin Clements <austin@google.com>
      1f77db94
    • Hana Kim's avatar
      runtime/pprof: print newly added fields of runtime.MemStats · 8833af3f
      Hana Kim authored
      in heap profile with debug mode
      
      Change-Id: I3a80d03a4aa556614626067a8fd698b3b00f4290
      Reviewed-on: https://go-review.googlesource.com/36962Reviewed-by: 's avatarAustin Clements <austin@google.com>
      8833af3f
    • Heschi Kreinick's avatar
      cmd/compile/internal/ssa: display NamedValues in SSA html output. · 35a95df5
      Heschi Kreinick authored
      Change-Id: If268b42b32e6bcd6e7913bffa6e493dc78af40aa
      Reviewed-on: https://go-review.googlesource.com/36539
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Run-TryBot: Heschi Kreinick <heschi@google.com>
      Reviewed-by: 's avatarMatthew Dempsky <mdempsky@google.com>
      35a95df5
    • Lynn Boger's avatar
      cmd/go: improve stale reason for packages · 2ac32b63
      Lynn Boger authored
      This adds more information to the pkg stale reason for debugging
      purposes.
      
      Change-Id: I7b626db4520baa1127195ae859f4da9b49304636
      Reviewed-on: https://go-review.googlesource.com/36944Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      2ac32b63
    • Ian Lance Taylor's avatar
      os: use poller for file I/O · c05b06a1
      Ian Lance Taylor authored
      This changes the os package to use the runtime poller for file I/O
      where possible. When a system call blocks on a pollable descriptor,
      the goroutine will be blocked on the poller but the thread will be
      released to run other goroutines. When using a non-pollable
      descriptor, the os package will continue to use thread-blocking system
      calls as before.
      
      For example, on GNU/Linux, the runtime poller uses epoll. epoll does
      not support ordinary disk files, so they will continue to use blocking
      I/O as before. The poller will be used for pipes.
      
      Since this means that the poller is used for many more programs, this
      modifies the runtime to only block waiting for the poller if there is
      some goroutine that is waiting on the poller. Otherwise, there is no
      point, as the poller will never make any goroutine ready. This
      preserves the runtime's current simple deadlock detection.
      
      This seems to crash FreeBSD systems, so it is disabled on FreeBSD.
      This is issue 19093.
      
      Using the poller on Windows requires opening the file with
      FILE_FLAG_OVERLAPPED. We should only do that if we can remove that
      flag if the program calls the Fd method. This is issue 19098.
      
      Update #6817.
      Update #7903.
      Update #15021.
      Update #18507.
      Update #19093.
      Update #19098.
      
      Change-Id: Ia5197dcefa7c6fbcca97d19a6f8621b2abcbb1fe
      Reviewed-on: https://go-review.googlesource.com/36800
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      c05b06a1
    • Dave Cheney's avatar
      internal/poll: remove unused poll.pollDesc methods · 81ec3f6a
      Dave Cheney authored
      Change-Id: Ic2b20c8238ff0ca5513d32e54ef2945fa4d0c3d2
      Reviewed-on: https://go-review.googlesource.com/37033
      Run-TryBot: Dave Cheney <dave@cheney.net>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      81ec3f6a
    • Marcel van Lohuizen's avatar
      testing: fix stats bug for sub benchmarks · 79fab70a
      Marcel van Lohuizen authored
      Fixes golang/go#18815.
      
      Change-Id: Ic9d5cb640a555c58baedd597ed4ca5dd9f275c97
      Reviewed-on: https://go-review.googlesource.com/36990
      Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      79fab70a
    • Robert Griesemer's avatar
      cmd/compile/internal/syntax: compiler directives must start at beginning of line · d390283f
      Robert Griesemer authored
      - ignore them, if they don't.
      - added tests
      
      Fixes #18393.
      
      Change-Id: I13f87b81ac6b9138ab5031bb3dd6bebc4c548156
      Reviewed-on: https://go-review.googlesource.com/37020
      Run-TryBot: Robert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarMatthew Dempsky <mdempsky@google.com>
      d390283f
    • Alex Brainman's avatar
      internal/testenv: do not delete target file · a8dc43ed
      Alex Brainman authored
      We did not create it. We should not delete it.
      
      Change-Id: If98454ab233ce25367e11a7c68d31b49074537dd
      Reviewed-on: https://go-review.googlesource.com/37030Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      a8dc43ed
    • Robert Griesemer's avatar
      cmd/compile: fix position for "missing type in composite literal" error · 2770c507
      Robert Griesemer authored
      Fixes #18231.
      
      Change-Id: If1615da4db0e6f0516369a1dc37340d80c78f237
      Reviewed-on: https://go-review.googlesource.com/37018Reviewed-by: 's avatarMatthew Dempsky <mdempsky@google.com>
      2770c507
    • Robert Griesemer's avatar
      cmd/compile/internal/syntax: establish principled position information · 5267ac27
      Robert Griesemer authored
      Until now, the parser set the position for each Node to the position of
      the first token belonging to that node. For compatibility with the now
      defunct gc parser, in many places that position information was modified
      when the gcCompat flag was set (which it was, by default). Furthermore,
      in some places, position information was not set at all.
      
      This change removes the gcCompat flag and all associated code, and sets
      position information for all nodes in a more principled way, as proposed
      by mdempsky (see #16943 for details). Specifically, the position of a
      node may not be at the very beginning of the respective production. For
      instance for an Operation `a + b`, the position associated with the node
      is the position of the `+`. Thus, for `a + b + c` we now get different
      positions for the two additions.
      
      This change does not pass toolstash -cmp because position information
      recorded in export data and pcline tables is different. There are no
      other functional changes.
      
      Added test suite testing the position of all nodes.
      
      Fixes #16943.
      
      Change-Id: I3fc02bf096bc3b3d7d2fa655dfd4714a1a0eb90c
      Reviewed-on: https://go-review.googlesource.com/37017
      Run-TryBot: Robert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarMatthew Dempsky <mdempsky@google.com>
      5267ac27
  4. 14 Feb, 2017 8 commits
    • Daniel Martí's avatar
      math/big: simplify bool expression · 6910756f
      Daniel Martí authored
      Change-Id: I280c53be455f2fe0474ad577c0f7b7908a4eccb2
      Reviewed-on: https://go-review.googlesource.com/36993Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      6910756f
    • Russ Cox's avatar
      encoding/xml: fix incorrect indirect code in chardata, comment, innerxml fields · 72aa757d
      Russ Cox authored
      The new tests in this CL have been checked against Go 1.7 as well
      and all pass in Go 1.7, with the one exception noted in a comment
      (an intentional change to omitempty already present before this CL).
      
      CL 15684 made the intentional change to omitempty.
      This CL fixes bugs introduced along the way.
      
      Most of these are corner cases that are arguably not that important,
      but they've always worked all the way back to Go 1, and someone
      cared enough to file #19063. The most significant problem found
      while adding tests is that in the case of a nil *string field with
      `xml:",chardata"`, the existing code silently stops processing not just
      that field but the entire remainder of the struct.
      Even if #19063 were not worth fixing, this chardata bug would be.
      
      Fixes #19063.
      
      Change-Id: I318cf8f9945e1a4615982d9904e109fde577ebf9
      Reviewed-on: https://go-review.googlesource.com/36954
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarDaniel Martí <mvdan@mvdan.cc>
      Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      72aa757d
    • Bryan C. Mills's avatar
      mime: add benchmarks for TypeByExtension and ExtensionsByType · eebd8f51
      Bryan C. Mills authored
      These are possible use-cases for sync.Map.
      
      Updates golang/go#18177
      
      Change-Id: I5e2a3d1249967c37d3f89a41122bf4a90522db11
      Reviewed-on: https://go-review.googlesource.com/36964Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      eebd8f51
    • Kirill Smelkov's avatar
      cmd/compile/internal/ssa: combine 2 byte loads + shifts into word load + rolw 8 on AMD64 · 4477fd09
      Kirill Smelkov authored
      ... and same for stores. This does for binary.BigEndian.Uint16() what
      was already done for Uint32 and Uint64 with BSWAP in 10f75748 (CL 32222).
      
      Here is how generated code changes e.g. for the following function
      (omitting saying the same prologue/epilogue):
      
      	func get16(b [2]byte) uint16 {
      		return binary.BigEndian.Uint16(b[:])
      	}
      
      "".get16 t=1 size=21 args=0x10 locals=0x0
      
      	// before
              0x0000 00000 (x.go:15)  MOVBLZX "".b+9(FP), AX
              0x0005 00005 (x.go:15)  MOVBLZX "".b+8(FP), CX
              0x000a 00010 (x.go:15)  SHLL    $8, CX
              0x000d 00013 (x.go:15)  ORL     CX, AX
      
      	// after
      	0x0000 00000 (x.go:15)	MOVWLZX	"".b+8(FP), AX
      	0x0005 00005 (x.go:15)	ROLW	$8, AX
      
      encoding/binary is speedup overall a bit:
      
      name                    old time/op    new time/op    delta
      ReadSlice1000Int32s-4     4.83µs ± 0%    4.83µs ± 0%     ~     (p=0.206 n=4+5)
      ReadStruct-4              1.29µs ± 2%    1.28µs ± 1%   -1.27%  (p=0.032 n=4+5)
      ReadInts-4                 384ns ± 1%     385ns ± 1%     ~     (p=0.968 n=4+5)
      WriteInts-4                534ns ± 3%     526ns ± 0%   -1.54%  (p=0.048 n=4+5)
      WriteSlice1000Int32s-4    5.02µs ± 0%    5.11µs ± 3%     ~     (p=0.175 n=4+5)
      PutUint16-4               0.59ns ± 0%    0.49ns ± 2%  -16.95%  (p=0.016 n=4+5)
      PutUint32-4               0.52ns ± 0%    0.52ns ± 0%     ~     (all equal)
      PutUint64-4               0.53ns ± 0%    0.53ns ± 0%     ~     (all equal)
      PutUvarint32-4            19.9ns ± 0%    19.9ns ± 1%     ~     (p=0.556 n=4+5)
      PutUvarint64-4            54.5ns ± 1%    54.2ns ± 0%     ~     (p=0.333 n=4+5)
      
      name                    old speed      new speed      delta
      ReadSlice1000Int32s-4    829MB/s ± 0%   828MB/s ± 0%     ~     (p=0.190 n=4+5)
      ReadStruct-4            58.0MB/s ± 2%  58.7MB/s ± 1%   +1.30%  (p=0.032 n=4+5)
      ReadInts-4              78.0MB/s ± 1%  77.8MB/s ± 1%     ~     (p=0.968 n=4+5)
      WriteInts-4             56.1MB/s ± 3%  57.0MB/s ± 0%     ~     (p=0.063 n=4+5)
      WriteSlice1000Int32s-4   797MB/s ± 0%   783MB/s ± 3%     ~     (p=0.190 n=4+5)
      PutUint16-4             3.37GB/s ± 0%  4.07GB/s ± 2%  +20.83%  (p=0.016 n=4+5)
      PutUint32-4             7.73GB/s ± 0%  7.72GB/s ± 0%     ~     (p=0.556 n=4+5)
      PutUint64-4             15.1GB/s ± 0%  15.1GB/s ± 0%     ~     (p=0.905 n=4+5)
      PutUvarint32-4           201MB/s ± 0%   201MB/s ± 0%     ~     (p=0.905 n=4+5)
      PutUvarint64-4           147MB/s ± 1%   147MB/s ± 0%     ~     (p=0.286 n=4+5)
      
      ( "a bit" only because most of the time is spent in reflection-like things
        there, not actual bytes decoding. Even for direct PutUint16 benchmark the
        looping adds overhead and lowers visible benefit. For code-generated encoders /
        decoders actual effect is more than 20% )
      
      Adding Uint32 and Uint64 raw benchmarks too for completeness.
      
      NOTE I had to adjust load-combining rule for bswap case to match first 2 bytes
      loads as result of "2-bytes load+shift" -> "loadw + rorw 8" rewrite. Reason is:
      for loads+shift, even e.g. into uint16 var
      
      	var b []byte
      	var v uin16
      	v = uint16(b[1]) | uint16(b[0])<<8
      
      the compiler eventually generates L(ong) shift - SHLLconst [8], probably
      because it is more straightforward / other reasons to work on the whole
      register. This way 2 bytes rewriting rule is using SHLLconst (not SHLWconst) in
      its pattern, and then it always gets matched first, even if 2-byte rule comes
      syntactically after 4-byte rule in AMD64.rules because 4-bytes rule seemingly
      needs more applyRewrite() cycles to trigger. If 2-bytes rule gets matched for
      inner half of
      
      	var b []byte
      	var v uin32
      	v = uint32(b[3]) | uint32(b[2])<<8 | uint32(b[1])<<16 | uint32(b[0])<<24
      
      and we keep 4-byte load rule unchanged, the result will be MOVW + RORW $8 and
      then series of byte loads and shifts - not one MOVL + BSWAPL.
      
      There is no such problem for stores: there compiler, since it probably knows
      store destination is 2 bytes wide, uses SHRWconst 8 (not SHRLconst 8) and thus
      2-byte store rule is not a subset of rule for 4-byte stores.
      
      Fixes #17151  (int16 was last missing piece there)
      
      Change-Id: Idc03ba965bfce2b94fef456b02ff6742194748f6
      Reviewed-on: https://go-review.googlesource.com/34636Reviewed-by: 's avatarIlya Tocar <ilya.tocar@intel.com>
      Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      4477fd09
    • Bryan C. Mills's avatar
      expvar: add benchmarks for steady-state Map Add calls · 7ffdb757
      Bryan C. Mills authored
      Add a benchmark for setting a String value, which we may
      want to treat differently from Int or Float due to the need to support
      Add methods for the latter.
      
      Update tests to use only the exported API instead of making (fragile)
      assumptions about unexported fields.
      
      The existing Map benchmarks construct a new Map for each iteration, which
      focuses the benchmark results on the initial allocation costs for the
      Map and its entries. This change adds variants of the benchmarks which
      use a long-lived map in order to measure steady-state performance for
      Map updates on existing keys.
      
      Updates #18177
      
      Change-Id: I62c920991d17d5898c592446af382cd5c04c528a
      Reviewed-on: https://go-review.googlesource.com/36959Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      7ffdb757
    • Michael Munday's avatar
      math/big: fix s390x test build tags · d2fea044
      Michael Munday authored
      The tests failed to compile when using the math_big_pure_go tag on
      s390x.
      
      Change-Id: I2a09f53ff6562ab9bc9b886cffc0f6205bbfcfbb
      Reviewed-on: https://go-review.googlesource.com/36956
      Run-TryBot: Michael Munday <munday@ca.ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      d2fea044
    • Cherry Zhang's avatar
      cmd/compile: undo special handling of zero-valued STRUCTLIT · 78200799
      Cherry Zhang authored
      CL 35261 introduces special handling of zero-valued STRUCTLIT for
      efficient struct zeroing. But it didn't cover all use cases, for
      example, CONVNOP STRUCTLIT is not handled.
      
      On the other hand, CL 34566 handles zeroing earlier, so we don't
      need the change in CL 35261 for efficient zeroing. Other uses of
      zero-valued struct literals are very rare. So undo the change in
      walk.go in CL 35261.
      
      Add a test for efficient zeroing.
      
      Fixes #19084.
      
      Change-Id: I0807f7423fb44d47bf325b3c1ce9611a14953853
      Reviewed-on: https://go-review.googlesource.com/36955Reviewed-by: 's avatarMatthew Dempsky <mdempsky@google.com>
      Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      78200799
    • Kirill Smelkov's avatar
      cmd/compile/internal/ssa: generate bswap/store for indexed bigendian byte stores too on AMD64 · bd91e356
      Kirill Smelkov authored
      Commit 10f75748 (CL 32222) added rewrite rules to combine byte loads/stores +
      shifts into larger loads/stores + bswap. For loads both MOVBload and
      MOVBloadidx1 were handled but for store only MOVBstore was there without
      MOVBstoreidx added to rewrite pattern. Fix it.
      
      Here is how generated code changes for the following 2 functions
      (ommitting staying the same prologue/epilogue):
      
          func put32(b []byte, i int, v uint32) {
                  binary.BigEndian.PutUint32(b[i:], v)
          }
      
          func put64(b []byte, i int, v uint64) {
                  binary.BigEndian.PutUint64(b[i:], v)
          }
      
      "".put32 t=1 size=100 args=0x28 locals=0x0
      
      	// before
      	0x0032 00050 (x.go:5)	MOVL	CX, DX
      	0x0034 00052 (x.go:5)	SHRL	$24, CX
      	0x0037 00055 (x.go:5)	MOVQ	"".b+8(FP), BX
      	0x003c 00060 (x.go:5)	MOVB	CL, (BX)(AX*1)
      	0x003f 00063 (x.go:5)	MOVL	DX, CX
      	0x0041 00065 (x.go:5)	SHRL	$16, DX
      	0x0044 00068 (x.go:5)	MOVB	DL, 1(BX)(AX*1)
      	0x0048 00072 (x.go:5)	MOVL	CX, DX
      	0x004a 00074 (x.go:5)	SHRL	$8, CX
      	0x004d 00077 (x.go:5)	MOVB	CL, 2(BX)(AX*1)
      	0x0051 00081 (x.go:5)	MOVB	DL, 3(BX)(AX*1)
      
      	// after
      	0x0032 00050 (x.go:5)	BSWAPL	CX
      	0x0034 00052 (x.go:5)	MOVQ	"".b+8(FP), DX
      	0x0039 00057 (x.go:5)	MOVL	CX, (DX)(AX*1)
      
      "".put64 t=1 size=155 args=0x28 locals=0x0
      
      	// before
      	0x0037 00055 (x.go:9)	MOVQ	CX, DX
      	0x003a 00058 (x.go:9)	SHRQ	$56, CX
      	0x003e 00062 (x.go:9)	MOVQ	"".b+8(FP), BX
      	0x0043 00067 (x.go:9)	MOVB	CL, (BX)(AX*1)
      	0x0046 00070 (x.go:9)	MOVQ	DX, CX
      	0x0049 00073 (x.go:9)	SHRQ	$48, DX
      	0x004d 00077 (x.go:9)	MOVB	DL, 1(BX)(AX*1)
      	0x0051 00081 (x.go:9)	MOVQ	CX, DX
      	0x0054 00084 (x.go:9)	SHRQ	$40, CX
      	0x0058 00088 (x.go:9)	MOVB	CL, 2(BX)(AX*1)
      	0x005c 00092 (x.go:9)	MOVQ	DX, CX
      	0x005f 00095 (x.go:9)	SHRQ	$32, DX
      	0x0063 00099 (x.go:9)	MOVB	DL, 3(BX)(AX*1)
      	0x0067 00103 (x.go:9)	MOVQ	CX, DX
      	0x006a 00106 (x.go:9)	SHRQ	$24, CX
      	0x006e 00110 (x.go:9)	MOVB	CL, 4(BX)(AX*1)
      	0x0072 00114 (x.go:9)	MOVQ	DX, CX
      	0x0075 00117 (x.go:9)	SHRQ	$16, DX
      	0x0079 00121 (x.go:9)	MOVB	DL, 5(BX)(AX*1)
      	0x007d 00125 (x.go:9)	MOVQ	CX, DX
      	0x0080 00128 (x.go:9)	SHRQ	$8, CX
      	0x0084 00132 (x.go:9)	MOVB	CL, 6(BX)(AX*1)
      	0x0088 00136 (x.go:9)	MOVB	DL, 7(BX)(AX*1)
      
      	// after
      	0x0033 00051 (x.go:9)	BSWAPQ	CX
      	0x0036 00054 (x.go:9)	MOVQ	"".b+8(FP), DX
      	0x003b 00059 (x.go:9)	MOVQ	CX, (DX)(AX*1)
      
      Updates #17151
      
      Change-Id: I3f4a7f28f210e62e153e60da5abd1d39508cc6c4
      Reviewed-on: https://go-review.googlesource.com/34635
      Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarIlya Tocar <ilya.tocar@intel.com>
      bd91e356