Release validation source bytes after checks

This commit is contained in:
2026-10-09 01:33:21 +00:00
parent a3e993d95a
commit 182da397e3
8 changed files with 169 additions and 32 deletions

View File

@@ -52,8 +52,11 @@ must close it when finished. The shared validation executor closes its tree on
every return before product checks or build callbacks run. A metadata-only every return before product checks or build callbacks run. A metadata-only
selection never starts the reader. Per-read contexts cancel an in-flight blob selection never starts the reader. Per-read contexts cancel an in-flight blob
exchange without tying the reader lifetime to a short-lived caller context. exchange without tying the reader lifetime to a short-lived caller context.
Until formatting and parsed-result reuse are in place, regular-file bytes stay Regular-file reads return independent caller-owned byte slices and are not
cached by object ID for the lifetime of the tree. retained by the tree. Symlink targets are retained as immutable owned strings
by blob object ID; explicitly reading a symlink still returns independent
bytes. The Go-format check caches only whether each successfully inspected
object is formatted, then reports every unformatted path that refers to it.
The internal Markdown parser uses Goldmark to record inline, reference, and The internal Markdown parser uses Goldmark to record inline, reference, and
autolinks, images, titles, and their source use sites, and to build heading autolinks, images, titles, and their source use sites, and to build heading

View File

@@ -1,6 +1,6 @@
# Computational Efficiency Implementation Plan # Computational Efficiency Implementation Plan
**Status:** Stages 1–5 complete; Stages 6–11 pending. **Status:** Stages 1–6 complete; Stages 7–11 pending.
## Purpose And Baseline ## Purpose And Baseline
@@ -359,6 +359,22 @@ buffers after formatting, and duplicate-content files retain per-path findings.
Record the tradeoff for explicit repeated raw reads. Update validation mechanics Record the tradeoff for explicit repeated raw reads. Update validation mechanics
with the new ownership and reuse choices. with the new ownership and reuse choices.
**Completion:** Complete. Regular-file reads now return independent bytes and
are not retained by `CandidateTree`; repeated explicit reads therefore transfer
the blob again. The tree caches owned symlink-target strings by object ID, and
explicit symlink reads still return independent byte slices. Go formatting
caches only successful formatted/unformatted outcomes by object ID, checks
cancellation per path, and reports every unformatted path even when contents
repeat. Validation tests and the formatting/Markdown destination benchmarks
passed. On the same Go 1.26.8, Git 2.53.0, Linux x86_64 Intel Core i5-7260U
setup, three repetitions measured unique-file formatting at 75.0–75.9 ms loose
and 37.9–42.5 ms packed, repeated-ID formatting at 1.97–2.17 ms, and the
Markdown workload at 16.3–17.0 ms. Formatting used about 2.89 MB/op for unique
files and 121 KB/op for repeated IDs. The retained-memory observation was
129,496 bytes with the candidate and Markdown cache alive, compared with
1,385,040–1,385,616 bytes in the baseline fixture. This retains Markdown parser
data by path until Stage 7; it is a workload-specific heap observation.
## Stage 7: Reuse Owned Markdown And Module Results ## Stage 7: Reuse Owned Markdown And Module Results
**Owners:** [markdown.go](../../internal/validation/markdown.go), **Owners:** [markdown.go](../../internal/validation/markdown.go),

View File

@@ -90,7 +90,6 @@ func TestCheckGoFormatPreservesParserAndInspectionCauses(t *testing.T) {
repositoryDir: t.TempDir(), repositoryDir: t.TempDir(),
entries: []git.TreeEntry{{Path: "missing.go", ObjectID: strings.Repeat("0", 40), Mode: "100644", Type: "blob"}}, entries: []git.TreeEntry{{Path: "missing.go", ObjectID: strings.Repeat("0", 40), Mode: "100644", Type: "blob"}},
byPath: map[string]int{"missing.go": 0}, byPath: map[string]int{"missing.go": 0},
blobs: make(map[string][]byte),
} }
cleanupCandidateTree(t, tree) cleanupCandidateTree(t, tree)
err := checkGoFormat(context.Background(), tree) err := checkGoFormat(context.Background(), tree)
@@ -104,6 +103,33 @@ func TestCheckGoFormatPreservesParserAndInspectionCauses(t *testing.T) {
}) })
} }
func TestCheckGoFormatReportsEveryPathForRepeatedUnformattedContent(t *testing.T) {
repository := newTreeRepository(t)
contents := []byte("package app\nfunc value(){return}\n")
writeTreeFile(t, repository, "first.go", contents)
writeTreeFile(t, repository, "nested/second.go", contents)
commit := commitCandidateTree(t, repository)
tree, err := ReadCandidateTree(context.Background(), repository, commit)
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
err = checkGoFormat(context.Background(), tree)
var failure *Failure
if !errors.As(err, &failure) || failure.Check != GoFormat {
t.Fatalf("checkGoFormat() error = %v, want go-format findings", err)
}
if len(failure.Findings) != 2 || failure.Findings[0].Path != "first.go" || failure.Findings[1].Path != "nested/second.go" {
t.Fatalf("findings = %#v, want both paths in tree order", failure.Findings)
}
first, _ := tree.Entry("first.go")
second, _ := tree.Entry("nested/second.go")
if first.ObjectID != second.ObjectID {
t.Fatalf("fixture object IDs = %q and %q, want repeated content", first.ObjectID, second.ObjectID)
}
}
func TestRunGoFormatIsOptionalAndStopsAfterFinding(t *testing.T) { func TestRunGoFormatIsOptionalAndStopsAfterFinding(t *testing.T) {
repository := newTreeRepository(t) repository := newTreeRepository(t)
writeTreeFile(t, repository, "unformatted.go", []byte("package fixture\nfunc Value( ){return 1}\n")) writeTreeFile(t, repository, "unformatted.go", []byte("package fixture\nfunc Value( ){return 1}\n"))

View File

@@ -69,11 +69,11 @@ func readModuleDeclaration(ctx context.Context, tree *CandidateTree, entry git.T
return contents, nil return contents, nil
} }
target, err := tree.ReadFile(ctx, entry.Path) target, err := tree.readSymlinkTarget(ctx, entry)
if err != nil { if err != nil {
return nil, fmt.Errorf("read symlink declaration: %w", err) return nil, fmt.Errorf("read symlink declaration: %w", err)
} }
destination, err := tree.ResolveDestination(ctx, entry.Path, string(target)) destination, err := tree.ResolveDestination(ctx, entry.Path, target)
if err != nil { if err != nil {
return nil, fmt.Errorf("resolve symlink declaration target %q: %w", target, err) return nil, fmt.Errorf("resolve symlink declaration target %q: %w", target, err)
} }

View File

@@ -229,7 +229,7 @@ func freshPerformanceTrees(tb testing.TB, fixture performanceTree, count int) []
trees := make([]*CandidateTree, count) trees := make([]*CandidateTree, count)
for index := range trees { for index := range trees {
tree := *base tree := *base
tree.blobs = make(map[string][]byte) tree.symlinkTargets = make(map[string]string)
trees[index] = &tree trees[index] = &tree
} }
if err := base.Close(); err != nil { if err := base.Close(); err != nil {

View File

@@ -13,14 +13,14 @@ import (
// CandidateTree is an inventory of one captured commit with blob contents // CandidateTree is an inventory of one captured commit with blob contents
// loaded lazily by object ID. // loaded lazily by object ID.
type CandidateTree struct { type CandidateTree struct {
repositoryDir string repositoryDir string
entries []git.TreeEntry entries []git.TreeEntry
byPath map[string]int byPath map[string]int
directories map[string]struct{} directories map[string]struct{}
blobs map[string][]byte blobReader *git.BlobReader
blobReader *git.BlobReader symlinkTargets map[string]string
closed bool closed bool
closeErr error closeErr error
} }
// ReadCandidateTree enumerates the captured commit without reading from the // ReadCandidateTree enumerates the captured commit without reading from the
@@ -31,11 +31,11 @@ func ReadCandidateTree(ctx context.Context, repositoryDir, commit string) (*Cand
return nil, err return nil, err
} }
tree := &CandidateTree{ tree := &CandidateTree{
repositoryDir: repositoryDir, repositoryDir: repositoryDir,
entries: entries, entries: entries,
byPath: make(map[string]int, len(entries)), byPath: make(map[string]int, len(entries)),
directories: make(map[string]struct{}), directories: make(map[string]struct{}),
blobs: make(map[string][]byte), symlinkTargets: make(map[string]string),
} }
tree.directories["."] = struct{}{} tree.directories["."] = struct{}{}
for index, entry := range entries { for index, entry := range entries {
@@ -52,8 +52,9 @@ func (tree *CandidateTree) Entries() []git.TreeEntry {
return append([]git.TreeEntry(nil), tree.entries...) return append([]git.TreeEntry(nil), tree.entries...)
} }
// ReadFile returns the committed blob bytes for path. Contents are cached by // ReadFile returns newly allocated, caller-owned bytes for a committed blob.
// object ID for later reads from this tree. // Regular-file content is not cached, so repeated reads request the object
// again through the tree's scoped blob reader.
func (tree *CandidateTree) ReadFile(ctx context.Context, path string) ([]byte, error) { func (tree *CandidateTree) ReadFile(ctx context.Context, path string) ([]byte, error) {
if tree.closed { if tree.closed {
return nil, fmt.Errorf("read committed path %q: candidate tree is closed", path) return nil, fmt.Errorf("read committed path %q: candidate tree is closed", path)
@@ -66,8 +67,41 @@ func (tree *CandidateTree) ReadFile(ctx context.Context, path string) ([]byte, e
if entry.Type != "blob" { if entry.Type != "blob" {
return nil, fmt.Errorf("committed path %q is a %s entry, not a blob", path, entry.Type) return nil, fmt.Errorf("committed path %q is a %s entry, not a blob", path, entry.Type)
} }
if contents, ok := tree.blobs[entry.ObjectID]; ok { if entry.Mode == "120000" {
return append([]byte(nil), contents...), nil target, err := tree.readSymlinkTarget(ctx, entry)
if err != nil {
return nil, err
}
return []byte(target), nil
}
return tree.readBlob(ctx, entry)
}
func (tree *CandidateTree) readSymlinkTarget(ctx context.Context, entry git.TreeEntry) (string, error) {
if tree.closed {
return "", fmt.Errorf("read committed path %q: candidate tree is closed", entry.Path)
}
if entry.Type != "blob" || entry.Mode != "120000" {
return "", fmt.Errorf("committed path %q is not a symlink blob", entry.Path)
}
if target, ok := tree.symlinkTargets[entry.ObjectID]; ok {
return target, nil
}
contents, err := tree.readBlob(ctx, entry)
if err != nil {
return "", err
}
target := string(contents)
if tree.symlinkTargets == nil {
tree.symlinkTargets = make(map[string]string)
}
tree.symlinkTargets[entry.ObjectID] = target
return target, nil
}
func (tree *CandidateTree) readBlob(ctx context.Context, entry git.TreeEntry) ([]byte, error) {
if tree.closed {
return nil, fmt.Errorf("read committed path %q: candidate tree is closed", entry.Path)
} }
if tree.blobReader == nil { if tree.blobReader == nil {
// The reader belongs to the tree and can outlive an individual read // The reader belongs to the tree and can outlive an individual read
@@ -77,12 +111,8 @@ func (tree *CandidateTree) ReadFile(ctx context.Context, path string) ([]byte, e
} }
contents, err := tree.blobReader.Read(ctx, entry.ObjectID) contents, err := tree.blobReader.Read(ctx, entry.ObjectID)
if err != nil { if err != nil {
return nil, fmt.Errorf("read committed path %q: %w", path, err) return nil, fmt.Errorf("read committed path %q: %w", entry.Path, err)
} }
if tree.blobs == nil {
tree.blobs = make(map[string][]byte)
}
tree.blobs[entry.ObjectID] = append([]byte(nil), contents...)
return contents, nil return contents, nil
} }
@@ -179,11 +209,12 @@ func (tree *CandidateTree) resolveDestination(ctx context.Context, directory str
if active[partial] { if active[partial] {
return Destination{Kind: DestinationSymlinkCycle, Path: partial}, nil return Destination{Kind: DestinationSymlinkCycle, Path: partial}, nil
} }
linkText, err := tree.ReadFile(ctx, partial) entry, _ := tree.Entry(partial)
linkText, err := tree.readSymlinkTarget(ctx, entry)
if err != nil { if err != nil {
return Destination{}, fmt.Errorf("read committed symlink %q: %w", partial, err) return Destination{}, fmt.Errorf("read committed symlink %q: %w", partial, err)
} }
target := string(linkText) target := linkText
targetDirectory := directory targetDirectory := directory
if path.IsAbs(target) { if path.IsAbs(target) {
relative, inside, err := tree.relativeAbsoluteDestination(target) relative, inside, err := tree.relativeAbsoluteDestination(target)

View File

@@ -78,6 +78,56 @@ func TestReadCandidateTreeUsesCapturedCommitAndEnumeratesGitEntries(t *testing.T
} }
} }
func TestReadFileReturnsOwnedBytesAndCachesOnlySymlinkTargets(t *testing.T) {
repository := newTreeRepository(t)
writeTreeFile(t, repository, "source.txt", []byte("committed content"))
if err := os.Symlink("source.txt", filepath.Join(repository, "alias.txt")); err != nil {
t.Fatal(err)
}
commit := commitCandidateTree(t, repository)
tree, err := ReadCandidateTree(context.Background(), repository, commit)
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
first, err := tree.ReadFile(context.Background(), "source.txt")
if err != nil {
t.Fatal(err)
}
first[0] = 'X'
second, err := tree.ReadFile(context.Background(), "source.txt")
if err != nil {
t.Fatal(err)
}
if string(second) != "committed content" {
t.Fatalf("second regular-file read = %q, want committed content", second)
}
if &first[0] == &second[0] {
t.Fatal("regular-file reads share a backing byte slice")
}
linkFirst, err := tree.ReadFile(context.Background(), "alias.txt")
if err != nil {
t.Fatal(err)
}
linkFirst[0] = 'X'
linkSecond, err := tree.ReadFile(context.Background(), "alias.txt")
if err != nil {
t.Fatal(err)
}
if string(linkSecond) != "source.txt" {
t.Fatalf("second symlink read = %q, want source.txt", linkSecond)
}
if len(tree.symlinkTargets) != 1 {
t.Fatalf("cached symlink target count = %d, want 1", len(tree.symlinkTargets))
}
destination, err := tree.ResolveDestination(context.Background(), "README.md", "alias.txt")
if err != nil || destination.Kind != DestinationEntry || destination.Path != "source.txt" {
t.Fatalf("ResolveDestination() = %#v, %v; want source.txt entry", destination, err)
}
}
func cleanupCandidateTree(tb testing.TB, tree *CandidateTree) { func cleanupCandidateTree(tb testing.TB, tree *CandidateTree) {
tb.Helper() tb.Helper()
tb.Cleanup(func() { _ = tree.Close() }) tb.Cleanup(func() { _ = tree.Close() })

View File

@@ -199,6 +199,7 @@ func checkWorkspaceFiles(tree *CandidateTree) error {
func checkGoFormat(ctx context.Context, tree *CandidateTree) error { func checkGoFormat(ctx context.Context, tree *CandidateTree) error {
var findings []Finding var findings []Finding
formattedByObjectID := make(map[string]bool)
for _, entry := range tree.Entries() { for _, entry := range tree.Entries() {
if entry.Type != "blob" || (entry.Mode != "100644" && entry.Mode != "100755") || !strings.HasSuffix(entry.Path, ".go") { if entry.Type != "blob" || (entry.Mode != "100644" && entry.Mode != "100755") || !strings.HasSuffix(entry.Path, ".go") {
continue continue
@@ -206,6 +207,12 @@ func checkGoFormat(ctx context.Context, tree *CandidateTree) error {
if err := ctx.Err(); err != nil { if err := ctx.Err(); err != nil {
return &Failure{Check: GoFormat, Cause: err} return &Failure{Check: GoFormat, Cause: err}
} }
if formatted, ok := formattedByObjectID[entry.ObjectID]; ok {
if !formatted {
findings = append(findings, Finding{Path: entry.Path, Message: "file is not gofmt-formatted"})
}
continue
}
contents, err := tree.ReadFile(ctx, entry.Path) contents, err := tree.ReadFile(ctx, entry.Path)
if err != nil { if err != nil {
return &Failure{ return &Failure{
@@ -228,7 +235,11 @@ func checkGoFormat(ctx context.Context, tree *CandidateTree) error {
Cause: fmt.Errorf("format committed Go file %q: %w", entry.Path, err), Cause: fmt.Errorf("format committed Go file %q: %w", entry.Path, err),
} }
} }
if !bytes.Equal(contents, formatted) { isFormatted := bytes.Equal(contents, formatted)
formattedByObjectID[entry.ObjectID] = isFormatted
contents = nil
formatted = nil
if !isFormatted {
findings = append(findings, Finding{Path: entry.Path, Message: "file is not gofmt-formatted"}) findings = append(findings, Finding{Path: entry.Path, Message: "file is not gofmt-formatted"})
} }
} }