Skip to content

Commit

Permalink
reduce mem. usage of unused and update staticcheck
Browse files Browse the repository at this point in the history
The primary improvement is in early clearing of
analyzed package's TypeInfo, facts, etc for
whole program analyzers (`unused`). Clear it when it
becomes unused and GC collects them early. Initially this
clearing was performed for all analyzers except `unused`.

Update staticcheck from v0.0.1-2019.2.3 to v0.0.1-2020.1.4

Also in this commit:
  * speed up loading packages from export data (2.5s -> 2.1s for std)
    by not using mutex for export data since it was allowed in
    x/tools#07722704da13
  * make an order of execution of linters stable
  * update renameio and robustio
  * use robustio in caching

Relates: #987, #994, #995, #1011
  • Loading branch information
jirfag committed May 3, 2020
1 parent 02a4077 commit 9eb569a
Show file tree
Hide file tree
Showing 16 changed files with 183 additions and 147 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,4 @@ require (
// See https://github.com/golangci/golangci-lint/issues/995
// Update only after mitigating this issue.
// TODO: Enable back tests with skip "Issue955" after update.
require honnef.co/go/tools v0.0.1-2019.2.3
require honnef.co/go/tools v0.0.1-2020.1.3
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -372,11 +372,13 @@ golang.org/x/tools v0.0.0-20190910044552-dd2b5c81c578/go.mod h1:b+2E5dAYhXwXZwtn
golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20191125144606-a911d9008d1f/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20200228224639-71482053b885/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
golang.org/x/tools v0.0.0-20200324003944-a576cf524670/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8=
golang.org/x/tools v0.0.0-20200414032229-332987a829c3/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200422022333-3d57cf2e726e h1:3Dzrrxi54Io7Aoyb0PYLsI47K2TxkRQg+cqUn+m04do=
golang.org/x/tools v0.0.0-20200422022333-3d57cf2e726e/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770 h1:M9Fif0OxNji8w+HvmhVQ8KJtiZOsjU9RgslJGhn95XE=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
Expand Down Expand Up @@ -413,6 +415,8 @@ gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=
honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
honnef.co/go/tools v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=
honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed h1:WX1yoOaKQfddO/mLzdV4wptyWgoH/6hwLs7QHTixo0I=
mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed/go.mod h1:Xkxe497xwlCKkIaQYRfC7CSLworTXY9RMqwhhCm+8Nc=
mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b h1:DxJ5nJdkhDlLok9K6qO+5290kphDJbHOQO1DFFFTeBo=
Expand Down
4 changes: 2 additions & 2 deletions internal/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"encoding/hex"
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
"strconv"
Expand All @@ -24,6 +23,7 @@ import (
"github.com/pkg/errors"

"github.com/golangci/golangci-lint/internal/renameio"
"github.com/golangci/golangci-lint/internal/robustio"
)

// An ActionID is a cache action key, the hash of a complete description of a
Expand Down Expand Up @@ -232,7 +232,7 @@ func (c *Cache) GetBytes(id ActionID) ([]byte, Entry, error) {
return nil, entry, err
}

data, err := ioutil.ReadFile(outputFile)
data, err := robustio.ReadFile(outputFile)
if err != nil {
return nil, entry, err
}
Expand Down
18 changes: 12 additions & 6 deletions internal/renameio/renameio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//+build !plan9
// +build !plan9

package renameio

Expand All @@ -22,8 +22,6 @@ import (
"github.com/golangci/golangci-lint/internal/robustio"
)

const windowsOS = "windows"

func TestConcurrentReadsAndWrites(t *testing.T) {
dir, err := ioutil.TempDir("", "renameio")
if err != nil {
Expand Down Expand Up @@ -117,7 +115,7 @@ func TestConcurrentReadsAndWrites(t *testing.T) {
}

var minWriteSuccesses int64 = attempts
if runtime.GOOS == windowsOS {
if runtime.GOOS == "windows" {
// Windows produces frequent "Access is denied" errors under heavy rename load.
// As long as those are the only errors and *some* of the writes succeed, we're happy.
minWriteSuccesses = attempts / 4
Expand All @@ -130,10 +128,18 @@ func TestConcurrentReadsAndWrites(t *testing.T) {
}

var minReadSuccesses int64 = attempts
if runtime.GOOS == windowsOS {

switch runtime.GOOS {
case "windows":
// Windows produces frequent "Access is denied" errors under heavy rename load.
// As long as those are the only errors and *some* of the writes succeed, we're happy.
// As long as those are the only errors and *some* of the reads succeed, we're happy.
minReadSuccesses = attempts / 4

case "darwin":
// The filesystem on macOS 10.14 occasionally fails with "no such file or
// directory" errors. See https://golang.org/issue/33041. The flake rate is
// fairly low, so ensure that at least 75% of attempts succeed.
minReadSuccesses = attempts - (attempts / 4)
}

if readSuccesses < minReadSuccesses {
Expand Down
4 changes: 2 additions & 2 deletions internal/renameio/umask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//+build !nacl,!plan9,!windows,!js
// +build !plan9,!windows,!js

package renameio

Expand All @@ -19,6 +19,7 @@ func TestWriteFileModeAppliesUmask(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create temporary directory: %v", err)
}
defer os.RemoveAll(dir)

const mode = 0644
const umask = 0007
Expand All @@ -29,7 +30,6 @@ func TestWriteFileModeAppliesUmask(t *testing.T) {
if err != nil {
t.Fatalf("Failed to write file: %v", err)
}
defer os.RemoveAll(dir)

fi, err := os.Stat(file)
if err != nil {
Expand Down
29 changes: 29 additions & 0 deletions internal/robustio/robustio_darwin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package robustio

import (
"os"
"syscall"
)

const errFileNotFound = syscall.ENOENT

// isEphemeralError returns true if err may be resolved by waiting.
func isEphemeralError(err error) bool {
switch werr := err.(type) {
case *os.PathError:
err = werr.Err
case *os.LinkError:
err = werr.Err
case *os.SyscallError:
err = werr.Err

}
if errno, ok := err.(syscall.Errno); ok {
return errno == errFileNotFound
}
return false
}
93 changes: 93 additions & 0 deletions internal/robustio/robustio_flaky.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// +build windows darwin

package robustio

import (
"io/ioutil"
"math/rand"
"os"
"syscall"
"time"
)

const arbitraryTimeout = 500 * time.Millisecond

const ERROR_SHARING_VIOLATION = 32

// retry retries ephemeral errors from f up to an arbitrary timeout
// to work around filesystem flakiness on Windows and Darwin.
func retry(f func() (err error, mayRetry bool)) error {
var (
bestErr error
lowestErrno syscall.Errno
start time.Time
nextSleep time.Duration = 1 * time.Millisecond
)
for {
err, mayRetry := f()
if err == nil || !mayRetry {
return err
}

if errno, ok := err.(syscall.Errno); ok && (lowestErrno == 0 || errno < lowestErrno) {
bestErr = err
lowestErrno = errno
} else if bestErr == nil {
bestErr = err
}

if start.IsZero() {
start = time.Now()
} else if d := time.Since(start) + nextSleep; d >= arbitraryTimeout {
break
}
time.Sleep(nextSleep)
nextSleep += time.Duration(rand.Int63n(int64(nextSleep)))
}

return bestErr
}

// rename is like os.Rename, but retries ephemeral errors.
//
// On windows it wraps os.Rename, which (as of 2019-06-04) uses MoveFileEx with
// MOVEFILE_REPLACE_EXISTING.
//
// Windows also provides a different system call, ReplaceFile,
// that provides similar semantics, but perhaps preserves more metadata. (The
// documentation on the differences between the two is very sparse.)
//
// Empirical error rates with MoveFileEx are lower under modest concurrency, so
// for now we're sticking with what the os package already provides.
func rename(oldpath, newpath string) (err error) {
return retry(func() (err error, mayRetry bool) {
err = os.Rename(oldpath, newpath)
return err, isEphemeralError(err)
})
}

// readFile is like ioutil.ReadFile, but retries ephemeral errors.
func readFile(filename string) ([]byte, error) {
var b []byte
err := retry(func() (err error, mayRetry bool) {
b, err = ioutil.ReadFile(filename)

// Unlike in rename, we do not retry errFileNotFound here: it can occur
// as a spurious error, but the file may also genuinely not exist, so the
// increase in robustness is probably not worth the extra latency.

return err, isEphemeralError(err) && err != errFileNotFound
})
return b, err
}

func removeAll(path string) error {
return retry(func() (err error, mayRetry bool) {
err = os.RemoveAll(path)
return err, isEphemeralError(err)
})
}
2 changes: 1 addition & 1 deletion internal/robustio/robustio_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//+build !windows
//+build !windows,!darwin

package robustio

Expand Down
89 changes: 9 additions & 80 deletions internal/robustio/robustio_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,93 +5,22 @@
package robustio

import (
"io/ioutil"
"math/rand"
"os"
"syscall"
"time"
)

const arbitraryTimeout = 500 * time.Millisecond

const ERROR_SHARING_VIOLATION = 32

// retry retries ephemeral errors from f up to an arbitrary timeout
// to work around spurious filesystem errors on Windows
func retry(f func() (err error, mayRetry bool)) error {
var (
bestErr error
lowestErrno syscall.Errno
start time.Time
nextSleep time.Duration = 1 * time.Millisecond
)
for {
err, mayRetry := f()
if err == nil || !mayRetry {
return err
}

if errno, ok := err.(syscall.Errno); ok && (lowestErrno == 0 || errno < lowestErrno) {
bestErr = err
lowestErrno = errno
} else if bestErr == nil {
bestErr = err
}

if start.IsZero() {
start = time.Now()
} else if d := time.Since(start) + nextSleep; d >= arbitraryTimeout {
break
}
time.Sleep(nextSleep)
nextSleep += time.Duration(rand.Int63n(int64(nextSleep)))
}

return bestErr
}

// rename is like os.Rename, but retries ephemeral errors.
//
// It wraps os.Rename, which (as of 2019-06-04) uses MoveFileEx with
// MOVEFILE_REPLACE_EXISTING.
//
// Windows also provides a different system call, ReplaceFile,
// that provides similar semantics, but perhaps preserves more metadata. (The
// documentation on the differences between the two is very sparse.)
//
// Empirical error rates with MoveFileEx are lower under modest concurrency, so
// for now we're sticking with what the os package already provides.
func rename(oldpath, newpath string) (err error) {
return retry(func() (err error, mayRetry bool) {
err = os.Rename(oldpath, newpath)
return err, isEphemeralError(err)
})
}

// readFile is like ioutil.ReadFile, but retries ephemeral errors.
func readFile(filename string) ([]byte, error) {
var b []byte
err := retry(func() (err error, mayRetry bool) {
b, err = ioutil.ReadFile(filename)

// Unlike in rename, we do not retry ERROR_FILE_NOT_FOUND here: it can occur
// as a spurious error, but the file may also genuinely not exist, so the
// increase in robustness is probably not worth the extra latency.

return err, isEphemeralError(err) && err != syscall.ERROR_FILE_NOT_FOUND
})
return b, err
}

func removeAll(path string) error {
return retry(func() (err error, mayRetry bool) {
err = os.RemoveAll(path)
return err, isEphemeralError(err)
})
}
const errFileNotFound = syscall.ERROR_FILE_NOT_FOUND

// isEphemeralError returns true if err may be resolved by waiting.
func isEphemeralError(err error) bool {
switch werr := err.(type) {
case *os.PathError:
err = werr.Err
case *os.LinkError:
err = werr.Err
case *os.SyscallError:
err = werr.Err
}
if errno, ok := err.(syscall.Errno); ok {
switch errno {
case syscall.ERROR_ACCESS_DENIED,
Expand Down
9 changes: 2 additions & 7 deletions pkg/golinters/goanalysis/load/guard.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ import (
)

type Guard struct {
loadMutexes map[*packages.Package]*sync.Mutex
mutexForExportData sync.Mutex
mutex sync.Mutex
loadMutexes map[*packages.Package]*sync.Mutex
mutex sync.Mutex
}

func NewGuard() *Guard {
Expand All @@ -26,10 +25,6 @@ func (g *Guard) MutexForPkg(pkg *packages.Package) *sync.Mutex {
return g.loadMutexes[pkg]
}

func (g *Guard) MutexForExportData() *sync.Mutex {
return &g.mutexForExportData
}

func (g *Guard) Mutex() *sync.Mutex {
return &g.mutex
}
Loading

0 comments on commit 9eb569a

Please sign in to comment.