From 11b5f96e11d2065ae719b8b897f2a4044699ec07 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 6 Sep 2026 05:07:46 +0200 Subject: [PATCH] perf: a recipe is parsed once instead of twice Reading a recipe walked the document three times: the lexer for flow depth, a full parse to check for a stray second document, and then the decoder, which parsed the whole file again from scratch. The decoder now works from the tree the document check already built. NOT the fix the report asked for. That one removed the parse and counted documents from the token stream, which would have reintroduced a defect this project already fixed: recipesIn counts documents with a BODY, and a comment before a leading "---" is a document without one. Counting raw separators refused files written in ordinary YAML house style, which is why TestOneRecipeIsAcceptedWhateverSeparatorsSurroundIt exists. The parse stays and the decoder's second one goes instead, so the counting semantics are untouched. Measured on the largest recipe the size limit allows, 900 kB and 20 000 targets, nine repetitions interleaved: validate 839 -> 754 ms, ranges disjoint. The report said 36% of the read; the discarded parse measured 13% and the change delivers 10%. On a few-kilobyte recipe none of this is visible - where it shows is the batch screen, which re-reads on every keystroke. A green suite is not evidence here, because this moves the WORDS of a refusal rather than the bytes of a file, and no byte guard can see that. The owner's condition was the refusal corpus before and after: tools/probes/refusalcorpus.py drives 48 malformed and awkward recipes through validate and generate --dry-run and compares stdout, stderr and the exit code. 96 of 96 identical. Five fuzz targets, 20s each, clean. Two mutation entries were stale afterwards and one of them was mine: the existing strictness entry quoted the call that was replaced, and the one I added beside it duplicated it while naming a guard about declared keys rather than about typos. It came back NOT CAUGHT, which was a statement about my entry and not about the code. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 14 +++++++++++++ internal/recipe/recipe.go | 44 +++++++++++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 4 deletions(-) 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