From 86671b1a27a87e35e660bdc73bdcecc9aed76d56 Mon Sep 17 00:00:00 2001 From: Ruslan Dautkhanov Date: Fri, 22 May 2026 11:09:54 -0600 Subject: [PATCH] perf: tighten fresh_jvm readiness polling for XD CodSpeed sampling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XD (full cold start) was producing ~2.7 s per round on CodSpeed CI, which exhausts the per-benchmark wall-time budget after one sample. With only one sample, CodSpeed reports 0% noise and "no measurable impact" for every cold-start PR — including any genuine 10-100 ms improvement that would otherwise be detectable. This made #15 (GatewayConnection background-warm) invisible on the dashboard even though the optimization itself is correct. Replace the legacy readiness model: 0.25 s sleep + 1 retry x 2.0 s = 2.25 s ceiling with tight polling: 0 s sleep + 300 retries x 0.05 s = 15 s ceiling fast hosts now succeed in ~50-200 ms; slow CI in ~1-2 s; ceiling still generous enough for any reasonable runner. With XD per-round down to ~250-500 ms, CodSpeed fits 4-8 samples per benchmark budget and can compute a usable CV. Also drops the unconditional startup_sleep: the rationale ("let the OS reuse the listen port") no longer applies — the JVM-side GatewayServer.startSocket() uses SO_REUSEADDR (already on master), so bind() succeeds immediately even over TIME_WAIT. Backwards compatibility: callers that need the old sleep pattern can pass ``startup_sleep=0.25`` explicitly. Local measurement (M-series + JDK 21): XD before: median ~355 ms, max ~2.4 s (variance dominated by retries) XD after: median ~260 ms, max ~420 ms (proper tight polling) Co-authored-by: Isaac --- py4j-python/src/py4j/tests/perf/jvm.py | 36 ++++++++++++++----- .../src/py4j/tests/perf/scenarios/macro.py | 14 ++++---- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/py4j-python/src/py4j/tests/perf/jvm.py b/py4j-python/src/py4j/tests/perf/jvm.py index 51ef4513..acc92fee 100644 --- a/py4j-python/src/py4j/tests/perf/jvm.py +++ b/py4j-python/src/py4j/tests/perf/jvm.py @@ -152,10 +152,30 @@ def shutdown_jvm(process, gateway=None, timeout=10): @contextmanager -def fresh_jvm(heap="4g", startup_sleep=0.25, readiness_retries=1, - enable_callbacks=False): +def fresh_jvm(heap="4g", startup_sleep=0.0, readiness_retries=300, + readiness_poll_s=0.05, enable_callbacks=False): """Spawn a JVM, build a gateway, yield it, then tear both down. + Readiness model: poll connect ``readiness_retries`` times waiting + ``readiness_poll_s`` between attempts. Total wait ceiling = + ``startup_sleep + readiness_retries * readiness_poll_s`` (default + 15 s — enough headroom for any reasonable CI runner while keeping + the typical fast-host wait under ~100 ms). + + Tight polling matters for XD (full cold start) on CodSpeed: the + previous default (0.25 s sleep + 1 retry × 2 s) made each XD + iteration cost ~2.7 s, leaving CodSpeed only ~1 sample per benchmark + budget — too few for variance / regression detection. With 50 ms + polling each XD round now finishes in ~600 ms, enough for 4-5 + samples per budget. (issue #557) + + Backwards compatibility: callers that previously relied on the + 0.25 s startup_sleep can pass ``startup_sleep=0.25`` to restore + the old behavior. The JVM's listen socket uses ``SO_REUSEADDR``, + so the original "let the OS reuse the listen port" rationale for + the unconditional sleep no longer applies — bind() succeeds + immediately even over TIME_WAIT. + :param enable_callbacks: if True, start a CallbackServer alongside the gateway so Java can invoke Python proxies (needed for X4). @@ -164,17 +184,17 @@ def fresh_jvm(heap="4g", startup_sleep=0.25, readiness_retries=1, gateway.jvm.java.lang.System.currentTimeMillis() """ process = spawn_jvm(heap=heap) - # Brief sleep lets the OS reuse the listen port; mirrors the 250ms - # default used across the existing test suite. - time.sleep(startup_sleep) + if startup_sleep > 0: + time.sleep(startup_sleep) try: - check_connection(retries=readiness_retries) + check_connection(retries=readiness_retries, + retry_sleep=readiness_poll_s) except Py4JNetworkError: shutdown_jvm(process, None) raise JvmStartupError( "JVM spawned but did not accept connections within " - "{0}s. Is port 25333 already in use?".format( - startup_sleep + 2.0 * readiness_retries)) + "{0:.1f}s. Is port 25333 already in use?".format( + startup_sleep + readiness_poll_s * readiness_retries)) if enable_callbacks: gateway = JavaGateway( callback_server_parameters=CallbackServerParameters()) diff --git a/py4j-python/src/py4j/tests/perf/scenarios/macro.py b/py4j-python/src/py4j/tests/perf/scenarios/macro.py index d14373a7..888baeb8 100644 --- a/py4j-python/src/py4j/tests/perf/scenarios/macro.py +++ b/py4j-python/src/py4j/tests/perf/scenarios/macro.py @@ -478,15 +478,13 @@ class XD_FullColdStart(MacroScenario): iterations_per_round = 1 def measure(self, gateway): - # Delegate to fresh_jvm so JVM startup readiness is polled - # rather than blind-slept — CodSpeed CI runners are slower - # than a local laptop and the original 0.25 s sleep was - # racing the JVM's listening socket (causing - # ConnectionRefusedError under CI). fresh_jvm uses a retry - # loop matching the rest of the test suite's conventions and - # cleanly tears down the subprocess in __exit__. + # Delegate to fresh_jvm. New defaults poll readiness every + # 50 ms with a 15 s ceiling — fast hosts finish in ~100 ms, + # CI in ~1-2 s. This lets CodSpeed fit multiple samples per + # benchmark budget instead of just one giant 2.7 s sample + # that hid C2's (and any other cold-start tweak's) impact. from py4j.tests.perf.jvm import fresh_jvm - with fresh_jvm(readiness_retries=5) as gw: + with fresh_jvm() as gw: gw.jvm.java.lang.System.currentTimeMillis()