Skip to content

Commit

Permalink
reduce mem. usage of unused and update staticcheck (#1063)
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 authored May 3, 2020
1 parent 77e211b commit 52c9b88
Show file tree
Hide file tree
Showing 16 changed files with 182 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
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ 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-20200331202046-9d5940d49312/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
Expand Down Expand Up @@ -420,6 +421,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 52c9b88

Please sign in to comment.