diff --git a/docs/internal/sources.md b/docs/internal/sources.md index 3ed9279..69204cc 100644 --- a/docs/internal/sources.md +++ b/docs/internal/sources.md @@ -143,7 +143,9 @@ between artifact conversion chunks, around each message, and before publishing the complete prompt. It cannot interrupt template work already in progress and never publishes a partial prompt after observing cancellation. It validates and canonicalizes message roles before carrying roles, session IDs, and cache -control into the rendered prompt. The +control into the rendered prompt. Message and session templates execute into +per-call string builders, so their rendered strings do not require a final +buffer-to-string copy. The [renderer tests](../../internal/prompt/renderer_test.go) own rendering behavior. ## Schemas And Output Validation diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8494b5c..f3ef4aa 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -2,7 +2,7 @@ Implement the remaining local work in the [performance roadmap](performance.md) in order. Each sequential stage is scoped for one Luna coding-agent prompt. -Stages 1–2 are complete; Stages 3–14 remain pending. Update status and evidence +Stages 1–3 are complete; Stages 4–14 remain pending. Update status and evidence as each work unit lands. The completed inspection plan is preserved at `481e2d4:docs/roadmap/implementation.md` in Git history. @@ -150,6 +150,11 @@ family. Existing empty-output repair behavior remains unchanged. ## Stage 3 — Render Messages And Sessions With Local String Builders +**Status: Complete (2026-10-09).** Message and session template execution now +uses per-call `strings.Builder` values. Exact rendering, cancellation, and +concurrent compiled-template behavior pass; benchmark evidence shows lower +allocation without a measurable retained-output increase. + **Outcome:** Template output becomes a string without a final buffer-to-string copy. @@ -623,6 +628,61 @@ Focused verification passed on the implementation commit: `go test -mod=readonly ./internal/validate ./internal/usecase`. The benchmark family above was rerun on that commit with the recorded command. +### Compiled-Rendering And Retained-Output Comparison + +The maintained Stage 1 benchmark fixture was measured before the rendering +change at `dd89be3` (after the basic-validation change, which does not affect +rendering), then again after replacing the message and session buffers in the +working tree based on that revision. Setup and fixture contents are unchanged. +Environment: Linux/amd64, Intel Core i5-7260U @ 2.20 GHz, benchmark parallelism +4, Go 1.26.8, ConfigKit `v0.5.0`, JSON Schema `v6.0.2`, and backend modules +`v3.20260930.0`. + +Command for both runs (five 500 ms samples per benchmark, without the race +detector): + +```sh +GOTOOLCHAIN=local GOWORK=off GOPROXY=off GONOPROXY=none GOSUMDB=off \ + go test -mod=readonly -run '^$' \ + -bench '^(BenchmarkCompiledRendering|BenchmarkRetainedRenderedOutput)$' \ + -benchmem -benchtime=500ms -count=5 ./internal/prompt +``` + +Times list all five `ns/op` samples in command order, followed by median and +range. Allocation figures are `B/op; allocs/op`. + +| Compiled-render case | Before `ns/op` (median; range) | Before allocations | After `ns/op` (median; range) | After allocations | +| --- | --- | --- | --- | --- | +| Small message | 2066, 1906, 2003, 2027, 2010 (2010; 1906–2066) | 1632; 20 | 1912, 2030, 1877, 2045, 1926 (1926; 1877–2045) | 1568; 20 | +| Artifact-backed 256 KiB message | 279116, 341635, 285472, 348345, 341764 (341635; 279116–348345) | 1050713–1050718; 25 | 165860, 239221, 255435, 223641, 214475 (223641; 165860–255435) | 788485–788489; 24 | +| Repeated across session and messages | 966012, 1073914, 1098700, 1154416, 1064160 (1073914; 966012–1154416) | 3946015–3946038; 74 | 778584, 777091, 888988, 739473, 802395 (778584; 739473–888988) | 2446632–2446651; 71–72 | + +The retained-output workload holds four separately rendered 256 KiB results +and reports post-GC live-heap change over the same setup-only fixture. Its +allocation and retained-heap samples were: + +| Measurement | Before | After | +| --- | --- | --- | +| `ns/op` (median; range) | 331625, 333337, 343766, 302864, 329355 (331625; 302864–343766) | 244284, 239994, 238773, 239356, 264424 (239994; 238773–264424) | +| `B/op; allocs/op` | 1050696–1050708; 25 | 788486–788495; 24 | +| Retained heap (`retained-B`, median; range) | 1049744, 1046160, 1052768, 1044928, 1047264 (1047264; 1044928–1052768) | 1045824, 1054000, 1048288, 1047504, 1047168 (1047504; 1045824–1054000) | + +The artifact-backed case removes about 256 KiB of allocation per render, and +the repeated case removes about 1.5 MiB; both also lose one allocation. Small +rendering changes by 64 bytes and 0 allocations. The retained-heap ranges +overlap, with medians differing by only 240 bytes for four 256 KiB outputs, +so the returned strings keep approximately the same live output storage rather +than retaining an additional builder buffer. This is expected: the builder's +backing storage is the returned string instead of being copied into a second +allocation. Measurements support retaining the simpler local-builder change +without a material retention trade-off; the existing per-render artifact-text +conversion and cache remain unchanged. + +Focused verification passed: `go test -race -mod=readonly ./internal/prompt`, +including exact message/session output, cancellation, artifact-text lifecycle, +and a new concurrent-render case that verifies independent request variables, +artifact text, and session IDs against shared compiled templates. + ## Open Questions No consumer-policy decisions remain open. File reservation and encoding error diff --git a/internal/prompt/go_renderer.go b/internal/prompt/go_renderer.go index 134a66d..2296a14 100644 --- a/internal/prompt/go_renderer.go +++ b/internal/prompt/go_renderer.go @@ -1,7 +1,6 @@ package prompt import ( - "bytes" "context" "errors" "fmt" @@ -126,7 +125,7 @@ func (r *goRenderer) Render(ctx context.Context, definition *domain.PromptDefini if err := ctx.Err(); err != nil { return nil, err } - var buf bytes.Buffer + var buf strings.Builder executeErr := tmpl.Execute(&buf, vars) if err := ctx.Err(); err != nil { return nil, err @@ -235,7 +234,7 @@ func executeSessionTemplate(ctx context.Context, tmpl *template.Template, vars m if err := ctx.Err(); err != nil { return "", err } - var buf bytes.Buffer + var buf strings.Builder executeErr := tmpl.Execute(&buf, vars) if err := ctx.Err(); err != nil { return "", err diff --git a/internal/prompt/renderer_test.go b/internal/prompt/renderer_test.go index f1d9426..928e1b5 100644 --- a/internal/prompt/renderer_test.go +++ b/internal/prompt/renderer_test.go @@ -4,8 +4,11 @@ import ( "bytes" "context" "errors" + "fmt" "strings" + "sync" "testing" + "text/template" "gitea.maximumdirect.net/eric/promptkit/internal/domain" ) @@ -501,6 +504,75 @@ func TestGoRendererArtifactTextLifecycle(t *testing.T) { } } +func TestCompiledRendererKeepsConcurrentRequestsIndependent(t *testing.T) { + const ( + promptID = "concurrent-render" + version = "v1" + ) + sessionTemplate, err := CompileTemplate("session", "session-{{.request}}") + if err != nil { + t.Fatalf("compile session template: %v", err) + } + messageTemplate, err := CompileTemplate("message", `request={{.request}};document={{input "document"}}`) + if err != nil { + t.Fatalf("compile message template: %v", err) + } + renderer := NewCompiledRenderer(compiledRendererTestLookup{ + id: promptID, + version: version, + templates: CompiledTemplates{ + Session: sessionTemplate, + Messages: []*template.Template{messageTemplate}, + }, + }) + definition := &domain.PromptDefinition{ + ID: promptID, Version: version, SessionID: "session-{{.request}}", + Inputs: []domain.PromptInput{{Name: "document"}}, + Templates: []domain.PromptMessageTemplate{{Role: "user", Content: `request={{.request}};document={{input "document"}}`}}, + } + + const requestCount = 24 + errs := make(chan error, requestCount) + var wait sync.WaitGroup + wait.Add(requestCount) + for i := 0; i < requestCount; i++ { + go func(i int) { + defer wait.Done() + request := fmt.Sprintf("request-%d", i) + document := fmt.Sprintf("document-%d", i) + rendered, err := renderer.Render(context.Background(), definition, + map[string]*domain.Artifact{"document": {Body: []byte(document)}}, + map[string]string{"request": request}, + ) + if err != nil { + errs <- fmt.Errorf("render %s: %w", request, err) + return + } + if rendered.SessionID != "session-"+request || len(rendered.Messages) != 1 || + rendered.Messages[0].Content != "request="+request+";document="+document { + errs <- fmt.Errorf("render %s returned session %q and messages %#v", request, rendered.SessionID, rendered.Messages) + } + }(i) + } + wait.Wait() + close(errs) + for err := range errs { + t.Error(err) + } +} + +type compiledRendererTestLookup struct { + id, version string + templates CompiledTemplates +} + +func (lookup compiledRendererTestLookup) GetCompiledTemplates(id, version string) (CompiledTemplates, bool) { + if id != lookup.id || version != lookup.version { + return CompiledTemplates{}, false + } + return lookup.templates, true +} + // BenchmarkGoRendererArtifactReferences measures NewGoRenderer's per-call // template-parsing fallback rather than the captured compiled path. func BenchmarkGoRendererArtifactReferences(b *testing.B) {