cue/load: move syntax cache into fileSystem

In order for the CUE syntax to be shared between cue/load and
internal/mod/modpkgload, it needs to be available in the `io.FS`
implementation passed to `modpkgload`, and thus it can't be inside the
`loader` because that gets created too late, after `modpkgload` has
already been invoked.

So we move the syntax cache into `fileSystem` so it will be easily
available to the `io.FS` implementation.

Also be a little more careful about caching: we make sure that we only
cache file syntax when the parsing is absolutely vanilla, which might be
slightly less efficient, but is very unlikely to make any significant
difference as non-vanilla loading only happens for files explicitly
mentioned on the command line, and those are usually not shared with
files used elsewhere in the build.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I282557ffde6af410a237f0a13f91c7c87abf1cef
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1197529
Reviewed-by: Paul Jolly <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: rogpeppe

cue/load/fs.go

@@ -26,12 +26,13 @@ import (
 	"strings"
 	"time"
 
+	"cuelang.org/go/cue"
 	"cuelang.org/go/cue/ast"
 	"cuelang.org/go/cue/build"
+	"cuelang.org/go/cue/cuecontext"
 	"cuelang.org/go/cue/errors"
-	"cuelang.org/go/cue/parser"
 	"cuelang.org/go/cue/token"
-	"cuelang.org/go/internal/cueimports"
+	"cuelang.org/go/internal/encoding"
 	"cuelang.org/go/mod/module"
 )
 
@@ -59,6 +60,7 @@ func (f *overlayFile) Sys() interface{}   { return nil }
 type fileSystem struct {
 	overlayDirs map[string]map[string]*overlayFile
 	cwd         string
+	fileCache   *fileCache
 }
 
 func (fs *fileSystem) getDir(dir string, create bool) map[string]*overlayFile {
@@ -136,6 +138,7 @@ func newFileSystem(cfg *Config) (*fileSystem, error) {
 			}
 		}
 	}
+	fs.fileCache = newFileCache(cfg)
 	return fs, nil
 }
 
@@ -184,58 +187,6 @@ func (fs *fileSystem) getOverlay(path string) *overlayFile {
 	return nil
 }
 
-func (fs *fileSystem) getCUESyntax(fi *build.File) (*ast.File, error) {
-	switch src := fi.Source.(type) {
-	case []byte, string:
-		return parser.ParseFile(fi.Filename, src, parser.ImportsOnly)
-	case *ast.File:
-		return src, nil
-	case nil:
-		if fi.Filename == "-" {
-			panic("source unexpectedly not provided for stdin")
-		}
-		return fs.readCUE(fi.Filename, true)
-	}
-	return nil, errors.Newf(token.NoPos, "unsupported source type %T", fi.Source)
-}
-
-func (fs *fileSystem) readCUE(path string, importsOnly bool) (*ast.File, error) {
-	var parseMode parser.Option
-	if importsOnly {
-		parseMode = parser.ImportsOnly
-	}
-	var data []byte
-	if f := fs.getOverlay(path); f != nil {
-		if f.file != nil {
-			return f.file, nil
-		}
-		if f.isDir {
-			return nil, fmt.Errorf("%q is a directory", path)
-		}
-		data = f.contents
-	} else {
-		var err error
-		f, err := fs.openFile(path)
-		if err != nil {
-			return nil, err
-		}
-		defer f.Close()
-		if importsOnly {
-			data, err = cueimports.Read(f)
-		} else {
-			data, err = io.ReadAll(f)
-		}
-		if err != nil {
-			return nil, fmt.Errorf("read %s: %v", path, err)
-		}
-	}
-	pf, err := parser.ParseFile(path, data, parseMode)
-	if err != nil {
-		return nil, err
-	}
-	return pf, nil
-}
-
 func (fs *fileSystem) stat(path string) (iofs.FileInfo, errors.Error) {
 	path = fs.makeAbs(path)
 	if fi := fs.getOverlay(path); fi != nil {
@@ -441,3 +392,54 @@ func (f *ioFSFile) ReadDir(n int) ([]iofs.DirEntry, error) {
 	f.entries = f.entries[n:]
 	return entries, err
 }
+
+func (fs *fileSystem) getCUESyntax(bf *build.File) (*ast.File, error) {
+	if bf.Encoding != build.CUE {
+		panic("getCUESyntax called with non-CUE file encoding")
+	}
+	// When it's a regular CUE file with no funny stuff going on, we
+	// check and update the syntax cache.
+	useCache := bf.Form == build.Schema && bf.Interpretation == ""
+	if useCache {
+		if syntax, ok := fs.fileCache.entries[bf.Filename]; ok {
+			return syntax.file, syntax.err
+		}
+	}
+	d := encoding.NewDecoder(fs.fileCache.ctx, bf, &fs.fileCache.config)
+	defer d.Close()
+	// Note: CUE files can never have multiple file parts.
+	f, err := d.File(), d.Err()
+	if useCache {
+		fs.fileCache.entries[bf.Filename] = fileCacheEntry{f, err}
+	}
+	return f, err
+}
+
+func newFileCache(c *Config) *fileCache {
+	return &fileCache{
+		config: encoding.Config{
+			// Note: no need to pass Stdin, as we take care
+			// always to pass a non-nil source when the file is "-".
+			ParseFile: c.ParseFile,
+		},
+		ctx:     cuecontext.New(),
+		entries: make(map[string]fileCacheEntry),
+	}
+}
+
+// fileCache caches data derived from the file system.
+type fileCache struct {
+	config  encoding.Config
+	ctx     *cue.Context
+	entries map[string]fileCacheEntry
+}
+
+type fileCacheEntry struct {
+	// TODO cache directory information too.
+
+	// file caches the work involved when decoding a file into an *ast.File.
+	// This can happen multiple times for the same file, for example when it is present in
+	// multiple different build instances in the same directory hierarchy.
+	file *ast.File
+	err  error
+}

cue/load/instances.go

@@ -96,7 +96,6 @@ func Instances(args []string, c *Config) []*build.Instance {
 		pkgArgs = pkgArgs1
 	}
 
-	synCache := newSyntaxCache(c)
 	tg := newTagger(c)
 	// Pass all arguments that look like packages to loadPackages
 	// so that they'll be available when looking up the packages
@@ -105,11 +104,11 @@ func Instances(args []string, c *Config) []*build.Instance {
 	if err != nil {
 		return []*build.Instance{c.newErrInstance(err)}
 	}
-	pkgs, err := loadPackages(ctx, c, synCache, expandedPaths, otherFiles, tg)
+	pkgs, err := loadPackages(ctx, c, expandedPaths, otherFiles, tg)
 	if err != nil {
 		return []*build.Instance{c.newErrInstance(err)}
 	}
-	l := newLoader(c, tg, synCache, pkgs)
+	l := newLoader(c, tg, pkgs)
 
 	if c.Context == nil {
 		c.Context = build.NewContext(
@@ -175,7 +174,6 @@ func Instances(args []string, c *Config) []*build.Instance {
 func loadPackages(
 	ctx context.Context,
 	cfg *Config,
-	synCache *syntaxCache,
 	pkgs []resolvedPackageArg,
 	otherFiles []*build.File,
 	tg *tagger,
@@ -205,7 +203,7 @@ func loadPackages(
 			// not a CUE file; assume it has no imports for now.
 			continue
 		}
-		syntax, err := synCache.getSyntax(f)
+		syntax, err := cfg.fileSystem.getCUESyntax(f)
 		if err != nil {
 			return nil, fmt.Errorf("cannot get syntax for %q: %v", f.Filename, err)
 		}

cue/load/loader.go

@@ -22,13 +22,9 @@ package load
 import (
 	"path/filepath"
 
-	"cuelang.org/go/cue"
-	"cuelang.org/go/cue/ast"
 	"cuelang.org/go/cue/build"
-	"cuelang.org/go/cue/cuecontext"
 	"cuelang.org/go/cue/errors"
 	"cuelang.org/go/cue/token"
-	"cuelang.org/go/internal/encoding"
 	"cuelang.org/go/internal/mod/modpkgload"
 
 	// Trigger the unconditional loading of all core builtin packages if load
@@ -44,11 +40,6 @@ type loader struct {
 	stk    importStack
 	pkgs   *modpkgload.Packages
 
-	// syntaxCache caches the work involved when decoding a file into an *ast.File.
-	// This can happen multiple times for the same file, for example when it is present in
-	// multiple different build instances in the same directory hierarchy.
-	syntaxCache *syntaxCache
-
 	// dirCachedBuildFiles caches the work involved when reading a
 	// directory. It is keyed by directory name. When we descend into
 	// subdirectories to load patterns such as ./... we often end up
@@ -62,13 +53,12 @@ type cachedDirFiles struct {
 	filenames []string
 }
 
-func newLoader(c *Config, tg *tagger, syntaxCache *syntaxCache, pkgs *modpkgload.Packages) *loader {
+func newLoader(c *Config, tg *tagger, pkgs *modpkgload.Packages) *loader {
 	return &loader{
 		cfg:                 c,
 		tagger:              tg,
 		pkgs:                pkgs,
 		dirCachedBuildFiles: make(map[string]cachedDirFiles),
-		syntaxCache:         syntaxCache,
 	}
 }
 
@@ -148,51 +138,10 @@ func (l *loader) cueFilesPackage(files []*build.File) *build.Instance {
 // addFiles populates p.Files by reading CUE syntax from p.BuildFiles.
 func (l *loader) addFiles(p *build.Instance) {
 	for _, bf := range p.BuildFiles {
-		f, err := l.syntaxCache.getSyntax(bf)
+		f, err := l.cfg.fileSystem.getCUESyntax(bf)
 		if err != nil {
 			p.ReportError(errors.Promote(err, "load"))
 		}
 		_ = p.AddSyntax(f)
 	}
 }
-
-type syntaxCache struct {
-	config encoding.Config
-	ctx    *cue.Context
-	cache  map[string]syntaxCacheEntry
-}
-
-type syntaxCacheEntry struct {
-	err  error
-	file *ast.File
-}
-
-func newSyntaxCache(cfg *Config) *syntaxCache {
-	return &syntaxCache{
-		config: encoding.Config{
-			// Note: no need to pass Stdin, as we take care
-			// always to pass a non-nil source when the file is "-".
-			ParseFile: cfg.ParseFile,
-		},
-		ctx:   cuecontext.New(),
-		cache: make(map[string]syntaxCacheEntry),
-	}
-}
-
-// getSyntax returns the CUE syntax corresponding to the file argument f.
-func (c *syntaxCache) getSyntax(bf *build.File) (*ast.File, error) {
-	syntax, ok := c.cache[bf.Filename]
-	if ok {
-		return syntax.file, syntax.err
-	}
-	if bf.Encoding != build.CUE {
-		panic("syntax for non-CUE file")
-	}
-	d := encoding.NewDecoder(c.ctx, bf, &c.config)
-	defer d.Close()
-	// Note: CUE files can never have multiple file parts.
-	syntax.file = d.File()
-	syntax.err = d.Err()
-	c.cache[bf.Filename] = syntax
-	return syntax.file, syntax.err
-}