Skip to content

Commit fed43b0

Browse files
committed
internal/filetypes: speed up common case
Creating a `*build.File` value is somewhat expensive in the general case, but the general case is fairly unusual: it's much more common to have files interpreted according to their extension only with defaults for all other encoding attributes. Make this operation more efficient (no CUE value lookup required) for this common case by caching the resulting `*build.File` values for each of the known file extensions at init time. While we're about it, change the existing "common case" logic to use a check that matches the actual invariants of the common case for CUE: specifically the default encoding for CUE uses `form: ""` not `form: "schema"`: this makes a difference in subsequent CLs in the checks for whether a given parsed CUE file can be cached. Signed-off-by: Roger Peppe <[email protected]> Change-Id: I46f8c823cca08a44a09dece8d5e9a3df55bcf928 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1197557 Reviewed-by: Paul Jolly <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Unity-Result: CUE porcuepine <[email protected]>
1 parent 27adbac commit fed43b0

File tree

3 files changed

+40
-24
lines changed

3 files changed

+40
-24
lines changed

internal/filetypes/filetypes.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func FromFile(b *build.File, mode Mode) (*FileInfo, error) {
7878
// isolation without interference from evaluating these files.
7979
if mode == Input &&
8080
b.Encoding == build.CUE &&
81-
b.Form == build.Schema &&
81+
b.Form == "" &&
8282
b.Interpretation == "" {
8383
return &FileInfo{
8484
File: b,
@@ -175,11 +175,9 @@ func ParseArgs(args []string) (files []*build.File, err error) {
175175
if !fileVal.Exists() {
176176
if len(a) == 1 && strings.HasSuffix(a[0], ".cue") {
177177
// Handle majority case.
178-
files = append(files, &build.File{
179-
Filename: a[0],
180-
Encoding: build.CUE,
181-
Form: build.Schema,
182-
})
178+
f := *fileForCUE
179+
f.Filename = a[0]
180+
files = append(files, &f)
183181
hasFiles = true
184182
continue
185183
}
@@ -269,15 +267,20 @@ func ParseFileAndType(file, scope string, mode Mode) (*build.File, error) {
269267
// Quickly discard files which we aren't interested in.
270268
// These cases are very common when loading `./...` in a large repository.
271269
typesInit()
272-
if scope == "" {
270+
if scope == "" && file != "-" {
273271
ext := fileExt(file)
274-
if file == "-" {
275-
// not handled here
276-
} else if ext == "" {
272+
if ext == "" {
277273
return nil, errors.Newf(token.NoPos, "no encoding specified for file %q", file)
278-
} else if !knownExtensions[ext] {
274+
}
275+
f, ok := fileForExt[ext]
276+
if !ok {
279277
return nil, errors.Newf(token.NoPos, "unknown file extension %s", ext)
280278
}
279+
if mode == Input {
280+
f1 := *f
281+
f1.Filename = file
282+
return &f1, nil
283+
}
281284
}
282285
modeVal, fileVal, err := parseType(scope, mode)
283286
if err != nil {

internal/filetypes/types.cue

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
55
// You may obtain a copy of the License at
6-
//
6+
//
77
// http://www.apache.org/licenses/LICENSE-2.0
88
//
99
// Unless required by applicable law or agreed to in writing, software
@@ -56,15 +56,15 @@ package build
5656
attributes?: bool // include/allow attributes
5757
}
5858

59-
// knownExtensions derives all the known file extensions
60-
// from those that are mentioned in modes,
61-
// allowing us to quickly discard files with unknown extensions.
62-
knownExtensions: {
63-
for mode in modes
64-
for ext, _ in mode.extensions {
65-
(ext): true
66-
}
67-
}
59+
// fileForExtVanilla holds the extensions supported in
60+
// input mode with scope="" - the most common form
61+
// of file type to evaluate.
62+
//
63+
// It's also used as a source of truth for all known file
64+
// extensions as all modes define attributes for
65+
// all file extensions. If that ever changed, we'd need
66+
// to change this.
67+
fileForExtVanilla: modes.input.extensions
6868

6969
// modes sets defaults for different operational modes.
7070
modes: [string]: {

internal/filetypes/types.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,38 @@ package filetypes
1616

1717
import (
1818
_ "embed"
19+
"fmt"
1920
"sync"
2021

2122
"cuelang.org/go/cue"
23+
"cuelang.org/go/cue/build"
2224
"cuelang.org/go/cue/cuecontext"
2325
)
2426

2527
//go:embed types.cue
2628
var typesCUE string
2729

28-
var typesValue cue.Value
29-
var knownExtensions map[string]bool
30+
var (
31+
typesValue cue.Value
32+
fileForExt map[string]*build.File
33+
fileForCUE *build.File
34+
)
3035

3136
var typesInit = sync.OnceFunc(func() {
3237
ctx := cuecontext.New()
3338
typesValue = ctx.CompileString(typesCUE, cue.Filename("types.cue"))
3439
if err := typesValue.Err(); err != nil {
3540
panic(err)
3641
}
37-
if err := typesValue.LookupPath(cue.MakePath(cue.Str("knownExtensions"))).Decode(&knownExtensions); err != nil {
42+
// Reading a file in input mode with a non-explicit scope is a very
43+
// common operation, so cache the build.File value for all
44+
// the known file extensions.
45+
if err := typesValue.LookupPath(cue.MakePath(cue.Str("fileForExtVanilla"))).Decode(&fileForExt); err != nil {
3846
panic(err)
3947
}
48+
fileForCUE = fileForExt[".cue"]
49+
// Check invariants assumed by FromFile
50+
if fileForCUE.Form != "" || fileForCUE.Interpretation != "" || fileForCUE.Encoding != build.CUE {
51+
panic(fmt.Errorf("unexpected value for CUE file type: %#v", fileForCUE))
52+
}
4053
})

0 commit comments

Comments
 (0)