Duplicate Code: Benchmark sampling/convergence loops

🔍 Duplicate Code Detected: Benchmark sampling/convergence loops

Analysis of commit 36c35b4c60fc7c5f09646003dc801bf467663da2 (and recent main commits)

Assignee: @copilot

Summary

Two separate benchmark implementations contain near-identical “collect samples until converged” logic (min sample count + max duration guard + margin-of-error threshold). This duplication makes it easy for the two benchmark types (duration vs memory) to drift in behavior and requires duplicated future fixes/feature work.

Duplication Details

Pattern: Sample collection loop with convergence criteria

• Severity: Medium

• Occurrences: 2 (structurally duplicated, >10 LOC each)

• Locations:

  • tools/benchmark/src/durationBenchmarking/getDuration.ts (lines 146-173)
  • tools/benchmark/src/mocha/memoryTestRunner.ts (lines 325-366)

• Code Sample (duration benchmark):

// tools/benchmark/src/durationBenchmarking/getDuration.ts (146-173)
private addSample(duration: number): boolean {
	this.samples.push(duration);
	if (this.samples.length < this.options.minBatchCount) {
		return true;
	}
	const soFar = this.timer.toSeconds(this.startTime, this.timer.now());
	if (soFar > this.options.maxBenchmarkDurationSeconds) {
		return false;
	}

	const stats = getArrayStatistics(this.samples);
	if (stats.marginOfErrorPercent < 1) {
		return false;
	}

	if (this.samples.length > 1_000_000) {
		return false;
	}
	return true;
}

• Code Sample (memory benchmark):

// tools/benchmark/src/mocha/memoryTestRunner.ts (325-366)
do {
	// ... run iteration, collect before/after samples ...
	runs++;
	// ... build heapUsedArray ...
	heapUsedStats = getArrayStatistics(heapUsedArray, args.samplePercentageToUse);

	if (
		runs >= args.minSampleCount &&
		timer.toSeconds(startTime, timer.now()) > args.maxBenchmarkDurationSeconds
	) {
		break;
	}
} while (
	runs < args.minSampleCount ||
	heapUsedStats.marginOfErrorPercent > args.maxRelativeMarginOfError
);

Impact Analysis

• Maintainability: Any changes to convergence behavior (e.g., different stop conditions, better batch-growth strategy, outlier trimming defaults, new diagnostics) must be implemented twice.
• Bug Risk: Fixes applied to one benchmark type may not be applied to the other, leading to inconsistent performance numbers or flaky benchmarks.
• Code Bloat: The duplicated loop logic and associated guardrails inflate code and obscure intent.

Refactoring Recommendations

1. Extract a shared “converge on samples” helper

   • Suggested location: tools/benchmark/src/sampling.ts (or a new tools/benchmark/src/convergence.ts)
   • Shape idea: a small generic utility like collectUntilConverged({ sampleOnce, getElapsedSeconds, minSamples, maxSeconds, isConverged }) that returns { samples, stats, elapsedSeconds, stopReason }.
   • Benefits: Single source of truth for convergence policy and easier tuning.

2. Centralize margin-of-error threshold policy

   • The duration path hardcodes stats.marginOfErrorPercent < 1, while memory uses maxRelativeMarginOfError.
   • Consider making both use an explicit option with a shared default, so policy changes are consistent.

Implementation Checklist

• Review duplication findings
• Define a shared convergence helper API (sync + async friendly)
• Migrate BenchmarkState.addSample() to the shared helper
• Migrate the memory benchmark loop to the shared helper
• Ensure behavior is unchanged (or document intentional differences)
• Update/extend tests for convergence behavior if applicable

Analysis Metadata

• Analyzed Files: 2 primary files (duration + memory benchmarking)
• Detection Method: Serena semantic analysis + pattern search
• Commits Considered: 36c35b4, bac1653, 707ca3, 70a57c7
• Analysis Date: 2026-03-03T07:59:53.879Z

AI generated by Duplicate Code Detector

To add this workflow in your repository, run gh aw add github/gh-aw/.github/workflows/duplicate-code-detector.md@94662b1dee8ce96c876ba9f33b3ab8be32de82a4. See usage guide.

Start by reading tools/benchmark/src/durationBenchmarking/getDuration.ts and tools/benchmark/src/mocha/memoryTestRunner.ts, focusing on the two sample-convergence loops and their differing thresholds. Review the proposed sampling.ts or convergence.ts location and existing benchmark tests. Done means both paths share an appropriate helper, behavior is preserved or documented, and convergence tests cover the relevant stop conditions.