diff --git a/evidence/idle-frame-admission-20261006/README.md b/evidence/idle-frame-admission-20261006/README.md new file mode 100644 index 0000000000..a17289b3da --- /dev/null +++ b/evidence/idle-frame-admission-20261006/README.md @@ -0,0 +1,15 @@ +# Synchronous idle admission + +Screen previously prepared visible/background roots before its timer decision, then acquired them again for callbacks. Timer.resume only sets an asyncio.Event; callback admission had no internal await and only queues sender-owned callbacks. Neither dispatches another task during this synchronous interval. Damage-only idle admission and its callbacks now borrow one acquired tuple, including a legitimately empty tuple. Nothing is retained across events or publication. + +Callback admission is now synchronous. Idle supplies any roots it acquired; the existing timer call_next invocation acquires its own roots. These are the two native callers; no tracked override was found across native, Toad and Core. Arbitrary external dynamic callers remain unresolved by lexical AST. The private method is no longer awaitable; its one awaited native caller was migrated without an async wrapper or alias. + +The existing flags derive one _refresh_requested predicate used by idle, _refresh_pending and spatial sender checks. Required work or dirty widgets resume the existing timer without preparing roots merely for that decision. Damage-only work still acquires preparation effects, rechecks flags changed by preparation, and excludes held regions. Held-only damage keeps the timer quiet; required callback and publication preparation still request source work. + +Paint is a distinct acquisition: queued source, follow, scroll, layout and screen membership can change after idle returns. _compositor_refresh still acquires its exact roots for damage exclusion, background/inline rendering and _on_frame_published. No readiness cache, frame state, mask, queue, new timer or Toad edit. Preparation effects occur at required sender admission/publication rather than a timer decision whose answer is already known. + +AST before:249 native production/467 tests/288 Toad/324 Core modules, zero omissions. After native:249/zero. Existing native callback invocation supports sync and async functions. Twelve production lines replaced. + +Seven real framework controls PASS/.79s: held and empty cohorts, required-scroll timer progress without idle-only preparation, fresh paint acquisition, held damage/release, real run_async sender tasks, inline and translucent-background exclusion. One original source sidebar App PASS/empty stderr,508 widgets/10 tabs/no provider. Each profiled toggle makes2 preparations versus3 in the retained prior source App. Frame medians37.0/44.8ms versus38.2/39.1ms have varying tails: no reliable overall speedup or installed/live/physical scrolling claim. + +The retained-tool fixture was read, not executed: its private_native_wire requires a native package purpose outside this assignment. No package, SDK, provider, public or borrowed-prefix operation. RESULT.json binds original logs/profiles; accepted prior evidence stays unchanged. diff --git a/evidence/idle-frame-admission-20261006/RESULT.json b/evidence/idle-frame-admission-20261006/RESULT.json new file mode 100644 index 0000000000..c9e7e8d95c --- /dev/null +++ b/evidence/idle-frame-admission-20261006/RESULT.json @@ -0,0 +1,43 @@ +{ + "source": "9d9f55e6bbf672d12c7bb17ba2b62417df905956", + "controls": "7 passed in 0.79s", + "app_exit": 0, + "app_widgets": 508, + "app_tabs": 10, + "providers": 0, + "stderr_bytes": 0, + "metrics": { + "before": { + "channels": { + "calls": 3, + "inclusive_seconds": 0.001621484 + }, + "thread": { + "calls": 3, + "inclusive_seconds": 0.001407948 + } + }, + "after": { + "channels": { + "calls": 2, + "inclusive_seconds": 0.001502318 + }, + "thread": { + "calls": 2, + "inclusive_seconds": 0.000951152 + } + } + }, + "toad_source": "85eb8012c15229972269a38e3f505b9d480f5114", + "core_source": "0683ce64f7788d9f2db66ccb27e23de102c63ac4", + "keepers": { + "evidence/idle-frame-admission-20261006/before.json": "953e37b93c15951d6d69e65a86adf2725b9f3d90496d5f8b8bf60f932b268547", + "evidence/idle-frame-admission-20261006/after.json": "9c043511686ad161079c22000bdfef1f9499db76a72d8c32a588ec64410d6bf2", + "evidence/idle-frame-admission-20261006/controls.log": "b113dae2d14be4bc5bdc12a61ce44a1bf5882998a2a55d21b46bd14e16f72170", + "/home/ts/.cache/agent-scratch/idle-frame-admission-20261006/app.stdout.log": "85ec0f4b34609d41e0cb6acfe4d37ffd4d918ca15242ff3bb86f32f7ee250093", + "/home/ts/.cache/agent-scratch/idle-frame-admission-20261006/app.stderr.log": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "/home/ts/.cache/agent-scratch/idle-frame-admission-20261006/toggle-channels.pstats": "7510e1e19fc1669057727af9a770555d18f650fde4060090ac3950f0d5b94c01", + "/home/ts/.cache/agent-scratch/idle-frame-admission-20261006/toggle-thread.pstats": "d50ca260304e65deb5cd795d21af79d4284f8ace9187f78cd7b7bc00864d762c" + }, + "limits": "Source headless App, not installed/live scrolling; variable timings do not establish overall gain." +} diff --git a/evidence/idle-frame-admission-20261006/after.json b/evidence/idle-frame-admission-20261006/after.json new file mode 100644 index 0000000000..9f00dd9d61 --- /dev/null +++ b/evidence/idle-frame-admission-20261006/after.json @@ -0,0 +1,121 @@ +{ + "source": "9d9f55e6bbf672d12c7bb17ba2b62417df905956", + "parsed": 249, + "omissions": [], + "sites": [ + [ + "src/textual/app.py", + 5031, + "FunctionDef", + "_on_idle" + ], + [ + "src/textual/message_pump.py", + 489, + "screen._refresh_pending", + "Load" + ], + [ + "src/textual/screen.py", + 1179, + "FunctionDef", + "_refresh_requested" + ], + [ + "src/textual/screen.py", + 1189, + "FunctionDef", + "_refresh_pending" + ], + [ + "src/textual/screen.py", + 1195, + "AsyncFunctionDef", + "_on_idle" + ], + [ + "src/textual/screen.py", + 1227, + "FunctionDef", + "_prepare_visible_screens" + ], + [ + "src/textual/screen.py", + 1348, + "FunctionDef", + "_invoke_and_clear_callbacks" + ], + [ + "src/textual/screen.py", + 1214, + "self._invoke_and_clear_callbacks", + "Load" + ], + [ + "src/textual/screen.py", + 1244, + "self._prepare_visible_screens", + "Load" + ], + [ + "src/textual/screen.py", + 1339, + "screen._refresh_requested", + "Load" + ], + [ + "src/textual/screen.py", + 1192, + "self._refresh_requested", + "Load" + ], + [ + "src/textual/screen.py", + 1202, + "self._refresh_requested", + "Load" + ], + [ + "src/textual/screen.py", + 1328, + "self._invoke_and_clear_callbacks", + "Load" + ], + [ + "src/textual/screen.py", + 1206, + "self._refresh_requested", + "Load" + ], + [ + "src/textual/screen.py", + 1356, + "self._prepare_visible_screens", + "Load" + ], + [ + "src/textual/screen.py", + 1205, + "self._prepare_visible_screens", + "Load" + ], + [ + "src/textual/widget.py", + 4812, + "AsyncFunctionDef", + "_on_idle" + ], + [ + "src/textual/widgets/_data_table.py", + 1870, + "AsyncFunctionDef", + "_on_idle" + ], + [ + "src/textual/widgets/_tree.py", + 1248, + "AsyncFunctionDef", + "_on_idle" + ] + ] +} diff --git a/evidence/idle-frame-admission-20261006/before.json b/evidence/idle-frame-admission-20261006/before.json new file mode 100644 index 0000000000..9d8fd0efe0 --- /dev/null +++ b/evidence/idle-frame-admission-20261006/before.json @@ -0,0 +1,187 @@ +[ + { + "repo": "/home/ts/wt/textual-frame-publication-20261006", + "rev": "5a0c733349a33aff0826102ee40a92345a568361", + "root": "src/textual", + "parsed": 249, + "omissions": [], + "sites": [ + [ + "src/textual/app.py", + 5031, + "FunctionDef", + "_on_idle" + ], + [ + "src/textual/message_pump.py", + 489, + "screen._refresh_pending", + "Load" + ], + [ + "src/textual/screen.py", + 1179, + "FunctionDef", + "_refresh_pending" + ], + [ + "src/textual/screen.py", + 1190, + "AsyncFunctionDef", + "_on_idle" + ], + [ + "src/textual/screen.py", + 1207, + "FunctionDef", + "_prepare_compositor_refresh" + ], + [ + "src/textual/screen.py", + 1218, + "FunctionDef", + "_prepare_visible_screens" + ], + [ + "src/textual/screen.py", + 1340, + "AsyncFunctionDef", + "_invoke_and_clear_callbacks" + ], + [ + "src/textual/screen.py", + 1194, + "self._refresh_pending", + "Load" + ], + [ + "src/textual/screen.py", + 1235, + "self._prepare_visible_screens", + "Load" + ], + [ + "src/textual/screen.py", + 1205, + "self._invoke_and_clear_callbacks", + "Load" + ], + [ + "src/textual/screen.py", + 1319, + "self._invoke_and_clear_callbacks", + "Load" + ], + [ + "src/textual/screen.py", + 1221, + "screen._prepare_compositor_refresh", + "Load" + ], + [ + "src/textual/screen.py", + 1346, + "self._prepare_visible_screens", + "Load" + ], + [ + "src/textual/screen.py", + 1195, + "self._prepare_visible_screens", + "Load" + ], + [ + "src/textual/widget.py", + 4812, + "AsyncFunctionDef", + "_on_idle" + ], + [ + "src/textual/widgets/_data_table.py", + 1870, + "AsyncFunctionDef", + "_on_idle" + ], + [ + "src/textual/widgets/_tree.py", + 1248, + "AsyncFunctionDef", + "_on_idle" + ] + ] + }, + { + "repo": "/home/ts/wt/textual-frame-publication-20261006", + "rev": "5a0c733349a33aff0826102ee40a92345a568361", + "root": "tests", + "parsed": 467, + "omissions": [], + "sites": [ + [ + "tests/test_batch_paint_admission.py", + 17, + "FunctionDef", + "_prepare_compositor_refresh" + ], + [ + "tests/test_batch_paint_admission.py", + 154, + "FunctionDef", + "_prepare_compositor_refresh" + ], + [ + "tests/test_batch_paint_admission.py", + 106, + "FunctionDef", + "_prepare_compositor_refresh" + ], + [ + "tests/test_batch_paint_admission.py", + 19, + "super()._prepare_compositor_refresh", + "Load" + ], + [ + "tests/test_batch_paint_admission.py", + 108, + "super()._prepare_compositor_refresh", + "Load" + ], + [ + "tests/test_refresh_callback_owner.py", + 75, + "FunctionDef", + "_prepare_compositor_refresh" + ], + [ + "tests/test_refresh_callback_owner.py", + 96, + "screen._prepare_compositor_refresh", + "Load" + ] + ] + }, + { + "repo": "/home/ts/wt/toad-sidebar-context-pointer-20261006", + "rev": "85eb8012c15229972269a38e3f505b9d480f5114", + "root": "src/toad", + "parsed": 288, + "omissions": [], + "sites": [ + [ + "src/toad/screens/workspace.py", + 160, + "FunctionDef", + "_prepare_compositor_refresh" + ] + ] + }, + { + "repo": "/home/ts/wt/comms-cleanup-live-integration-20260929", + "rev": "0683ce64f7788d9f2db66ccb27e23de102c63ac4", + "root": "src/agent_comms", + "parsed": 324, + "omissions": [], + "sites": [] + } +] diff --git a/evidence/idle-frame-admission-20261006/controls.log b/evidence/idle-frame-admission-20261006/controls.log new file mode 100644 index 0000000000..e25f8c3911 --- /dev/null +++ b/evidence/idle-frame-admission-20261006/controls.log @@ -0,0 +1,2 @@ +....... [100%] +7 passed in 0.79s diff --git a/src/textual/screen.py b/src/textual/screen.py index b59a14f5d4..fdfbe86b4f 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -1176,25 +1176,34 @@ def _on_mount(self, event: events.Mount) -> None: ) @property - def _refresh_pending(self) -> bool: - """Whether the existing scene owners still have an unpainted update.""" + def _refresh_requested(self) -> bool: + """Whether pending screen work requires the timer regardless of damage.""" return bool( self._layout_required or self._scroll_required or self._repaint_required or self._recompose_required - or self._dirty_widgets - or self._compositor._dirty_regions + ) + + @property + def _refresh_pending(self) -> bool: + """Whether the existing scene owners still have an unpainted update.""" + return bool( + self._refresh_requested or self._dirty_widgets or self._compositor._dirty_regions ) async def _on_idle(self, event: events.Idle) -> None: # Check for any widgets marked as 'dirty' (needs a repaint) event.prevent_default() + # Borrow only within this synchronous admission. Paint reacquires its + # own cohort after queued source/layout work has had a chance to run. + roots: tuple[Widget, ...] | None = None if not self.app._batch_count and self.is_current: - if self._refresh_pending: + if self._refresh_requested or self._dirty_widgets: + self._update_timer.resume() + elif self._compositor._dirty_regions: roots = tuple(root for _, roots in self._prepare_visible_screens() for root in roots) - if (self._layout_required or self._scroll_required or self._repaint_required - or self._recompose_required or self._dirty_widgets + if (self._refresh_requested or self._dirty_widgets or self._compositor._exclude_regions( self._compositor._dirty_regions, tuple(region for root in roots @@ -1202,7 +1211,7 @@ async def _on_idle(self, event: events.Idle) -> None: )): self._update_timer.resume() - await self._invoke_and_clear_callbacks() + self._invoke_and_clear_callbacks(roots) def _prepare_compositor_refresh(self) -> tuple[Widget, ...]: """Prepare the frame and return original subtrees whose paint is held.""" @@ -1327,8 +1336,7 @@ def _sender_refresh_pending(self, sender: Widget, roots: tuple[Widget, ...]) -> return True owner = sender.screen for screen in (self, owner) if owner is not self else (self,): - if (screen._layout_required or screen._scroll_required or screen._recompose_required - or screen._repaint_required): + if screen._refresh_requested: return True if any(sender in widget.walk_ancestors(with_self=True) for widget in screen._dirty_widgets): @@ -1337,13 +1345,15 @@ def _sender_refresh_pending(self, sender: Widget, roots: tuple[Widget, ...]) -> return True return False - async def _invoke_and_clear_callbacks(self) -> None: + def _invoke_and_clear_callbacks(self, roots: tuple[Widget, ...] | None = None) -> None: """Admit painted senders back to their own original message pumps.""" if self.app._batch_count or not self._callbacks: return # No callback runs during this synchronous admission. Borrow one scene # cohort, then let each sender's existing callback handler own execution. - roots = tuple(root for _, roots in self._prepare_visible_screens() for root in roots) if self.is_current else () + # None means not acquired; an empty borrowed tuple is a valid cohort. + if roots is None: + roots = tuple(root for _, roots in self._prepare_visible_screens() for root in roots) if self.is_current else () index = 0 for _ in range(len(self._callbacks)): if self.app._batch_count or index >= len(self._callbacks): diff --git a/tests/test_batch_paint_admission.py b/tests/test_batch_paint_admission.py index 24051ac511..be4df66c94 100644 --- a/tests/test_batch_paint_admission.py +++ b/tests/test_batch_paint_admission.py @@ -1,6 +1,9 @@ """Real framework lifecycle checks, not installed terminal acceptance.""" import asyncio +from contextlib import nullcontext + +import pytest from textual.app import App, ComposeResult from textual.containers import Vertical @@ -174,6 +177,74 @@ def compose(self): yield Label("SIDEBAR_OLD", id="sidebar") +@pytest.mark.parametrize("held", [False, True]) +async def test_idle_shares_acquired_roots_and_paint_reacquires(held): + from textual import events + + class IdleScreen(IndependentScreen): + def __init__(self): + super().__init__() + self.acquisitions = [] + + def _prepare_compositor_refresh(self): + roots = super()._prepare_compositor_refresh() + self.acquisitions.append(roots) + return roots + + class IdleApp(IndependentApp): + def get_default_screen(self): + return IdleScreen() + + app = IdleApp() + completed = asyncio.Event() + async with app.run_test(size=(40, 8)) as pilot: + await pilot.pause() + screen = app.screen + compositor = screen._compositor + holder = app.query_one(HeldVertical) + sidebar = app.query_one("#sidebar") + + # A pending scroll already requires the timer. Preparation belongs to + # actual publication, rather than this timer-resume decision. + assert not screen._callbacks and not screen._refresh_pending + screen._scroll_required = True + screen._update_timer.pause() + before = len(screen.acquisitions) + await screen._on_idle(events.Idle()) + assert len(screen.acquisitions) == before + assert screen._update_timer._active.is_set() + screen._on_timer_update() + assert len(screen.acquisitions) > before + await pilot.pause() + + async with (holder.lock if held else nullcontext()): + assert not screen._refresh_pending + compositor._dirty_regions.add((holder if held else sidebar).region) + screen._invoke_later(completed.set, app) + screen._update_timer.pause() + before = len(screen.acquisitions) + await screen._on_idle(events.Idle()) + assert len(screen.acquisitions) == before + 1 + assert screen.acquisitions[-1] == ((holder,) if held else ()) + assert screen._update_timer._active.is_set() is not held + assert not completed.is_set() + assert any(sender is app for _, sender in screen._callbacks) + + # Publication is a distinct acquisition, including when the idle + # cohort was legitimately empty. Held-only damage is not consumed. + screen._compositor_refresh() + assert len(screen.acquisitions) == before + 2 + if held: + assert compositor._dirty_regions + assert not completed.is_set() + else: + assert not compositor._dirty_regions + + holder.refresh() + await asyncio.wait_for(completed.wait(), 2) + assert not compositor._dirty_regions + + async def test_partial_publication_keeps_geometry_and_owner_callbacks(): from textual._compositor import ChopsUpdate from textual.geometry import Region