From e8996d66d72951e2f156b9aca558adfaaf7790aa Mon Sep 17 00:00:00 2001 From: CleverThis Date: Tue, 5 May 2026 05:10:04 +0000 Subject: [PATCH] test(core): fix ASV benchmark timing methodology and remove unused imports - Remove unused `from typing import Any` import from core_retry_patterns_bench.py - Refactor CircuitBreakerStateTransitionBench into three separate benchmark classes (CircuitBreakerClosedToOpenTransitionBench, CircuitBreakerOpenToHalfOpenTransitionBench, CircuitBreakerHalfOpenToClosedTransitionBench), each with a proper setup() method that pre-creates the CircuitBreaker in the correct initial state so only the transition itself is measured - Move open-state CircuitBreaker creation for async fast-fail benchmark into setup() in CircuitBreakerAsyncBench, eliminating instance creation overhead from the timed method Addresses reviewer feedback on PR #10961. --- benchmarks/core_circuit_breaker_bench.py | 103 ++++++++++++----------- benchmarks/core_retry_patterns_bench.py | 1 - 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/benchmarks/core_circuit_breaker_bench.py b/benchmarks/core_circuit_breaker_bench.py index 060082c58..4ff9edb74 100644 --- a/benchmarks/core_circuit_breaker_bench.py +++ b/benchmarks/core_circuit_breaker_bench.py @@ -91,25 +91,14 @@ class CircuitBreakerOpenStateBench: assert self.breaker.state == CircuitBreakerState.OPEN -class CircuitBreakerStateTransitionBench: - """Benchmark state transition overhead.""" +class CircuitBreakerClosedToOpenTransitionBench: + """Benchmark closed-to-open state transition overhead.""" timeout = 60 def setup(self) -> None: - """Set up circuit breaker.""" + """Set up a fresh circuit breaker in closed state.""" self.breaker = CircuitBreaker( - failure_threshold=2, - recovery_timeout=0.1, # Short timeout for testing - expected_exception=Exception, - half_open_max_successes=1, - cooldown_seconds=0.05, - name="test_service", - ) - - def time_closed_to_open_transition(self) -> None: - """Benchmark transition from closed to open state.""" - breaker = CircuitBreaker( failure_threshold=2, recovery_timeout=60.0, expected_exception=Exception, @@ -117,16 +106,25 @@ class CircuitBreakerStateTransitionBench: cooldown_seconds=30.0, name="test_service", ) + + def time_closed_to_open_transition(self) -> None: + """Benchmark transition from closed to open state.""" # Trigger failures to transition to open for _ in range(3): try: - breaker.call(lambda: 1 / 0) + self.breaker.call(lambda: 1 / 0) except ZeroDivisionError: pass - def time_open_to_half_open_transition(self) -> None: - """Benchmark transition from open to half-open state.""" - breaker = CircuitBreaker( + +class CircuitBreakerOpenToHalfOpenTransitionBench: + """Benchmark open-to-half-open state transition overhead.""" + + timeout = 60 + + def setup(self) -> None: + """Set up circuit breaker already in open state.""" + self.breaker = CircuitBreaker( failure_threshold=2, recovery_timeout=0.05, # Very short timeout expected_exception=Exception, @@ -134,23 +132,32 @@ class CircuitBreakerStateTransitionBench: cooldown_seconds=0.01, name="test_service", ) - # Force to open + # Force to open state for _ in range(3): try: - breaker.call(lambda: 1 / 0) + self.breaker.call(lambda: 1 / 0) except ZeroDivisionError: pass - # Wait for recovery timeout + # Wait for recovery timeout so transition to half-open is possible time.sleep(0.1) + + def time_open_to_half_open_transition(self) -> None: + """Benchmark transition from open to half-open state.""" # Attempt call to trigger half-open transition try: - breaker.call(lambda: "success") + self.breaker.call(lambda: "success") except CircuitBreakerOpen: pass - def time_half_open_to_closed_transition(self) -> None: - """Benchmark transition from half-open to closed state.""" - breaker = CircuitBreaker( + +class CircuitBreakerHalfOpenToClosedTransitionBench: + """Benchmark half-open-to-closed state transition overhead.""" + + timeout = 60 + + def setup(self) -> None: + """Set up circuit breaker already in half-open state.""" + self.breaker = CircuitBreaker( failure_threshold=2, recovery_timeout=0.05, expected_exception=Exception, @@ -158,16 +165,19 @@ class CircuitBreakerStateTransitionBench: cooldown_seconds=0.01, name="test_service", ) - # Force to open + # Force to open state for _ in range(3): try: - breaker.call(lambda: 1 / 0) + self.breaker.call(lambda: 1 / 0) except ZeroDivisionError: pass - # Wait for recovery timeout + # Wait for recovery timeout so breaker enters half-open on next call time.sleep(0.1) - # Successful call in half-open should close - breaker.call(lambda: "success") + + def time_half_open_to_closed_transition(self) -> None: + """Benchmark transition from half-open to closed state.""" + # Successful call in half-open should close the breaker + self.breaker.call(lambda: "success") class CircuitBreakerAsyncBench: @@ -176,7 +186,7 @@ class CircuitBreakerAsyncBench: timeout = 60 def setup(self) -> None: - """Set up async circuit breaker.""" + """Set up async circuit breakers.""" self.breaker = CircuitBreaker( failure_threshold=5, recovery_timeout=60.0, @@ -185,6 +195,20 @@ class CircuitBreakerAsyncBench: cooldown_seconds=30.0, name="test_service", ) + # Set up a separate breaker already in open state for fast-fail benchmark + self.open_breaker = CircuitBreaker( + failure_threshold=2, + recovery_timeout=60.0, + expected_exception=Exception, + half_open_max_successes=2, + cooldown_seconds=30.0, + name="test_service_open", + ) + for _ in range(3): + try: + self.open_breaker.call(lambda: 1 / 0) + except ZeroDivisionError: + pass def time_async_call_success(self) -> None: """Benchmark successful async call overhead.""" @@ -204,27 +228,12 @@ class CircuitBreakerAsyncBench: def time_async_fast_fail(self) -> None: """Benchmark async fast-fail when open.""" - # First set up breaker in open state - breaker = CircuitBreaker( - failure_threshold=2, - recovery_timeout=60.0, - expected_exception=Exception, - half_open_max_successes=2, - cooldown_seconds=30.0, - name="test_service", - ) - # Force to open - for _ in range(3): - try: - breaker.call(lambda: 1 / 0) - except ZeroDivisionError: - pass async def dummy_async() -> str: return "should not execute" try: - asyncio.run(breaker.async_call(dummy_async)) + asyncio.run(self.open_breaker.async_call(dummy_async)) except CircuitBreakerOpen: pass diff --git a/benchmarks/core_retry_patterns_bench.py b/benchmarks/core_retry_patterns_bench.py index 172947819..eec08350b 100644 --- a/benchmarks/core_retry_patterns_bench.py +++ b/benchmarks/core_retry_patterns_bench.py @@ -7,7 +7,6 @@ happy-path invocation for the public retry factory functions. from __future__ import annotations import asyncio -from typing import Any from cleveragents.core.retry_patterns import ( get_retry_decorator,