Scan YAML framing preambles once

2026-10-09 01:30:17 +00:00
parent ae656d8fa9
commit 3f8841f298
4 changed files with 145 additions and 64 deletions
--- a/docs/roadmap/implementation.md
+++ b/docs/roadmap/implementation.md
@@ -1,6 +1,6 @@
 # Performance implementation plan

-Status: implementation in progress; Stages 1–4 are complete, Stages 5–11 have not started.
+Status: implementation in progress; Stages 1–5 are complete, Stages 6–11 have not started.

 Date: 2026-10-09.

@@ -429,7 +429,7 @@ tree construction remains in each parse.

 ## Stage 5: scan YAML preambles without splitting documents

-**Status:** not started. **Prerequisite:** Stage 4.
+**Status:** complete. **Prerequisite:** Stage 4.

 **Files:** [show.go](../../show.go), [show_test.go](../../show_test.go),
 [configkit_test.go](../../configkit_test.go), framing benchmarks, and evidence.
@@ -464,7 +464,33 @@ tree construction remains in each parse.
 **Completion:** each document has one opening analysis, no document-wide line
 index, and the selected complete-buffer delivery model.

-**Evidence:** pending.
+**Evidence:** `GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none
+GOSUMDB=off go test -mod=readonly -count=1 .` passed. Existing aggregate tests
+continue to cover acquisition-before-output, directive document separators,
+decoded values, diagnostics, and writer failures. New framing cases cover a
+missing final newline, CRLF, a directive preamble, a control character in the
+scope label, unchanged native bytes, decoded values, and one-document output.
+
+Measurements used Go 1.26.8, linux/amd64, Linux 6.17.0-41-generic, Intel Core
+i5-7260U CPU @ 2.20 GHz, based on source revision `ae656d8` with this stage's
+changes in the working tree. The five-sample command passed:
+`GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -run '^$' -bench '^BenchmarkPerformanceYAMLFraming$' -benchmem -benchtime=200ms -count=5 .`.
+Raw output is in `/tmp/commandkit-performance-yaml-after-stage5.txt`.
+
+Direct 4 KiB ordinary framing measured 1,135–1,455 ns/op, 4,864 B/op, and one
+allocation, compared with 5,883–6,842 ns/op, 11,056 B/op, and three allocations
+at baseline. Direct 1 MiB directive framing measured 189,303–359,655 ns/op,
+1,056,768–1,056,769 B/op, and one allocation, compared with 1,208,678–1,502,417
+ns/op, 2,539,520–2,539,523 B/op, and two allocations. Aggregate 4 KiB show
+measured 29,951–49,108 ns/op, 36,999–37,001 B/op, and 206 allocations, compared
+with 44,395–48,161 ns/op, 55,537–55,538 B/op, and 211 allocations. Aggregate
+1 MiB show measured 1,326,104–1,557,880 ns/op, 4,246,730–4,246,767 B/op, and
+211 allocations, compared with 4,818,907–6,009,877 ns/op, 8,695,182–8,695,197
+B/op, and 215–216 allocations. Single-scope 4 KiB display remained at
+17,935–17,936 B/op and 183 allocations; timing variation overlaps the baseline.
+The complete framed output still requires a full-size copy under OD-2. The
+allocation reduction comes from eliminating document-wide split/index storage
+and reusing the opening analysis for framing and aggregate directive handling.

 ## Stage 6: install validated registrations without copying the registry

--- a/performance_reports_test.go
+++ b/performance_reports_test.go
@@ -72,14 +72,14 @@ func BenchmarkPerformanceYAMLFraming(b *testing.B) {
 	}
 	for _, fixture := range fixtures {
 		b.Run("framing/"+fixture.name, func(b *testing.B) {
-			framed := frameEffectiveYAML(fixture.scope, fixture.native)
-			if len(framed) <= len(fixture.native) || hasYAMLDirective(fixture.native) != strings.Contains(fixture.name, "directive") {
+			framed, hasDirective := frameEffectiveYAML(fixture.scope, fixture.native)
+			if len(framed) <= len(fixture.native) || hasDirective != strings.Contains(fixture.name, "directive") {
 				b.Fatalf("framing fixture result has unexpected size or directive classification: native=%d framed=%d", len(fixture.native), len(framed))
 			}
 			b.ReportAllocs()
 			b.ResetTimer()
 			for i := 0; i < b.N; i++ {
-				performanceFramedBytes = frameEffectiveYAML(fixture.scope, fixture.native)
+				performanceFramedBytes, _ = frameEffectiveYAML(fixture.scope, fixture.native)
 			}
 		})
 		b.Run("aggregate/"+fixture.name, func(b *testing.B) {
--- a/show.go
+++ b/show.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
-	"strings"
 	"unicode"
 )

@@ -99,14 +98,14 @@ func runConfigShow[C any](invocation Invocation, suite ConfigSuite[C], input con
 		if err := invocation.Context.Err(); err != nil {
 			return showFailure(err, diagnostics, written)
 		}
-		if index > 0 && hasYAMLDirective(native.Bytes()) {
+		framed, hasDirective := frameEffectiveYAML(document.name, native.Bytes())
+		if index > 0 && hasDirective {
 			if err := writeReport(invocation.Streams.Out, []byte("...\n")); err != nil {
 				outcome := showFailure(err, diagnostics, true)
 				outcome.DeliveryError = err
 				return outcome
 			}
 		}
-		framed := frameEffectiveYAML(document.name, native.Bytes())
 		if index < len(documents)-1 && len(framed) > 0 && framed[len(framed)-1] != '\n' {
 			framed = append(framed, '\n')
 		}
@@ -122,24 +121,50 @@ func runConfigShow[C any](invocation Invocation, suite ConfigSuite[C], input con
 	return outcome
 }

-func hasYAMLDirective(native []byte) bool {
-	lines := bytes.Split(native, []byte("\n"))
-	for _, line := range lines {
-		trimmed := bytes.TrimSpace(line)
-		if isYAMLDocumentStart(string(trimmed)) {
-			return false
-		}
-		if bytes.HasPrefix(trimmed, []byte("%YAML ")) || bytes.HasPrefix(trimmed, []byte("%TAG ")) {
-			return true
-		}
-		if len(trimmed) > 0 && trimmed[0] != '#' {
-			return false
-		}
-	}
-	return false
+type yamlPreamble struct {
+	markerStart  int
+	markerEnd    int
+	markerFound  bool
+	inlineMarker bool
+	preambleEnd  int
+	hasDirective bool
 }

-func frameEffectiveYAML(scope string, native []byte) []byte {
+func analyzeYAMLPreamble(native []byte) yamlPreamble {
+	var analysis yamlPreamble
+	for start := 0; start < len(native); {
+		end := len(native)
+		if newline := bytes.IndexByte(native[start:], '\n'); newline >= 0 {
+			end = start + newline + 1
+		}
+		lineEnd := end
+		if lineEnd > start && native[lineEnd-1] == '\n' {
+			lineEnd--
+		}
+		trimmed := bytes.TrimSpace(native[start:lineEnd])
+		if isYAMLDocumentStart(trimmed) {
+			content := bytes.TrimSpace(trimmed[3:])
+			analysis.markerStart = start
+			analysis.markerEnd = end
+			analysis.markerFound = true
+			analysis.inlineMarker = len(content) > 0 && content[0] != '#'
+			return analysis
+		}
+		if bytes.HasPrefix(trimmed, []byte("%YAML ")) || bytes.HasPrefix(trimmed, []byte("%TAG ")) {
+			analysis.hasDirective = true
+		}
+		if len(trimmed) == 0 || trimmed[0] == '#' || bytes.HasPrefix(trimmed, []byte("%YAML ")) || bytes.HasPrefix(trimmed, []byte("%TAG ")) {
+			analysis.preambleEnd = end
+		} else {
+			return analysis
+		}
+		start = end
+	}
+	return analysis
+}
+
+func frameEffectiveYAML(scope string, native []byte) ([]byte, bool) {
+	analysis := analyzeYAMLPreamble(native)
 	label := scope
 	for _, r := range scope {
 		if unicode.IsControl(r) {
@@ -148,53 +173,36 @@ func frameEffectiveYAML(scope string, native []byte) []byte {
 		}
 	}
 	comment := []byte("# scope: " + label + "\n")
-	lines := bytes.SplitAfter(native, []byte("\n"))
-	position := 0
-	preambleEnd := 0
-	hasDirective := false
-	for _, line := range lines {
-		trimmed := strings.TrimSpace(string(line))
-		if isYAMLDocumentStart(trimmed) {
-			out := make([]byte, 0, len(native)+len(comment))
-			// Inline content may be a block scalar; place the label before
-			// its marker rather than interrupting the native value.
-			content := strings.TrimSpace(strings.TrimPrefix(trimmed, "---"))
-			if content != "" && !strings.HasPrefix(content, "#") {
-				out = append(out, native[:position]...)
-				out = append(out, comment...)
-				out = append(out, native[position:]...)
-				return out
-			}
-			out = append(out, native[:position+len(line)]...)
-			if len(line) > 0 && line[len(line)-1] != '\n' {
-				out = append(out, '\n')
-			}
+	if analysis.markerFound {
+		out := make([]byte, 0, len(native)+len(comment))
+		// Inline content may be a block scalar; place the label before
+		// its marker rather than interrupting the native value.
+		if analysis.inlineMarker {
+			out = append(out, native[:analysis.markerStart]...)
 			out = append(out, comment...)
-			out = append(out, native[position+len(line):]...)
-			return out
+			out = append(out, native[analysis.markerStart:]...)
+			return out, analysis.hasDirective
 		}
-		if strings.HasPrefix(trimmed, "%YAML ") || strings.HasPrefix(trimmed, "%TAG ") {
-			hasDirective = true
+		out = append(out, native[:analysis.markerEnd]...)
+		if analysis.markerEnd > analysis.markerStart && native[analysis.markerEnd-1] != '\n' {
+			out = append(out, '\n')
 		}
-		if trimmed == "" || strings.HasPrefix(trimmed, "#") || strings.HasPrefix(trimmed, "%YAML ") || strings.HasPrefix(trimmed, "%TAG ") {
-			preambleEnd = position + len(line)
-		} else {
-			break
-		}
-		position += len(line)
+		out = append(out, comment...)
+		out = append(out, native[analysis.markerEnd:]...)
+		return out, analysis.hasDirective
 	}
 	result := make([]byte, 0, len(native)+len(comment)+5)
-	if hasDirective {
-		result = append(result, native[:preambleEnd]...)
+	if analysis.hasDirective {
+		result = append(result, native[:analysis.preambleEnd]...)
 	}
 	result = append(result, []byte("---\n")...)
 	result = append(result, comment...)
-	if hasDirective {
-		result = append(result, native[preambleEnd:]...)
+	if analysis.hasDirective {
+		result = append(result, native[analysis.preambleEnd:]...)
 	} else {
 		result = append(result, native...)
 	}
-	return result
+	return result, analysis.hasDirective
 }

 func showFailure(cause error, diagnostics []Diagnostic, reportStarted bool) Outcome {
@@ -204,6 +212,6 @@ func showFailure(cause error, diagnostics []Diagnostic, reportStarted bool) Outc
 	return outcome
 }

-func isYAMLDocumentStart(line string) bool {
-	return strings.HasPrefix(line, "---") && (len(line) == 3 || line[3] == ' ' || line[3] == '\t')
+func isYAMLDocumentStart(line []byte) bool {
+	return bytes.HasPrefix(line, []byte("---")) && (len(line) == 3 || line[3] == ' ' || line[3] == '\t')
 }
--- a/show_test.go
+++ b/show_test.go
@@ -20,7 +20,7 @@ func TestEffectiveYAMLFramingPreservesOneNativeDocument(t *testing.T) {
 			if err := yaml.Unmarshal([]byte(native), &want); err != nil {
 				t.Fatal(err)
 			}
-			framed := frameEffectiveYAML("app", []byte(native))
+			framed, _ := frameEffectiveYAML("app", []byte(native))
 			withoutLabel := bytes.Replace(framed, []byte("# scope: app\n"), nil, 1)
 			if !strings.HasPrefix(native, "---") && !strings.HasPrefix(native, "%") {
 				withoutLabel = bytes.TrimPrefix(withoutLabel, []byte("---\n"))
@@ -45,3 +45,50 @@ func TestEffectiveYAMLFramingPreservesOneNativeDocument(t *testing.T) {
 		})
 	}
 }
+
+func TestEffectiveYAMLFramingPreservesBoundariesAndDirectives(t *testing.T) {
+	for _, test := range []struct {
+		name      string
+		scope     string
+		native    string
+		directive bool
+		label     string
+	}{
+		{name: "no trailing newline", scope: "app", native: "value: plain", label: "# scope: app\n"},
+		{name: "CRLF", scope: "app", native: "# authored\r\n---\r\nvalue: yes\r\n", label: "# scope: app\n"},
+		{name: "directive preamble", scope: "app", native: "%YAML 1.1\n# authored\n---\nvalue: yes", directive: true, label: "# scope: app\n"},
+		{name: "control character label", scope: "line\nbreak", native: "value: yes", label: "# scope: \"line\\nbreak\"\n"},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			var want any
+			if err := yaml.Unmarshal([]byte(test.native), &want); err != nil {
+				t.Fatal(err)
+			}
+			framed, hasDirective := frameEffectiveYAML(test.scope, []byte(test.native))
+			if hasDirective != test.directive {
+				t.Fatalf("hasDirective = %t, want %t", hasDirective, test.directive)
+			}
+			withoutLabel := bytes.Replace(framed, []byte(test.label), nil, 1)
+			if !strings.HasPrefix(test.native, "---") && !strings.HasPrefix(test.native, "%") {
+				withoutLabel = bytes.TrimPrefix(withoutLabel, []byte("---\n"))
+			}
+			if !bytes.Equal(withoutLabel, []byte(test.native)) {
+				t.Fatalf("native content changed: got %q want %q", withoutLabel, test.native)
+			}
+			if !bytes.Contains(framed, []byte(test.label)) {
+				t.Fatalf("missing scope label %q in %q", test.label, framed)
+			}
+			decoder := yaml.NewDecoder(bytes.NewReader(framed))
+			var first any
+			if err := decoder.Decode(&first); err != nil {
+				t.Fatal(err)
+			}
+			if !reflect.DeepEqual(first, want) {
+				t.Fatalf("decoded value = %#v, want %#v", first, want)
+			}
+			if err := decoder.Decode(new(any)); err != io.EOF {
+				t.Fatalf("unexpected extra YAML document: %v", err)
+			}
+		})
+	}
+}