Skip to content

[code-simplifier] refactor: standardize benchmark warm-up comments and add missing warm-up (#22464 follow-up) #22488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 1 commit into from
Mar 23, 2026
Merged

[code-simplifier] refactor: standardize benchmark warm-up comments and add missing warm-up (#22464 follow-up) #22488

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions pkg/workflow/compiler_performance_benchmark_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,11 @@ Standard workflow for memory profiling.

compiler := NewCompiler(WithNoEmit(true))

// Warm up: run once before timing to initialize one-time caches
// (schema compilation, regex caches) so they don't skew per-op metrics.
// Warm up: run once before timing to prime one-time caches (schema compilation, etc.)
_ = compiler.CompileWorkflow(testFile)

b.ResetTimer()
b.ReportAllocs()

// Track memory allocations
for b.Loop() {
_ = compiler.CompileWorkflow(testFile)
}
Expand Down Expand Up @@ -249,7 +246,7 @@ Test parsing performance.

compiler := NewCompiler()

// Warm up: prime the schema compilation cache before timed measurement.
// Warm up: run once before timing to prime one-time caches (schema compilation, etc.)
_, _ = compiler.ParseWorkflowFile(testFile)

b.ResetTimer()
Expand Down Expand Up @@ -295,7 +292,7 @@ Test validation performance.
compiler := NewCompiler(WithNoEmit(true))
compiler.SetStrictMode(true)

// Warm up: prime the schema compilation cache before timed measurement.
// Warm up: run once before timing to prime one-time caches (schema compilation, etc.)
_ = compiler.CompileWorkflow(testFile)

b.ResetTimer()
Expand Down Expand Up @@ -335,6 +332,10 @@ Test YAML generation.
compiler := NewCompiler()
compiler.SetNoEmit(true)

// Warm up: run once before timing to prime one-time caches (schema compilation, etc.)
_ = compiler.CompileWorkflow(testFile)

b.ResetTimer()
b.ReportAllocs()
for b.Loop() {
_ = compiler.CompileWorkflow(testFile)
Comment on lines +336 to 341
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warm-up compile ignores the returned error. If compilation starts failing, the benchmark will silently measure the error path (and still report results), which can be misleading. Consider checking the error in the warm-up call (and/or the timed loop) and calling b.Fatal on failure so the benchmark fails fast.

Suggested change
_ = compiler.CompileWorkflow(testFile)
b.ResetTimer()
b.ReportAllocs()
for b.Loop() {
_ = compiler.CompileWorkflow(testFile)
if err := compiler.CompileWorkflow(testFile); err != nil {
b.Fatalf("warm-up compilation failed: %v", err)
}
b.ResetTimer()
b.ReportAllocs()
for b.Loop() {
if err := compiler.CompileWorkflow(testFile); err != nil {
b.Fatalf("benchmark compilation failed: %v", err)
}

Copilot uses AI. Check for mistakes.
Expand Down
Loading