Skip to content

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

Merged
pelikhan merged 1 commit intomainfrom
code-simplifier/benchmark-consistency-2026-03-23-2e7a8324423dc434
Mar 23, 2026
Merged

[code-simplifier] refactor: standardize benchmark warm-up comments and add missing warm-up (#22464 follow-up)#22488
pelikhan merged 1 commit intomainfrom
code-simplifier/benchmark-consistency-2026-03-23-2e7a8324423dc434

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Follow-up cleanup to #22464, which added warm-up phases to benchmarks.

Files Simplified

  • pkg/workflow/compiler_performance_benchmark_test.go — consistent warm-up patterns across all 7 benchmarks

Improvements Made

1. Unified warm-up comment phrasing

Three different comment styles existed after #22464:

  • "Warm up: run once before timing to prime one-time caches (schema compilation, etc.)" (×3)
  • "Warm up: run once before timing to initialize one-time caches\n// (schema compilation, regex caches) so they don't skew per-op metrics." (×1)
  • "Warm up: prime the schema compilation cache before timed measurement." (×2)

All 7 benchmarks now use the same phrasing.

2. Added missing warm-up to BenchmarkYAMLGeneration

BenchmarkYAMLGeneration was the only benchmark without a warm-up phase or b.ResetTimer() call, even though it runs compiler.CompileWorkflow (which involves schema compilation). Added warm-up + b.ResetTimer() for measurement accuracy consistent with all other benchmarks.

3. Removed redundant inline comment

// Track memory allocations before the benchmark loop in BenchmarkCompileMemoryUsage was redundant — b.ReportAllocs() already conveys this intent.

Changes Based On

  • #22464 — perf: fix BenchmarkCompileMemoryUsage regression (+81.9%)

Testing

  • ✅ Benchmarks execute successfully (go test -bench BenchmarkCompileSimpleWorkflow|BenchmarkYAMLGeneration -benchtime=1x)
  • ✅ Unit tests pass (go test -run TestCompile ./pkg/workflow/)
  • ✅ Build succeeds (go build ./pkg/workflow/)
  • ✅ No functional changes — benchmark behavior is identical

Generated by Code Simplifier ·

  • expires on Mar 24, 2026, 7:00 PM UTC

…-up to BenchmarkYAMLGeneration

- Unify all 7 warm-up comments to the same phrasing:
  "Warm up: run once before timing to prime one-time caches (schema compilation, etc.)"
- Remove redundant '// Track memory allocations' comment in BenchmarkCompileMemoryUsage
- Add missing warm-up + b.ResetTimer() to BenchmarkYAMLGeneration, consistent with all other benchmarks

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review March 23, 2026 19:04
Copilot AI review requested due to automatic review settings March 23, 2026 19:04
@pelikhan pelikhan merged commit 6eb0c4b into main Mar 23, 2026
39 of 40 checks passed
@pelikhan pelikhan deleted the code-simplifier/benchmark-consistency-2026-03-23-2e7a8324423dc434 branch March 23, 2026 19:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Follow-up cleanup to standardize benchmark warm-up comments and ensure BenchmarkYAMLGeneration includes a warm-up + b.ResetTimer() for more accurate measurements.

Changes:

  • Standardized warm-up comment phrasing across benchmarks in compiler_performance_benchmark_test.go.
  • Added a warm-up compile + b.ResetTimer() to BenchmarkYAMLGeneration.
  • Removed a redundant “Track memory allocations” inline comment (since b.ReportAllocs() already conveys this).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +336 to 341
_ = compiler.CompileWorkflow(testFile)

b.ResetTimer()
b.ReportAllocs()
for b.Loop() {
_ = compiler.CompileWorkflow(testFile)
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants