Render widgets concurrently instead of one at a time
A layout with several network-backed widgets (photos, weather, calendar) paid their fetch latency serially in one /frame/* request, which could exceed the firmware's fixed HTTP timeout and show a false "server failed" status screen even though the server was still working -- most visibly on the hold-triggered "cycle layouts" action, which swaps in a whole new, cold-started widget set. Each widget now renders on its own DB session in a thread pool (a plain Session isn't thread-safe to share, but the per-frame threading.Lock in frame_locked/widget_locked already made this kind of concurrency safe by design -- see app/db.py); regions are still collected in sort_order so overlapping widgets paint in the same z-order as before.
This commit is contained in:
@@ -14,6 +14,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import logging
|
import logging
|
||||||
import time
|
import time
|
||||||
|
from concurrent.futures import ThreadPoolExecutor
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
from fastapi import APIRouter, Depends, HTTPException, Request
|
||||||
from fastapi.responses import FileResponse, Response
|
from fastapi.responses import FileResponse, Response
|
||||||
@@ -23,7 +24,7 @@ from sqlalchemy.orm import Session
|
|||||||
|
|
||||||
from .. import grid, mail, quiet_hours
|
from .. import grid, mail, quiet_hours
|
||||||
from ..auth import get_server_settings, require_device
|
from ..auth import get_server_settings, require_device
|
||||||
from ..db import frame_locked, get_db
|
from ..db import SessionLocal, frame_locked, get_db
|
||||||
from ..firmware import firmware_path
|
from ..firmware import firmware_path
|
||||||
from ..global_actions import GLOBAL_ACTIONS
|
from ..global_actions import GLOBAL_ACTIONS
|
||||||
from ..image_pipeline import draw_widget_border, logical_render_size, render_panel, render_placeholder, resolve_border_color
|
from ..image_pipeline import draw_widget_border, logical_render_size, render_panel, render_placeholder, resolve_border_color
|
||||||
@@ -82,6 +83,40 @@ def _setup_placeholder(frame: Frame, request: Request, manage: dict | None = Non
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _render_one_widget(frame_id: int, widget_id: int, orientation: str, panel_w: int, panel_h: int,
|
||||||
|
cell: tuple[int, int, int, int], is_normal_wake: bool,
|
||||||
|
) -> tuple[tuple[int, int, int, int], object] | None:
|
||||||
|
"""Renders exactly one widget on its own DB session, so several of
|
||||||
|
these can run concurrently in a thread pool -- see app/db.py's
|
||||||
|
module docstring: handlers already run multi-threaded (sync
|
||||||
|
handlers in FastAPI's threadpool, one process), and frame_locked/
|
||||||
|
widget_locked's per-frame threading.Lock is what makes that safe,
|
||||||
|
not anything about which Session object is in play. A SQLAlchemy
|
||||||
|
Session itself is never safe to share across threads, so each
|
||||||
|
concurrent render gets a fresh one rather than reusing the
|
||||||
|
request's. Most of a widget's render time is spent waiting on an
|
||||||
|
external call (Immich, a weather provider, CalDAV) with the DB
|
||||||
|
untouched, which is exactly the time this buys back."""
|
||||||
|
db = SessionLocal()
|
||||||
|
try:
|
||||||
|
frame = db.get(Frame, frame_id)
|
||||||
|
widget = db.get(Widget, widget_id)
|
||||||
|
if widget is None:
|
||||||
|
return None # deleted between the listing query and this fetch -- skip it, not a 500
|
||||||
|
module = WIDGET_TYPES.get(widget.widget_type)
|
||||||
|
if module is None:
|
||||||
|
return None # unrecognized widget_type -- shouldn't happen, skip defensively rather than 500
|
||||||
|
px, py, pw, ph = grid.cell_to_pixels(orientation, panel_w, panel_h, cell)
|
||||||
|
img = module.render(db, frame, widget, pw, ph, is_normal_wake=is_normal_wake)
|
||||||
|
draw_widget_border(
|
||||||
|
img, widget.border_style, widget.border_thickness,
|
||||||
|
resolve_border_color(widget.border_color_index, frame.palette_rgb),
|
||||||
|
)
|
||||||
|
return (px, py, pw, ph), img
|
||||||
|
finally:
|
||||||
|
db.close()
|
||||||
|
|
||||||
|
|
||||||
def _render_widgets(db: Session, frame: Frame, manage: dict | None, is_normal_wake: bool,
|
def _render_widgets(db: Session, frame: Frame, manage: dict | None, is_normal_wake: bool,
|
||||||
as_png: bool = False, capture_snapshot: bool = False) -> bytes | tuple[bytes, bytes]:
|
as_png: bool = False, capture_snapshot: bool = False) -> bytes | tuple[bytes, bytes]:
|
||||||
"""The widget-system compositor: renders every widget on this frame
|
"""The widget-system compositor: renders every widget on this frame
|
||||||
@@ -92,25 +127,35 @@ def _render_widgets(db: Session, frame: Frame, manage: dict | None, is_normal_wa
|
|||||||
image_pipeline.render_panel for the single shared paste/enhance/
|
image_pipeline.render_panel for the single shared paste/enhance/
|
||||||
overlay/quantize/pack pass. Replaces the old per-mode RENDERERS
|
overlay/quantize/pack pass. Replaces the old per-mode RENDERERS
|
||||||
dict -- a frame can now show several widgets at once instead of
|
dict -- a frame can now show several widgets at once instead of
|
||||||
exactly one mode owning the whole panel."""
|
exactly one mode owning the whole panel.
|
||||||
|
|
||||||
|
Widgets render concurrently (_render_one_widget, each on its own DB
|
||||||
|
session) rather than one at a time -- a layout with several
|
||||||
|
network-backed widgets (photos, weather, calendar) previously paid
|
||||||
|
their fetch latency serially, which could push a single /frame/*
|
||||||
|
response past the firmware's fixed HTTP timeout and show a
|
||||||
|
misleading "server failed" status screen even though the server
|
||||||
|
was simply still working. Futures are submitted in sort_order and
|
||||||
|
collected in that same order (not completion order) -- overlapping
|
||||||
|
widgets must still paint in the original z-order."""
|
||||||
all_widgets = db.scalars(
|
all_widgets = db.scalars(
|
||||||
select(Widget).where(Widget.frame_id == frame.id).order_by(Widget.sort_order)
|
select(Widget).where(Widget.frame_id == frame.id).order_by(Widget.sort_order)
|
||||||
).all()
|
).all()
|
||||||
panel_w, panel_h = logical_render_size(frame.orientation)
|
panel_w, panel_h = logical_render_size(frame.orientation)
|
||||||
regions = []
|
regions = []
|
||||||
for widget in all_widgets:
|
if all_widgets:
|
||||||
module = WIDGET_TYPES.get(widget.widget_type)
|
with ThreadPoolExecutor(max_workers=min(len(all_widgets), 8)) as pool:
|
||||||
if module is None:
|
futures = [
|
||||||
continue # unrecognized widget_type -- shouldn't happen, skip defensively rather than 500
|
pool.submit(
|
||||||
px, py, pw, ph = grid.cell_to_pixels(
|
_render_one_widget, frame.id, widget.id, frame.orientation, panel_w, panel_h,
|
||||||
frame.orientation, panel_w, panel_h, (widget.x, widget.y, widget.w, widget.h)
|
(widget.x, widget.y, widget.w, widget.h), is_normal_wake,
|
||||||
)
|
)
|
||||||
img = module.render(db, frame, widget, pw, ph, is_normal_wake=is_normal_wake)
|
for widget in all_widgets
|
||||||
draw_widget_border(
|
]
|
||||||
img, widget.border_style, widget.border_thickness,
|
for future in futures:
|
||||||
resolve_border_color(widget.border_color_index, frame.palette_rgb),
|
result = future.result()
|
||||||
)
|
if result is not None:
|
||||||
regions.append(((px, py, pw, ph), img))
|
regions.append(result)
|
||||||
return render_panel(
|
return render_panel(
|
||||||
regions, orientation=frame.orientation, palette_rgb=frame.palette_rgb,
|
regions, orientation=frame.orientation, palette_rgb=frame.palette_rgb,
|
||||||
color_boost=frame.color_boost, contrast_boost=frame.contrast_boost,
|
color_boost=frame.color_boost, contrast_boost=frame.contrast_boost,
|
||||||
|
|||||||
@@ -144,3 +144,50 @@ def test_two_widget_frame_composites_both_and_next_targets_calendar(client, db_s
|
|||||||
photo_cfg = db_session.get(PhotoWidgetConfig, photo_widget.id)
|
photo_cfg = db_session.get(PhotoWidgetConfig, photo_widget.id)
|
||||||
assert cal_cfg.browse_offset == 1
|
assert cal_cfg.browse_offset == 1
|
||||||
assert photo_cfg.current_asset_id == "" # untouched -- NEXT was never bound to it
|
assert photo_cfg.current_asset_id == "" # untouched -- NEXT was never bound to it
|
||||||
|
|
||||||
|
|
||||||
|
def test_widgets_render_concurrently(client, db_session, monkeypatch):
|
||||||
|
"""Two independent, slow widgets on one frame should render in
|
||||||
|
roughly the time of the slowest one, not the sum -- the actual fix
|
||||||
|
for the "hold to cycle layouts times out and shows a false server-
|
||||||
|
failed status screen" bug: several network-backed widgets (photos,
|
||||||
|
weather, calendar) rendering one after another could push a single
|
||||||
|
/frame/* response past the firmware's fixed HTTP timeout even though
|
||||||
|
the server was simply still working."""
|
||||||
|
monkeypatch.setattr(widgets.photos, "immich_client_for", lambda frame: object())
|
||||||
|
monkeypatch.setattr(widgets.photos, "list_assets", lambda client, album_id: _ASSETS)
|
||||||
|
|
||||||
|
from PIL import Image
|
||||||
|
source = Image.new("RGB", (100, 80), (10, 20, 30))
|
||||||
|
|
||||||
|
def _slow_fetch(client, mode, asset_id):
|
||||||
|
time.sleep(0.25)
|
||||||
|
return source, None
|
||||||
|
|
||||||
|
monkeypatch.setattr(widgets.photos, "fetch_source_and_faces", _slow_fetch)
|
||||||
|
|
||||||
|
frame = Frame(
|
||||||
|
name="Concurrency Frame", device_id="112233445566", device_token="devtok-3",
|
||||||
|
manage_token="mtok-3", orientation="landscape", created_at=time.time(),
|
||||||
|
)
|
||||||
|
db_session.add(frame)
|
||||||
|
db_session.flush()
|
||||||
|
|
||||||
|
widget_a = Widget(frame_id=frame.id, widget_type="photos", x=0, y=0, w=4, h=5,
|
||||||
|
sort_order=0, created_at=time.time())
|
||||||
|
widget_b = Widget(frame_id=frame.id, widget_type="photos", x=4, y=0, w=4, h=5,
|
||||||
|
sort_order=1, created_at=time.time())
|
||||||
|
db_session.add_all([widget_a, widget_b])
|
||||||
|
db_session.flush()
|
||||||
|
db_session.add(PhotoWidgetConfig(widget_id=widget_a.id, album_id="album-a"))
|
||||||
|
db_session.add(PhotoWidgetConfig(widget_id=widget_b.id, album_id="album-b"))
|
||||||
|
db_session.commit()
|
||||||
|
|
||||||
|
start = time.monotonic()
|
||||||
|
resp = client.get(f"/frame/image?id={frame.device_id}&token={frame.device_token}")
|
||||||
|
elapsed = time.monotonic() - start
|
||||||
|
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert len(resp.content) == EXPECTED_BYTES
|
||||||
|
# Serial would be ~0.5s (2 x 0.25s); concurrent should land near 0.25s.
|
||||||
|
assert elapsed < 0.45, f"widgets rendered serially, not concurrently ({elapsed:.2f}s)"
|
||||||
|
|||||||
Reference in New Issue
Block a user