diff --git a/CHANGELOG.md b/CHANGELOG.md index 71db9ec..6dff7c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,20 @@ because it turns other people's test suites red. in front of the picture, so it has to know how large the picture is before it starts. +- **A recipe is parsed once instead of twice.** Reading a recipe checked it + for a stray second document and then handed the whole file to the decoder, + which parsed it again from scratch. The decoder now works from what was + already parsed. + + Measured on the largest recipe the size limit allows, 900 kB and 20 000 + targets: `validate` went from 839 ms to 754 ms. An ordinary recipe of a few + kilobytes was already instant. Where it shows is the batch screen, which + re-reads the recipe on every keystroke. + + **Every refusal says exactly what it said before** - 48 malformed recipes + through two commands, compared character for character including the exit + code. + - **A password protected archive allocates once per entry instead of once per block written.** Producing a 128 MB locked `.zip` used to make the collector run 48 times. It runs 6. The files are identical and the wall clock barely diff --git a/internal/recipe/recipe.go b/internal/recipe/recipe.go index c2ade4a..f548b9c 100644 --- a/internal/recipe/recipe.go +++ b/internal/recipe/recipe.go @@ -8,6 +8,7 @@ import ( "unicode/utf8" "github.com/goccy/go-yaml" + "github.com/goccy/go-yaml/ast" ) // SchemaVersion is the recipe schema this build understands. It is versioned @@ -194,14 +195,15 @@ func Parse(src []byte, name string) (*Recipe, error) { // One file is one recipe. Everything after a document separator would be // dropped by the decoder, which means somebody gets half the fixtures they // asked for and a run that says it went fine. - if _, err := oneDocument(src, name); err != nil { + doc, err := oneDocument(src, name) + if err != nil { return nil, err } // Strict decoding turns an unknown key into an error. A typo in // "siez: 10mb" accepted in silence gives a file of the default size and an // hour spent wondering why the test passes when it should not. - if err := decodeStrict(src, &raw); err != nil { + if err := decodeStrict(doc, &raw); err != nil { return nil, &SyntaxError{Name: name, Detail: strings.TrimRight(err.Error(), "\n")} } @@ -226,7 +228,7 @@ func Parse(src []byte, name string) (*Recipe, error) { // Scoped to this one call rather than to the whole of Parse. A crash in our own // validation should still arrive as a crash, not be quietly relabelled as a // problem with the user's file. -func decodeStrict(src []byte, raw *rawRecipe) (err error) { +func decodeStrict(f *ast.File, raw *rawRecipe) (err error) { defer func() { if r := recover(); r != nil { // The panic value itself says "invalid memory address", which tells @@ -235,7 +237,41 @@ func decodeStrict(src []byte, raw *rawRecipe) (err error) { err = fmt.Errorf("this file could not be read as YAML. Look for a tag or anchor marker such as ! or & with nothing after it") } }() - return yaml.UnmarshalWithOptions(src, raw, yaml.Strict()) + // Decoded from the document the one-document check already parsed, rather + // than from the bytes again. Handing the decoder the source makes it parse + // the whole file a second time, and this file is read once per run and once + // per keystroke on the batch screen. + // + // Measured 2026-09-06 on the largest recipe the size limit allows, 900 kB + // and 20 000 targets: the second parse is 107 ms of an 841 ms validate, + // ranges disjoint. + // + // The document count still comes from the parsed tree rather than from + // counting separators in the token stream, and that is deliberate: a + // comment before a leading "---" is a document with no body, and counting + // raw separators refused files that are ordinary YAML house style. That was + // a real defect once and TestOneRecipeIsAcceptedWhateverSeparatorsSurroundIt + // exists because of it. + body := recipeBody(f) + if body == nil { + // Nothing but comments or separators. The validator below says what is + // missing, in its own words, rather than the decoder complaining about + // an empty document. + return nil + } + return yaml.NodeToValue(body, raw, yaml.Strict()) +} + +// recipeBody is the document that holds the recipe, or nil when the file holds +// no document with a body. It picks the same document recipesIn counts. +func recipeBody(f *ast.File) ast.Node { + for _, d := range f.Docs { + if d.Body == nil || d.Body.Type() == ast.CommentType { + continue + } + return d.Body + } + return nil } // rawRecipe carries every key the recipe document describes, including the