Changing to only caching decls

grantnelson-wf · grantnelson-wf · commit 51bb17e17f16 · 2025-12-10T14:43:13.000-07:00
diff --git a/build/build.go b/build/build.go
@@ -764,6 +764,10 @@ type Session struct {
 	// The files in these sources haven't been sorted nor simplified yet.
 	sources map[string]*sources.Sources
 
+	// cachedDecls is a map of cached declarations.
+	// This is keyed using resolved import paths.
+	cachedDecls map[string]*compiler.DeclCache
+
 	// Binary archives produced during the current session and assumed to be
 	// up to date with input sources and dependencies. In the -w ("watch") mode
 	// must be cleared upon entering watching.
@@ -779,6 +783,7 @@ func NewSession(options *Options) (*Session, error) {
 		options:          options,
 		importPaths:      make(map[string]map[string]string),
 		sources:          make(map[string]*sources.Sources),
+		cachedDecls:      make(map[string]*compiler.DeclCache),
 		UpToDateArchives: make(map[string]*compiler.Archive),
 	}
 	s.xctx = NewBuildContext(s.InstallSuffix(), s.options.BuildTags)
@@ -1055,7 +1060,7 @@ var getExeModTime = func() func() time.Time {
 // to the session's sources map.
 func (s *Session) LoadPackages(pkg *PackageData) (*sources.Sources, error) {
 	if srcs, ok := s.sources[pkg.ImportPath]; ok {
-		return srcs, nil
+		return srcs, nil // already loaded
 	}
 
 	if exeModTime := getExeModTime(); exeModTime.After(pkg.SrcModTime) {
@@ -1080,46 +1085,39 @@ func (s *Session) LoadPackages(pkg *PackageData) (*sources.Sources, error) {
 		pkg.SrcModTime = fileModTime
 	}
 
-	// Try to load the package from the build cache.
-	var srcs *sources.Sources
-	if s.buildCache != nil {
-		cachedSrcs := &sources.Sources{
-			DeclCache: compiler.NewDeclCache(true),
-		}
-		if s.buildCache.Load(cachedSrcs, pkg.ImportPath, pkg.SrcModTime) {
-			srcs = cachedSrcs
-		}
-	}
-
 	// If the package was not found in the cache, build the package
 	// by parsing and augmenting the original files with overlay files.
-	if srcs == nil {
-		fileSet := token.NewFileSet()
-		files, overlayJsFiles, err := parseAndAugment(s.xctx, pkg, pkg.IsTest, fileSet)
-		if err != nil {
-			return nil, err
-		}
-		embed, err := embedFiles(pkg, fileSet, files)
-		if err != nil {
-			return nil, err
-		}
-		if embed != nil {
-			files = append(files, embed)
-		}
+	fileSet := token.NewFileSet()
+	files, overlayJsFiles, err := parseAndAugment(s.xctx, pkg, pkg.IsTest, fileSet)
+	if err != nil {
+		return nil, err
+	}
+	embed, err := embedFiles(pkg, fileSet, files)
+	if err != nil {
+		return nil, err
+	}
+	if embed != nil {
+		files = append(files, embed)
+	}
 
-		srcs = &sources.Sources{
-			ImportPath: pkg.ImportPath,
-			Dir:        pkg.Dir,
-			Files:      files,
-			FileSet:    fileSet,
-			JSFiles:    append(pkg.JSFiles, overlayJsFiles...),
-			DeclCache:  compiler.NewDeclCache(s.buildCache != nil),
-		}
+	srcs := &sources.Sources{
+		ImportPath: pkg.ImportPath,
+		Dir:        pkg.Dir,
+		Files:      files,
+		FileSet:    fileSet,
+		JSFiles:    append(pkg.JSFiles, overlayJsFiles...),
 	}
 
 	// Add the sources to the session's sources map.
 	s.sources[pkg.ImportPath] = srcs
 
+	// Try to load the cached decls from the build cache.
+	if s.buildCache != nil {
+		declCache := &compiler.DeclCache{}
+		s.buildCache.Load(declCache, pkg.ImportPath, pkg.SrcModTime)
+		s.cachedDecls[pkg.ImportPath] = declCache
+	}
+
 	// Import dependencies from the augmented files,
 	// whilst skipping any that have been already imported.
 	for _, importedPkgPath := range srcs.UnresolvedImports(pkg.Imports...) {
@@ -1167,7 +1165,8 @@ func (s *Session) CompilePackage(srcs *sources.Sources, tContext *types.Context)
 		return archive, nil
 	}
 
-	archive, err := compiler.Compile(srcs, tContext, s.options.Minify)
+	declCache := s.cachedDecls[srcs.ImportPath]
+	archive, err := compiler.Compile(srcs, declCache, tContext, s.options.Minify)
 	if err != nil {
 		return nil, err
 	}
@@ -1179,8 +1178,11 @@ func (s *Session) CompilePackage(srcs *sources.Sources, tContext *types.Context)
 	// Store the built package sources in the cache for future use.
 	// The sources should contain all cachable declarations at this point.
 	// Skip storing cache if the sources haven't changed since read from cache.
-	if s.buildCache != nil && srcs.Changed() {
-		s.buildCache.Store(srcs, srcs.ImportPath, time.Now())
+	if s.buildCache != nil {
+		dc := s.cachedDecls[srcs.ImportPath]
+		if dc != nil && dc.Changed() {
+			s.buildCache.Store(dc, srcs.ImportPath, time.Now())
+		}
 	}
 
 	s.UpToDateArchives[srcs.ImportPath] = archive
diff --git a/build/cache/cache.go b/build/cache/cache.go
@@ -91,7 +91,7 @@ var _ Cache = (*BuildCache)(nil)
 // the cache. For example, any artifacts that were cached for a Linux build
 // must not be reused for a non-Linux build. GopherJS version change also
 // invalidates the cache. It is callers responsibility to ensure that artifacts
-// passed the Store function were generated with the same build
+// passed to the Store function were generated with the same build
 // parameters as the cache is configured.
 //
 // There is no upper limit for the total cache size. It can be cleared
@@ -117,9 +117,7 @@ type BuildCache struct {
 	// TestedPackage is the import path of the package being tested, or
 	// empty when not building for tests. The package under test is built
 	// with *_test.go sources included so we should always skip reading
-	// and writing cache in that case. Since we are caching prior to
-	// type-checking for generics, any package importing the package under
-	// test should be unaffected.
+	// and writing cache in that case.
 	TestedPackage string
 }
 
diff --git a/compiler/compiler_test.go b/compiler/compiler_test.go
@@ -1180,7 +1180,7 @@ func compileProject(t *testing.T, root *packages.Package, minify bool) map[strin
 
 	archives := map[string]*Archive{}
 	for _, srcs := range allSrcs {
-		a, err := Compile(srcs, tContext, minify)
+		a, err := Compile(srcs, nil, tContext, minify)
 		if err != nil {
 			t.Fatal(`failed to compile:`, err)
 		}
diff --git a/compiler/declCache.go b/compiler/declCache.go
@@ -1,39 +1,57 @@
 package compiler
 
 import (
-	"encoding/gob"
 	"fmt"
 	"sort"
 )
 
 type DeclCache struct {
-	enabled bool
 	decls   map[string]*Decl
 	changed bool
 }
 
-func init() {
-	// Register any types that are referenced by an interface so that
-	// the gob encoder/decoder can handle them.
-	gob.Register(&DeclCache{})
-}
-
-func NewDeclCache(enabled bool) *DeclCache {
-	return &DeclCache{enabled: enabled}
+// Chacnged reports whether the cache has had declarations added to it since
+// it was created or last read from storage or created empty.
+//
+// If there have been no changes, the cache does not need to be written back
+// to storage. Typically declarations will only be added when the cache is
+// empty and not loaded from storage since otherwise all the needed declarations
+// (other than those not being cached) would already be present.
+func (dc *DeclCache) Changed() bool {
+	return dc != nil && dc.changed
 }
 
 func (dc *DeclCache) GetDecl(fullname string) *Decl {
-	if dc == nil || !dc.enabled {
+	if dc == nil {
 		return nil // cache is disabled
 	}
 	return dc.decls[fullname]
 }
 
 func (dc *DeclCache) PutDecl(decl *Decl) {
-	if dc == nil || !dc.enabled {
+	if dc == nil {
 		return // cache is disabled
 	}
 
+	if decl.ForGeneric {
+		// Do not cache declarations for generic instantiations.
+		// The type arguments may come from a package depending on this one
+		// and not on one of this package's dependencies.
+		//
+		// If one of this package's dependencies changes, the cache will not be used.
+		// However, currently, changes to packages depending on this one
+		// may change and this package's cache may still be used.
+		// Therefore, if the package depending on this one changes a type from
+		// being blocking to non-blocking or vice versa, the cached declaration
+		// may be invalid.
+		return
+	}
+
+	if isUnqueDeclFullName(decl.FullName) {
+		// Only cache declarations with unique names.
+		return
+	}
+
 	if dc.decls == nil {
 		dc.decls = map[string]*Decl{}
 	}
@@ -47,12 +65,8 @@ func (dc *DeclCache) PutDecl(decl *Decl) {
 	dc.changed = true
 }
 
-func (dc *DeclCache) Changed() bool {
-	return dc != nil && dc.changed
-}
-
 func (dc *DeclCache) Read(decode func(any) error) error {
-	if dc == nil || !dc.enabled {
+	if dc == nil {
 		return nil // cache is disabled
 	}
 
@@ -75,7 +89,7 @@ func (dc *DeclCache) Read(decode func(any) error) error {
 }
 
 func (dc *DeclCache) Write(encode func(any) error) error {
-	if dc == nil || !dc.enabled {
+	if dc == nil {
 		return nil // cache is disabled
 	}
 
diff --git a/compiler/declNames.go b/compiler/declNames.go
@@ -5,11 +5,20 @@ import (
 	"go/token"
 	"go/types"
 	"path"
+	"strings"
 
 	"github.com/gopherjs/gopherjs/compiler/internal/symbol"
 	"github.com/gopherjs/gopherjs/compiler/internal/typeparams"
 )
 
+const (
+	varDeclNotUniqueFullName      = `var:blank`
+	funcVarDeclNotUniqueFullName  = `funcVar:init`
+	mainFuncDeclNotUniqueFullName = `init:main`
+	funcDeclNotUniqueFullName     = `func:init`
+	typeDeclNestedPrefix          = `type:nested:`
+)
+
 // importDeclFullName returns a unique name for an import declaration.
 // This import name may be duplicated in different packages if they both
 // import the same package, they are only unique per package.
@@ -22,7 +31,7 @@ func importDeclFullName(importedPkg *types.Package) string {
 //
 // If no variables are named, this will attempt to create a unique name
 // using the integer value of the first valid position of the blank variables.
-// Otherwise the name is `var:blank` and is not unique.
+// Otherwise the name is not unique (i.e. varDeclNotUniqueFullName).
 func varDeclFullName(init *types.Initializer, fSet *token.FileSet) string {
 	for _, lhs := range init.Lhs {
 		if lhs.Name() != `_` {
@@ -36,7 +45,7 @@ func varDeclFullName(init *types.Initializer, fSet *token.FileSet) string {
 		}
 	}
 
-	return `var:blank`
+	return varDeclNotUniqueFullName
 }
 
 // funcVarDeclFullName returns a name for a package-level variable
@@ -47,30 +56,36 @@ func varDeclFullName(init *types.Initializer, fSet *token.FileSet) string {
 // The name is unique unless the function is an `init` function.
 // If the name is for an `init` function, the position of the function
 // is used to create a unique name if it is valid,
-// otherwise the name is not unique.
+// otherwise the name is not unique (i.e. funcVarDeclNotUniqueFullName).
 func funcVarDeclFullName(o *types.Func, fSet *token.FileSet) string {
 	name := `funcVar:` + symbol.New(o).String()
 	if o.Name() == `init` {
 		name += declFullNameDiscriminator(o.Pos(), fSet)
 	}
+	// In the case of an `init` function with an invalid position,
+	// the name is not unique and will be equal to funcVarDeclNotUnqueName.
 	return name
 }
 
 // mainFuncDeclFullName returns the name for the declaration used to invoke the
 // main function of the program. There should only be one decl with this name.
+// This will always return mainFuncDeclNotUniqueFullName.
 func mainFuncDeclFullName() string {
-	return `init:main`
+	return mainFuncDeclNotUniqueFullName
 }
 
 // funcDeclFullName returns a name for a package-level function
 // declaration for the given instance of a function.
 // The name is unique unless the function is an `init` function
 // then it tries to use the position of the function to create a unique name.
+// Otherwise the name is not unique (i.e. funcDeclNotUniqueFullName).
 func funcDeclFullName(inst typeparams.Instance, fSet *token.FileSet) string {
 	name := `func:` + inst.String()
 	if inst.IsTrivial() && inst.Object.Name() == `init` {
 		name += declFullNameDiscriminator(inst.Object.Pos(), fSet)
 	}
+	// In the case of an `init` function with an invalid position,
+	// the name is not unique and will be equal to funcDeclNotUniqueFullName.
 	return name
 }
 
@@ -87,12 +102,14 @@ func typeVarDeclFullName(o *types.TypeName) string {
 // unless the type is a nested type then the name is only unique per the
 // function or method it is declared in. In that case, the position of the
 // declaration is used to try to create a unique name.
+// If the position is invalid, the nested name may be unique or not,
+// we cannot guarantee uniqueness in that case.
 func typeDeclFullName(inst typeparams.Instance, fSet *token.FileSet) string {
-	name := `type:` + inst.String()
 	if typeparams.FindNestingFunc(inst.Object) != nil {
-		name += declFullNameDiscriminator(inst.Object.Pos(), fSet)
+		return typeDeclNestedPrefix + inst.String() +
+			declFullNameDiscriminator(inst.Object.Pos(), fSet)
 	}
-	return name
+	return `type:` + inst.String()
 }
 
 // anonTypeDeclFullName returns a unique name for a package-level type
@@ -114,3 +131,16 @@ func declFullNameDiscriminator(pos token.Pos, fSet *token.FileSet) string {
 	p := fSet.Position(pos)
 	return fmt.Sprintf("@%s:%d:%d", path.Base(p.Filename), p.Line, p.Column)
 }
+
+// isUnqueDeclFullName reports whether the given declaration full name is unique.
+// A unique declaration name can be safely cached and reused.
+func isUnqueDeclFullName(name string) bool {
+	switch name {
+	case ``, varDeclNotUniqueFullName, funcVarDeclNotUniqueFullName,
+		mainFuncDeclNotUniqueFullName, funcDeclNotUniqueFullName:
+		return false // not unique since it equals one of the known non-unique names
+	}
+
+	// Check if the name is for a nested type declaration without a position discriminator.
+	return !strings.HasPrefix(name, typeDeclNestedPrefix) || strings.Contains(name, "@")
+}
diff --git a/compiler/package.go b/compiler/package.go
@@ -118,8 +118,7 @@ type funcContext struct {
 	funcLitCounter int
 }
 
-func newRootCtx(tContext *types.Context, srcs *sources.Sources, minify bool) *funcContext {
-	declCache, _ := srcs.DeclCache.(*DeclCache)
+func newRootCtx(tContext *types.Context, srcs *sources.Sources, declCache *DeclCache, minify bool) *funcContext {
 	funcCtx := &funcContext{
 		FuncInfo: srcs.TypeInfo.InitFuncInfo,
 		pkgCtx: &pkgContext{
@@ -158,7 +157,7 @@ type flowData struct {
 //
 // Provided sources must be prepared so that the type information has been determined,
 // and the source files have been sorted by name to ensure reproducible JavaScript output.
-func Compile(srcs *sources.Sources, tContext *types.Context, minify bool) (_ *Archive, err error) {
+func Compile(srcs *sources.Sources, declCache *DeclCache, tContext *types.Context, minify bool) (_ *Archive, err error) {
 	defer func() {
 		e := recover()
 		if e == nil {
@@ -174,7 +173,7 @@ func Compile(srcs *sources.Sources, tContext *types.Context, minify bool) (_ *Ar
 		err = bailout(fmt.Errorf("unexpected compiler panic while building package %q: %v", srcs.ImportPath, e))
 	}()
 
-	rootCtx := newRootCtx(tContext, srcs, minify)
+	rootCtx := newRootCtx(tContext, srcs, declCache, minify)
 
 	importedPaths, importDecls := rootCtx.importDecls()
 
diff --git a/compiler/sources/serializer.go b/compiler/sources/serializer.go
diff --git a/compiler/sources/serializer_test.go b/compiler/sources/serializer_test.go
diff --git a/compiler/sources/sources.go b/compiler/sources/sources.go

Original file line number	Diff line number	Diff line change
`@@ -1180,7 +1180,7 @@ func compileProject(t testing.T, root packages.Package, minify bool) map[strin`
`1180`	`1180`
`1181`	`1181`	`archives := map[string]*Archive{}`
`1182`	`1182`	`for _, srcs := range allSrcs {`
`1183`		`- a, err := Compile(srcs, tContext, minify)`
	`1183`	`+ a, err := Compile(srcs, nil, tContext, minify)`
`1184`	`1184`	`if err != nil {`
`1185`	`1185`	t.Fatal(`failed to compile:`, err)
`1186`	`1186`	`}`