From fbaec31d2642fc43e0ca6b586e3cb3a53e116989 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:28:04 +0100 Subject: [PATCH 01/18] feat: extend stopwatch --- pkg/timeutils/stopwatch.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/pkg/timeutils/stopwatch.go b/pkg/timeutils/stopwatch.go index d944dea2ea0e..95b16de9fc70 100644 --- a/pkg/timeutils/stopwatch.go +++ b/pkg/timeutils/stopwatch.go @@ -114,3 +114,25 @@ func (s *Stopwatch) TrackStage(name string, f func()) { s.stages[name] += time.Since(startedAt) s.mu.Unlock() } + +func (s *Stopwatch) TrackStageErr(name string, f func() error) error { + startedAt := time.Now() + err := f() + + s.mu.Lock() + s.stages[name] += time.Since(startedAt) + s.mu.Unlock() + + return err +} + +func TrackStage[T any](s *Stopwatch, name string, f func() (T, error)) (T, error) { + var result T + var err error + + s.TrackStage(name, func() { + result, err = f() + }) + + return result, err +} From 2bfe70faa1b214f3cf4309d75f05f71b4a1697de Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:31:23 +0100 Subject: [PATCH 02/18] chore: cosmetic changes --- internal/cache/cache.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index ed52fcf4ac43..58f9525b7639 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -25,8 +25,11 @@ const ( HashModeNeedAllDeps ) -// Cache is a per-package data cache. A cached data is invalidated when -// package, or it's dependencies change. +var ErrMissing = errors.New("missing data") + +// Cache is a per-package data cache. +// A cached data is invalidated when package, +// or it's dependencies change. type Cache struct { lowLevelCache cache.Cache pkgHashes sync.Map @@ -90,8 +93,6 @@ func (c *Cache) Put(pkg *packages.Package, mode HashMode, key string, data any) return nil } -var ErrMissing = errors.New("missing data") - func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data any) error { var aID cache.ActionID var err error @@ -148,9 +149,9 @@ func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionI return key.Sum(), nil } -// packageHash computes a package's hash. The hash is based on all Go -// files that make up the package, as well as the hashes of imported -// packages. +// packageHash computes a package's hash. +// The hash is based on all Go files that make up the package, +// as well as the hashes of imported packages. func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error) { type hashResults map[HashMode]string hashResI, ok := c.pkgHashes.Load(pkg) From f54f29b5f1c4c8998faec2a2572687e0063d0d10 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:33:35 +0100 Subject: [PATCH 03/18] chore: factorize encode and decode --- internal/cache/cache.go | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 58f9525b7639..747ea4eb9cc4 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -57,13 +57,9 @@ func (c *Cache) Close() { } func (c *Cache) Put(pkg *packages.Package, mode HashMode, key string, data any) error { - var err error - buf := &bytes.Buffer{} - c.sw.TrackStage("gob", func() { - err = gob.NewEncoder(buf).Encode(data) - }) + buf, err := c.encode(data) if err != nil { - return fmt.Errorf("failed to gob encode: %w", err) + return err } var aID cache.ActionID @@ -123,14 +119,7 @@ func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data any) return fmt.Errorf("failed to get data from low-level cache by key %s for package %s: %w", key, pkg.Name, err) } - c.sw.TrackStage("gob", func() { - err = gob.NewDecoder(bytes.NewReader(b)).Decode(data) - }) - if err != nil { - return fmt.Errorf("failed to gob decode: %w", err) - } - - return nil + return c.decode(b, data) } func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionID, error) { @@ -228,6 +217,29 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error return hashRes[mode], nil } +func (c *Cache) encode(data any) (*bytes.Buffer, error) { + buf := &bytes.Buffer{} + err := c.sw.TrackStageErr("gob", func() error { + return gob.NewEncoder(buf).Encode(data) + }) + if err != nil { + return nil, fmt.Errorf("failed to gob encode: %w", err) + } + + return buf, nil +} + +func (c *Cache) decode(b []byte, data any) error { + err := c.sw.TrackStageErr("gob", func() error { + return gob.NewDecoder(bytes.NewReader(b)).Decode(data) + }) + if err != nil { + return fmt.Errorf("failed to gob decode: %w", err) + } + + return nil +} + func SetSalt(b *bytes.Buffer) { cache.SetSalt(b.Bytes()) } From 216b378846d1de20e89bb8c2d4d8fcabf6b3c798 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:35:27 +0100 Subject: [PATCH 04/18] chore: factorize buildKey --- internal/cache/cache.go | 47 ++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 747ea4eb9cc4..4b2f69782282 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -62,24 +62,14 @@ func (c *Cache) Put(pkg *packages.Package, mode HashMode, key string, data any) return err } - var aID cache.ActionID - - c.sw.TrackStage("key build", func() { - aID, err = c.pkgActionID(pkg, mode) - if err == nil { - subkey, subkeyErr := cache.Subkey(aID, key) - if subkeyErr != nil { - err = fmt.Errorf("failed to build subkey: %w", subkeyErr) - } - aID = subkey - } - }) + actionID, err := c.buildKey(pkg, mode, key) if err != nil { return fmt.Errorf("failed to calculate package %s action id: %w", pkg.Name, err) } + c.ioSem <- struct{}{} c.sw.TrackStage("cache io", func() { - err = cache.PutBytes(c.lowLevelCache, aID, buf.Bytes()) + err = cache.PutBytes(c.lowLevelCache, actionID, buf.Bytes()) }) <-c.ioSem if err != nil { @@ -90,18 +80,7 @@ func (c *Cache) Put(pkg *packages.Package, mode HashMode, key string, data any) } func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data any) error { - var aID cache.ActionID - var err error - c.sw.TrackStage("key build", func() { - aID, err = c.pkgActionID(pkg, mode) - if err == nil { - subkey, subkeyErr := cache.Subkey(aID, key) - if subkeyErr != nil { - err = fmt.Errorf("failed to build subkey: %w", subkeyErr) - } - aID = subkey - } - }) + actionID, err := c.buildKey(pkg, mode, key) if err != nil { return fmt.Errorf("failed to calculate package %s action id: %w", pkg.Name, err) } @@ -109,7 +88,7 @@ func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data any) var b []byte c.ioSem <- struct{}{} c.sw.TrackStage("cache io", func() { - b, _, err = cache.GetBytes(c.lowLevelCache, aID) + b, _, err = cache.GetBytes(c.lowLevelCache, actionID) }) <-c.ioSem if err != nil { @@ -122,6 +101,22 @@ func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data any) return c.decode(b, data) } +func (c *Cache) buildKey(pkg *packages.Package, mode HashMode, key string) (cache.ActionID, error) { + return timeutils.TrackStage[cache.ActionID](c.sw, "key build", func() (cache.ActionID, error) { + actionID, err := c.pkgActionID(pkg, mode) + if err != nil { + return actionID, err + } + + subkey, subkeyErr := cache.Subkey(actionID, key) + if subkeyErr != nil { + return actionID, fmt.Errorf("failed to build subkey: %w", subkeyErr) + } + + return subkey, nil + }) +} + func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionID, error) { hash, err := c.packageHash(pkg, mode) if err != nil { From c7d018b68afb283098f59632e25afd89e30e1086 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:37:07 +0100 Subject: [PATCH 05/18] chore: factorize putBytes --- internal/cache/cache.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 4b2f69782282..0c045207a6fc 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -67,11 +67,7 @@ func (c *Cache) Put(pkg *packages.Package, mode HashMode, key string, data any) return fmt.Errorf("failed to calculate package %s action id: %w", pkg.Name, err) } - c.ioSem <- struct{}{} - c.sw.TrackStage("cache io", func() { - err = cache.PutBytes(c.lowLevelCache, actionID, buf.Bytes()) - }) - <-c.ioSem + err = c.putBytes(actionID, buf) if err != nil { return fmt.Errorf("failed to save data to low-level cache by key %s for package %s: %w", key, pkg.Name, err) } @@ -212,6 +208,22 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error return hashRes[mode], nil } +func (c *Cache) putBytes(actionID cache.ActionID, buf *bytes.Buffer) error { + c.ioSem <- struct{}{} + + err := c.sw.TrackStageErr("cache io", func() error { + return cache.PutBytes(c.lowLevelCache, actionID, buf.Bytes()) + }) + + <-c.ioSem + + if err != nil { + return err + } + + return nil +} + func (c *Cache) encode(data any) (*bytes.Buffer, error) { buf := &bytes.Buffer{} err := c.sw.TrackStageErr("gob", func() error { From 03efcb31d809991a11e4b8b564e55ccf9e66334c Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:38:17 +0100 Subject: [PATCH 06/18] chore: factorize getBytes --- internal/cache/cache.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 0c045207a6fc..24a2e98cde82 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -81,12 +81,7 @@ func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data any) return fmt.Errorf("failed to calculate package %s action id: %w", pkg.Name, err) } - var b []byte - c.ioSem <- struct{}{} - c.sw.TrackStage("cache io", func() { - b, _, err = cache.GetBytes(c.lowLevelCache, actionID) - }) - <-c.ioSem + cachedData, err := c.getBytes(actionID) if err != nil { if cache.IsErrMissing(err) { return ErrMissing @@ -94,7 +89,7 @@ func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data any) return fmt.Errorf("failed to get data from low-level cache by key %s for package %s: %w", key, pkg.Name, err) } - return c.decode(b, data) + return c.decode(cachedData, data) } func (c *Cache) buildKey(pkg *packages.Package, mode HashMode, key string) (cache.ActionID, error) { @@ -224,6 +219,23 @@ func (c *Cache) putBytes(actionID cache.ActionID, buf *bytes.Buffer) error { return nil } +func (c *Cache) getBytes(actionID cache.ActionID) ([]byte, error) { + c.ioSem <- struct{}{} + + cachedData, err := timeutils.TrackStage[[]byte](c.sw, "cache io", func() ([]byte, error) { + b, _, errGB := cache.GetBytes(c.lowLevelCache, actionID) + return b, errGB + }) + + <-c.ioSem + + if err != nil { + return nil, err + } + + return cachedData, nil +} + func (c *Cache) encode(data any) (*bytes.Buffer, error) { buf := &bytes.Buffer{} err := c.sw.TrackStageErr("gob", func() error { From 4390f8a4d9ddc39027764c94a3b2a432bb095d92 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:39:23 +0100 Subject: [PATCH 07/18] chore: factorize fileHash --- internal/cache/cache.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 24a2e98cde82..d617ab1cf35b 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -147,9 +147,7 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error fmt.Fprintf(key, "pkgpath %s\n", pkg.PkgPath) for _, f := range pkg.CompiledGoFiles { - c.ioSem <- struct{}{} - h, fErr := cache.FileHash(f) - <-c.ioSem + h, fErr := c.fileHash(f) if fErr != nil { return "", fmt.Errorf("failed to calculate file %s hash: %w", f, fErr) } @@ -236,6 +234,20 @@ func (c *Cache) getBytes(actionID cache.ActionID) ([]byte, error) { return cachedData, nil } +func (c *Cache) fileHash(f string) ([cache.HashSize]byte, error) { + c.ioSem <- struct{}{} + + h, err := cache.FileHash(f) + + <-c.ioSem + + if err != nil { + return [cache.HashSize]byte{}, err + } + + return h, nil +} + func (c *Cache) encode(data any) (*bytes.Buffer, error) { buf := &bytes.Buffer{} err := c.sw.TrackStageErr("gob", func() error { From 7e6c2c631954f416c0fcea70274690952510ef56 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:45:04 +0100 Subject: [PATCH 08/18] refactor: map to slice --- internal/cache/cache.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index d617ab1cf35b..a5599f83fb6b 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -7,9 +7,11 @@ import ( "errors" "fmt" "runtime" - "sort" + "slices" + "strings" "sync" + "golang.org/x/exp/maps" "golang.org/x/tools/go/packages" "github.com/golangci/golangci-lint/internal/go/cache" @@ -156,12 +158,10 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error curSum := key.Sum() hashRes[HashModeNeedOnlySelf] = hex.EncodeToString(curSum[:]) - imps := make([]*packages.Package, 0, len(pkg.Imports)) - for _, imp := range pkg.Imports { - imps = append(imps, imp) - } - sort.Slice(imps, func(i, j int) bool { - return imps[i].PkgPath < imps[j].PkgPath + imps := maps.Values(pkg.Imports) + + slices.SortFunc(imps, func(a, b *packages.Package) int { + return strings.Compare(a.PkgPath, b.PkgPath) }) calcDepsHash := func(depMode HashMode) error { From c94dcfb66b4ac19358dfd1af2c91d8904b29bd50 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:47:15 +0100 Subject: [PATCH 09/18] refactor: extract computeDepsHash --- internal/cache/cache.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index a5599f83fb6b..e2ca39d9a1c6 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -164,30 +164,14 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error return strings.Compare(a.PkgPath, b.PkgPath) }) - calcDepsHash := func(depMode HashMode) error { - for _, dep := range imps { - if dep.PkgPath == "unsafe" { - continue - } - - depHash, depErr := c.packageHash(dep, depMode) - if depErr != nil { - return fmt.Errorf("failed to calculate hash for dependency %s with mode %d: %w", dep.Name, depMode, depErr) - } - - fmt.Fprintf(key, "import %s %s\n", dep.PkgPath, depHash) - } - return nil - } - - if err := calcDepsHash(HashModeNeedOnlySelf); err != nil { + if err := c.computeDepsHash(HashModeNeedOnlySelf, imps, key); err != nil { return "", err } curSum = key.Sum() hashRes[HashModeNeedDirectDeps] = hex.EncodeToString(curSum[:]) - if err := calcDepsHash(HashModeNeedAllDeps); err != nil { + if err := c.computeDepsHash(HashModeNeedAllDeps, imps, key); err != nil { return "", err } curSum = key.Sum() @@ -201,6 +185,23 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error return hashRes[mode], nil } +func (c *Cache) computeDepsHash(depMode HashMode, imps []*packages.Package, key *cache.Hash) error { + for _, dep := range imps { + if dep.PkgPath == "unsafe" { + continue + } + + depHash, err := c.packageHash(dep, depMode) + if err != nil { + return fmt.Errorf("failed to calculate hash for dependency %s with mode %d: %w", dep.Name, depMode, err) + } + + fmt.Fprintf(key, "import %s %s\n", dep.PkgPath, depHash) + } + + return nil +} + func (c *Cache) putBytes(actionID cache.ActionID, buf *bytes.Buffer) error { c.ioSem <- struct{}{} From 120c80e5cd1f769c83d1bd6d5ecbdb0238c14586 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:48:01 +0100 Subject: [PATCH 10/18] refactor: move hashResults type --- internal/cache/cache.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index e2ca39d9a1c6..9772c0d0a651 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -29,6 +29,8 @@ const ( var ErrMissing = errors.New("missing data") +type hashResults map[HashMode]string + // Cache is a per-package data cache. // A cached data is invalidated when package, // or it's dependencies change. @@ -130,7 +132,6 @@ func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionI // The hash is based on all Go files that make up the package, // as well as the hashes of imported packages. func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error) { - type hashResults map[HashMode]string hashResI, ok := c.pkgHashes.Load(pkg) if ok { hashRes := hashResI.(hashResults) From daed3b80a31f582812d654dfcded2063738a3ccb Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 2 Nov 2024 21:49:24 +0100 Subject: [PATCH 11/18] chore: add some air --- internal/cache/cache.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 9772c0d0a651..a1c5a40319f5 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -122,6 +122,7 @@ func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionI if err != nil { return cache.ActionID{}, fmt.Errorf("failed to make a hash: %w", err) } + fmt.Fprintf(key, "pkgpath %s\n", pkg.PkgPath) fmt.Fprintf(key, "pkghash %s\n", hash) @@ -138,24 +139,28 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error if _, ok := hashRes[mode]; !ok { return "", fmt.Errorf("no mode %d in hash result", mode) } + return hashRes[mode], nil } - hashRes := hashResults{} - key, err := cache.NewHash("package hash") if err != nil { return "", fmt.Errorf("failed to make a hash: %w", err) } + hashRes := hashResults{} + fmt.Fprintf(key, "pkgpath %s\n", pkg.PkgPath) + for _, f := range pkg.CompiledGoFiles { h, fErr := c.fileHash(f) if fErr != nil { return "", fmt.Errorf("failed to calculate file %s hash: %w", f, fErr) } + fmt.Fprintf(key, "file %s %x\n", f, h) } + curSum := key.Sum() hashRes[HashModeNeedOnlySelf] = hex.EncodeToString(curSum[:]) @@ -175,6 +180,7 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error if err := c.computeDepsHash(HashModeNeedAllDeps, imps, key); err != nil { return "", err } + curSum = key.Sum() hashRes[HashModeNeedAllDeps] = hex.EncodeToString(curSum[:]) @@ -183,6 +189,7 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error } c.pkgHashes.Store(pkg, hashRes) + return hashRes[mode], nil } From ecf19cac7c126f4fef0390ed361df638e73a4b65 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 3 Nov 2024 13:54:51 +0100 Subject: [PATCH 12/18] refactor: simplify Close method --- internal/cache/cache.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index a1c5a40319f5..009351a98285 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -52,12 +52,10 @@ func NewCache(sw *timeutils.Stopwatch, log logutils.Log) (*Cache, error) { } func (c *Cache) Close() { - c.sw.TrackStage("close", func() { - err := c.lowLevelCache.Close() - if err != nil { - c.log.Errorf("cache close: %v", err) - } - }) + err := c.sw.TrackStageErr("close", c.lowLevelCache.Close) + if err != nil { + c.log.Errorf("cache close: %v", err) + } } func (c *Cache) Put(pkg *packages.Package, mode HashMode, key string, data any) error { From 431a802bb7b558e3f1de39832f527a66490873dc Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 3 Nov 2024 14:00:01 +0100 Subject: [PATCH 13/18] chore: factorize computePkgHash --- internal/cache/cache.go | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 009351a98285..f00b4a798b29 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -127,9 +127,6 @@ func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionI return key.Sum(), nil } -// packageHash computes a package's hash. -// The hash is based on all Go files that make up the package, -// as well as the hashes of imported packages. func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error) { hashResI, ok := c.pkgHashes.Load(pkg) if ok { @@ -141,9 +138,27 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error return hashRes[mode], nil } + hashRes, err := c.computePkgHash(pkg) + if err != nil { + return "", err + } + + if _, ok := hashRes[mode]; !ok { + return "", fmt.Errorf("invalid mode %d", mode) + } + + c.pkgHashes.Store(pkg, hashRes) + + return hashRes[mode], nil +} + +// computePkgHash computes a package's hash. +// The hash is based on all Go files that make up the package, +// as well as the hashes of imported packages. +func (c *Cache) computePkgHash(pkg *packages.Package) (hashResults, error) { key, err := cache.NewHash("package hash") if err != nil { - return "", fmt.Errorf("failed to make a hash: %w", err) + return nil, fmt.Errorf("failed to make a hash: %w", err) } hashRes := hashResults{} @@ -153,7 +168,7 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error for _, f := range pkg.CompiledGoFiles { h, fErr := c.fileHash(f) if fErr != nil { - return "", fmt.Errorf("failed to calculate file %s hash: %w", f, fErr) + return nil, fmt.Errorf("failed to calculate file %s hash: %w", f, fErr) } fmt.Fprintf(key, "file %s %x\n", f, h) @@ -169,26 +184,20 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error }) if err := c.computeDepsHash(HashModeNeedOnlySelf, imps, key); err != nil { - return "", err + return nil, err } curSum = key.Sum() hashRes[HashModeNeedDirectDeps] = hex.EncodeToString(curSum[:]) if err := c.computeDepsHash(HashModeNeedAllDeps, imps, key); err != nil { - return "", err + return nil, err } curSum = key.Sum() hashRes[HashModeNeedAllDeps] = hex.EncodeToString(curSum[:]) - if _, ok := hashRes[mode]; !ok { - return "", fmt.Errorf("invalid mode %d", mode) - } - - c.pkgHashes.Store(pkg, hashRes) - - return hashRes[mode], nil + return hashRes, nil } func (c *Cache) computeDepsHash(depMode HashMode, imps []*packages.Package, key *cache.Hash) error { From cf511284229ac041ce1052e966a138cb42c0c768 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 3 Nov 2024 15:03:29 +0100 Subject: [PATCH 14/18] refactor: simplify packageHash method --- internal/cache/cache.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index f00b4a798b29..e44ce705b3f4 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -128,14 +128,14 @@ func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionI } func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error) { - hashResI, ok := c.pkgHashes.Load(pkg) - if ok { - hashRes := hashResI.(hashResults) - if _, ok := hashRes[mode]; !ok { - return "", fmt.Errorf("no mode %d in hash result", mode) + results, found := c.pkgHashes.Load(pkg) + if found { + hashRes := results.(hashResults) + if result, ok := hashRes[mode]; ok { + return result, nil } - return hashRes[mode], nil + return "", fmt.Errorf("no mode %d in hash result", mode) } hashRes, err := c.computePkgHash(pkg) @@ -143,13 +143,14 @@ func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error return "", err } - if _, ok := hashRes[mode]; !ok { + result, found := hashRes[mode] + if !found { return "", fmt.Errorf("invalid mode %d", mode) } c.pkgHashes.Store(pkg, hashRes) - return hashRes[mode], nil + return result, nil } // computePkgHash computes a package's hash. From dcb6b71867bc531f27d8d15533b05740876794d2 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 3 Nov 2024 15:03:52 +0100 Subject: [PATCH 15/18] tests: add tests --- internal/cache/cache_test.go | 117 +++++++++++++++++++++++++++++++ internal/cache/testdata/hello.go | 7 ++ 2 files changed, 124 insertions(+) create mode 100644 internal/cache/cache_test.go create mode 100644 internal/cache/testdata/hello.go diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go new file mode 100644 index 000000000000..013c44ad726a --- /dev/null +++ b/internal/cache/cache_test.go @@ -0,0 +1,117 @@ +package cache + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/tools/go/packages" + + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/timeutils" +) + +func setupCache(t *testing.T) *Cache { + t.Helper() + + log := logutils.NewStderrLog("skip") + sw := timeutils.NewStopwatch("pkgcache", log) + + pkgCache, err := NewCache(sw, log) + require.NoError(t, err) + + return pkgCache +} + +func fakePackage() *packages.Package { + return &packages.Package{ + PkgPath: "github.com/golangci/example", + CompiledGoFiles: []string{ + "./testdata/hello.go", + }, + Imports: map[string]*packages.Package{ + "a": { + PkgPath: "github.com/golangci/example/a", + }, + "b": { + PkgPath: "github.com/golangci/example/b", + }, + "unsafe": { + PkgPath: "unsafe", + }, + }, + } +} + +func Test_buildKey(t *testing.T) { + pkgCache := setupCache(t) + + pkg := fakePackage() + + actionID, err := pkgCache.buildKey(pkg, HashModeNeedAllDeps, "") + require.NoError(t, err) + + assert.Equal(t, "f32bf1bf010aa9b570e081c64ec9e22e17aafa1e822990ba952905ec5fdf8d9d", fmt.Sprintf("%x", actionID)) +} + +func Test_pkgActionID(t *testing.T) { + pkgCache := setupCache(t) + + pkg := fakePackage() + + actionID, err := pkgCache.pkgActionID(pkg, HashModeNeedAllDeps) + require.NoError(t, err) + + assert.Equal(t, "f690f05acd1024386ae912d9ad9c04080523b9a899f6afe56ab3108d88215c1d", fmt.Sprintf("%x", actionID)) +} + +func Test_packageHash_load(t *testing.T) { + pkgCache := setupCache(t) + + pkg := fakePackage() + + pkgCache.pkgHashes.Store(pkg, hashResults{HashModeNeedAllDeps: "fake"}) + + hash, err := pkgCache.packageHash(pkg, HashModeNeedAllDeps) + require.NoError(t, err) + + assert.Equal(t, "fake", hash) +} + +func Test_packageHash_store(t *testing.T) { + pkgCache := setupCache(t) + + pkg := fakePackage() + + hash, err := pkgCache.packageHash(pkg, HashModeNeedAllDeps) + require.NoError(t, err) + + assert.Equal(t, "9c602ef861197b6807e82c99caa7c4042eb03c1a92886303fb02893744355131", hash) + + results, ok := pkgCache.pkgHashes.Load(pkg) + require.True(t, ok) + + hashRes := results.(hashResults) + + require.Len(t, hashRes, 3) + + assert.Equal(t, "8978e3d76c6f99e9663558d7147a7790f229a676804d1fde706a611898547b74", hashRes[HashModeNeedOnlySelf]) + assert.Equal(t, "b1aef902a0619b5cbfc2d6e2e91a73dd58dd448e58274b2d7a5ff8efd97aefa4", hashRes[HashModeNeedDirectDeps]) + assert.Equal(t, "9c602ef861197b6807e82c99caa7c4042eb03c1a92886303fb02893744355131", hashRes[HashModeNeedAllDeps]) +} + +func Test_computeHash(t *testing.T) { + pkgCache := setupCache(t) + + pkg := fakePackage() + + results, err := pkgCache.computePkgHash(pkg) + require.NoError(t, err) + + require.Len(t, results, 3) + + assert.Equal(t, "8978e3d76c6f99e9663558d7147a7790f229a676804d1fde706a611898547b74", results[HashModeNeedOnlySelf]) + assert.Equal(t, "b1aef902a0619b5cbfc2d6e2e91a73dd58dd448e58274b2d7a5ff8efd97aefa4", results[HashModeNeedDirectDeps]) + assert.Equal(t, "9c602ef861197b6807e82c99caa7c4042eb03c1a92886303fb02893744355131", results[HashModeNeedAllDeps]) +} diff --git a/internal/cache/testdata/hello.go b/internal/cache/testdata/hello.go new file mode 100644 index 000000000000..2e65d51de9cb --- /dev/null +++ b/internal/cache/testdata/hello.go @@ -0,0 +1,7 @@ +package testdata + +import "fmt" + +func Hello() { + fmt.Println("hello world") +} From ad84286f49a31d8ad57cca20d4dc4b525ec6c178 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 5 Nov 2024 12:47:16 +0100 Subject: [PATCH 16/18] tests: rename --- internal/cache/cache_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go index 013c44ad726a..12b0888afb7e 100644 --- a/internal/cache/cache_test.go +++ b/internal/cache/cache_test.go @@ -44,7 +44,7 @@ func fakePackage() *packages.Package { } } -func Test_buildKey(t *testing.T) { +func TestCache_buildKey(t *testing.T) { pkgCache := setupCache(t) pkg := fakePackage() @@ -55,7 +55,7 @@ func Test_buildKey(t *testing.T) { assert.Equal(t, "f32bf1bf010aa9b570e081c64ec9e22e17aafa1e822990ba952905ec5fdf8d9d", fmt.Sprintf("%x", actionID)) } -func Test_pkgActionID(t *testing.T) { +func TestCache_pkgActionID(t *testing.T) { pkgCache := setupCache(t) pkg := fakePackage() @@ -66,7 +66,7 @@ func Test_pkgActionID(t *testing.T) { assert.Equal(t, "f690f05acd1024386ae912d9ad9c04080523b9a899f6afe56ab3108d88215c1d", fmt.Sprintf("%x", actionID)) } -func Test_packageHash_load(t *testing.T) { +func TestCache_packageHash_load(t *testing.T) { pkgCache := setupCache(t) pkg := fakePackage() @@ -79,7 +79,7 @@ func Test_packageHash_load(t *testing.T) { assert.Equal(t, "fake", hash) } -func Test_packageHash_store(t *testing.T) { +func TestCache_packageHash_store(t *testing.T) { pkgCache := setupCache(t) pkg := fakePackage() @@ -101,7 +101,7 @@ func Test_packageHash_store(t *testing.T) { assert.Equal(t, "9c602ef861197b6807e82c99caa7c4042eb03c1a92886303fb02893744355131", hashRes[HashModeNeedAllDeps]) } -func Test_computeHash(t *testing.T) { +func TestCache_computeHash(t *testing.T) { pkgCache := setupCache(t) pkg := fakePackage() From 92fd8c8c09db479c3c3962786ecfdcf99fa9ab1e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 5 Nov 2024 13:01:42 +0100 Subject: [PATCH 17/18] tests: Get and Put --- internal/cache/cache_test.go | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go index 12b0888afb7e..616e6344a281 100644 --- a/internal/cache/cache_test.go +++ b/internal/cache/cache_test.go @@ -44,6 +44,47 @@ func fakePackage() *packages.Package { } } +type Foo struct { + Value string +} + +func TestCache_Put(t *testing.T) { + t.Setenv("GOLANGCI_LINT_CACHE", t.TempDir()) + + pkgCache := setupCache(t) + + pkg := fakePackage() + + in := &Foo{Value: "hello"} + + err := pkgCache.Put(pkg, HashModeNeedAllDeps, "key", in) + require.NoError(t, err) + + out := &Foo{} + err = pkgCache.Get(pkg, HashModeNeedAllDeps, "key", out) + require.NoError(t, err) + + assert.Equal(t, in, out) + + pkgCache.Close() +} + +func TestCache_Get_missing_data(t *testing.T) { + t.Setenv("GOLANGCI_LINT_CACHE", t.TempDir()) + + pkgCache := setupCache(t) + + pkg := fakePackage() + + out := &Foo{} + err := pkgCache.Get(pkg, HashModeNeedAllDeps, "key", out) + require.Error(t, err) + + require.ErrorIs(t, err, ErrMissing) + + pkgCache.Close() +} + func TestCache_buildKey(t *testing.T) { pkgCache := setupCache(t) From 131f8847b0d213bc8843c81eaf81bc23f83912b1 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 5 Nov 2024 13:35:44 +0100 Subject: [PATCH 18/18] chore: remove redondant typing --- internal/cache/cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index e44ce705b3f4..c249084e1c53 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -95,7 +95,7 @@ func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data any) } func (c *Cache) buildKey(pkg *packages.Package, mode HashMode, key string) (cache.ActionID, error) { - return timeutils.TrackStage[cache.ActionID](c.sw, "key build", func() (cache.ActionID, error) { + return timeutils.TrackStage(c.sw, "key build", func() (cache.ActionID, error) { actionID, err := c.pkgActionID(pkg, mode) if err != nil { return actionID, err @@ -237,7 +237,7 @@ func (c *Cache) putBytes(actionID cache.ActionID, buf *bytes.Buffer) error { func (c *Cache) getBytes(actionID cache.ActionID) ([]byte, error) { c.ioSem <- struct{}{} - cachedData, err := timeutils.TrackStage[[]byte](c.sw, "cache io", func() ([]byte, error) { + cachedData, err := timeutils.TrackStage(c.sw, "cache io", func() ([]byte, error) { b, _, errGB := cache.GetBytes(c.lowLevelCache, actionID) return b, errGB })