mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-10 09:06:36 +00:00
revert(vegas): gate only the prefetch thread; gating ESPN fetches measured worse
84043468 also gated the ESPN chunk fetches and the background data
service's workers, for the hourly sports refresh. A burst test on hdpi
(baseball and football refreshing every 5 minutes, 10-minute soaks, G F F G):
G prefetch gated only 0.87%, 0.83% late; 6+ late 20, 16; fetches 0.3-1.8s
F + fetch threads gated 1.23%, 1.05% late; 6+ late 12, 16; fetches 1.6-4.6s
Every parked fetch thread wakes at each swap and has to take the GIL again
just to park at the end of the window, so twenty of them cost more than
they saved, and the fetches ran two to three times as long. The plugins'
own copies of espn_dates (half the burst) were never gated anyway.
espn_dates and the background data service go back to main's versions and
the module-level active gate goes. Kept from that commit: the render thread
is never gated, a live refresh from another thread can't take its place,
and nested blocks keep the outer boundary.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -26,7 +26,6 @@ from enum import Enum
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
import pytz
|
||||
from src.cache_manager import CacheManager
|
||||
from src.common import render_gate
|
||||
from src.common.json_body import response_json
|
||||
from src.common.espn_dates import (
|
||||
RANGE_RETRY_SECONDS,
|
||||
@@ -318,12 +317,6 @@ class BackgroundDataService:
|
||||
return request_id
|
||||
|
||||
def _fetch_data_worker(self, request: FetchRequest) -> FetchResult:
|
||||
"""Fetch one request on a worker thread, giving way to Vegas's render
|
||||
thread while it scrolls (src/common/render_gate.py)."""
|
||||
with render_gate.yielding():
|
||||
return self._fetch_data(request)
|
||||
|
||||
def _fetch_data(self, request: FetchRequest) -> FetchResult:
|
||||
"""
|
||||
Worker function that performs the actual data fetching.
|
||||
|
||||
|
||||
+13
-24
@@ -47,12 +47,6 @@ except ImportError:
|
||||
def response_json(response: Any) -> Any:
|
||||
return response.json()
|
||||
|
||||
try:
|
||||
from src.common.render_gate import yielding as _yielding
|
||||
except ImportError:
|
||||
# Older cores (see above) have no render gate: fetch freely.
|
||||
from contextlib import nullcontext as _yielding
|
||||
|
||||
# Above this, ESPN returns a truncated list instead of an error. See module
|
||||
# docstring: 500 is the largest value measured to return complete data.
|
||||
ESPN_MAX_LIMIT = 500
|
||||
@@ -199,25 +193,20 @@ def _fetch_one_chunk(
|
||||
|
||||
One bad chunk must not sink the rest of the season, so every error is
|
||||
logged and swallowed here rather than raised to the gather below.
|
||||
|
||||
While Vegas scrolls, a chunk runs only when the render thread is waiting on
|
||||
the panel (``render_gate``): a season refresh is a couple of dozen of these
|
||||
at once, and they used to crowd the render thread off the GIL for seconds.
|
||||
"""
|
||||
with _yielding():
|
||||
try:
|
||||
response = session.get(
|
||||
url,
|
||||
params=dict(params, dates=chunk, limit=ESPN_MAX_LIMIT),
|
||||
headers=headers,
|
||||
timeout=timeout,
|
||||
)
|
||||
response.raise_for_status()
|
||||
return response_json(response)
|
||||
except Exception as exc: # noqa: BLE001 - see docstring
|
||||
if logger:
|
||||
logger.warning("ESPN chunk %s failed, skipping it: %s", chunk, exc)
|
||||
return None
|
||||
try:
|
||||
response = session.get(
|
||||
url,
|
||||
params=dict(params, dates=chunk, limit=ESPN_MAX_LIMIT),
|
||||
headers=headers,
|
||||
timeout=timeout,
|
||||
)
|
||||
response.raise_for_status()
|
||||
return response_json(response)
|
||||
except Exception as exc: # noqa: BLE001 - see docstring
|
||||
if logger:
|
||||
logger.warning("ESPN chunk %s failed, skipping it: %s", chunk, exc)
|
||||
return None
|
||||
|
||||
|
||||
def _fetch_chunks(
|
||||
|
||||
@@ -31,17 +31,12 @@ And a parked thread is never held more than ``MAX_WAIT_SECONDS`` at a time, so
|
||||
whatever the gate gets wrong costs a frame, not a freeze. The render thread
|
||||
itself is never gated, whatever it calls.
|
||||
|
||||
The prefetch thread is not the only one that competes. Once an hour the sports
|
||||
plugins refresh their schedules together, about twenty ESPN chunk-fetch threads
|
||||
at once (hdpi, 2026-09-24), and the render thread queued behind all of them for
|
||||
a 1.9s freeze. So Vegas also makes its gate the *active* one, and code that runs
|
||||
background fetches wraps them in the module-level ``yielding()``, which uses the
|
||||
active gate if there is one and does nothing otherwise: ``espn_dates``' chunk
|
||||
fetches and the background data service's workers.
|
||||
It gates the prefetch thread only. Gating the ESPN fetch threads as well was
|
||||
tried for the hourly sports refresh, twenty-odd of them at once, and measured
|
||||
worse on hdpi (0.85% late frames without it, 1.14% with it, across a burst every
|
||||
five minutes): each parked thread has to take the GIL again just to park at the
|
||||
end of every window, and the fetches ran two to three times as long.
|
||||
|
||||
Only worth enabling on a binding whose SwapOnVSync releases the GIL (see
|
||||
scripts/build_rgbmatrix_nogil.sh). With one that keeps it, the window never
|
||||
lets the background thread run and it only makes progress in the timeouts.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -51,8 +46,7 @@ import sys
|
||||
import threading
|
||||
import time
|
||||
from collections import deque
|
||||
from contextlib import nullcontext
|
||||
from typing import Any, Callable, ContextManager, Deque, List, Optional
|
||||
from typing import Any, Callable, Deque, List, Optional
|
||||
|
||||
#: Park background threads this long before the refresh a swap will return on,
|
||||
#: so a short C call already under way has finished by then.
|
||||
@@ -248,25 +242,3 @@ class _Yielding:
|
||||
sys.setprofile(self._previous)
|
||||
self.gate._local.base = self._previous_base # pylint: disable=protected-access
|
||||
|
||||
|
||||
#: The gate of the Vegas run in progress, if any; see the module docstring.
|
||||
_active: Optional[RenderGate] = None
|
||||
|
||||
|
||||
def set_active(gate: Optional[RenderGate]) -> None:
|
||||
"""Make ``gate`` the one ``yielding()`` uses (None: no gate, run freely)."""
|
||||
global _active # pylint: disable=global-statement
|
||||
_active = gate
|
||||
|
||||
|
||||
def active() -> Optional[RenderGate]:
|
||||
"""The gate ``yielding()`` currently uses, if any."""
|
||||
return _active
|
||||
|
||||
|
||||
def yielding() -> ContextManager[Any]:
|
||||
"""``with render_gate.yielding():`` gives way to the render thread while a
|
||||
Vegas run has a gate, and does nothing otherwise. For background work that
|
||||
does not know whether Vegas is running."""
|
||||
gate = _active
|
||||
return gate.yielding() if gate is not None else nullcontext()
|
||||
|
||||
@@ -357,7 +357,6 @@ class VegasModeCoordinator:
|
||||
getattr(self.render_pipeline, '_prefetch_lock', None),
|
||||
getattr(self.plugin_adapter, '_cache_lock', None))
|
||||
self.display_manager.render_gate = gate
|
||||
render_gate.set_active(gate)
|
||||
logger.info("Vegas: prefetch gated on vsync")
|
||||
|
||||
def _remove_render_gate(self) -> None:
|
||||
@@ -365,8 +364,6 @@ class VegasModeCoordinator:
|
||||
if gate is None:
|
||||
return
|
||||
self.display_manager.render_gate = None
|
||||
if render_gate.active() is gate:
|
||||
render_gate.set_active(None)
|
||||
logger.info("Vegas: prefetch gate parked the prefetch %d times, %.1fs in all",
|
||||
gate.parks, gate.parked_seconds)
|
||||
|
||||
|
||||
@@ -299,60 +299,6 @@ class TestWhoIsGated:
|
||||
assert kept and final is None
|
||||
|
||||
|
||||
class TestActiveGate:
|
||||
@pytest.fixture(autouse=True)
|
||||
def no_gate_left_behind(self):
|
||||
yield
|
||||
render_gate.set_active(None)
|
||||
|
||||
def test_without_a_vegas_run_nothing_is_gated(self):
|
||||
render_gate.set_active(None)
|
||||
with render_gate.yielding():
|
||||
assert sys.getprofile() is None
|
||||
|
||||
def test_with_one_background_work_gives_way(self):
|
||||
gate = RenderGate()
|
||||
render_gate.set_active(gate)
|
||||
with render_gate.yielding():
|
||||
assert sys.getprofile() == gate._hook
|
||||
assert sys.getprofile() is None
|
||||
|
||||
def test_espn_chunk_fetches_give_way(self):
|
||||
from src.common import espn_dates
|
||||
gate = RenderGate()
|
||||
render_gate.set_active(gate)
|
||||
seen = []
|
||||
|
||||
class Response:
|
||||
content = b'{"events": []}'
|
||||
|
||||
def raise_for_status(self):
|
||||
pass
|
||||
|
||||
def json(self):
|
||||
return {"events": []}
|
||||
|
||||
class Session:
|
||||
def get(self, *args, **kwargs):
|
||||
seen.append(sys.getprofile() == gate._hook)
|
||||
return Response()
|
||||
assert espn_dates._fetch_one_chunk(
|
||||
Session(), "http://x", {}, {}, 5, None, "20260924") == {"events": []}
|
||||
assert seen == [True]
|
||||
|
||||
def test_background_data_fetches_give_way(self):
|
||||
from src.background_data_service import BackgroundDataService
|
||||
gate = RenderGate()
|
||||
render_gate.set_active(gate)
|
||||
service = BackgroundDataService.__new__(BackgroundDataService)
|
||||
service._shutdown = True # never started: nothing for __del__ to stop
|
||||
seen = []
|
||||
service._fetch_data = lambda request: seen.append(
|
||||
sys.getprofile() == gate._hook) or "result"
|
||||
assert service._fetch_data_worker(object()) == "result"
|
||||
assert seen == [True]
|
||||
|
||||
|
||||
class TestDisplayManager:
|
||||
@pytest.fixture
|
||||
def dm(self):
|
||||
@@ -443,10 +389,8 @@ class TestVegasWiring:
|
||||
assert c._state_lock in gate._guarded
|
||||
assert c.stream_manager._buffer_lock in gate._guarded
|
||||
assert c.plugin_adapter._cache_lock in gate._guarded
|
||||
assert render_gate.active() is gate # background fetches use it too
|
||||
c._remove_render_gate()
|
||||
assert c.display_manager.render_gate is None
|
||||
assert render_gate.active() is None
|
||||
|
||||
@pytest.mark.parametrize("releases", [False, None])
|
||||
def test_ignored_without_a_binding_that_releases_the_gil(self, monkeypatch, releases):
|
||||
|
||||
Reference in New Issue
Block a user