Skip to content

Commit bac9955

Browse files
authored
Merge pull request #69 from MHFU-ModKit/studio/stalls
perf(studio): no findings stall every few seconds
2 parents 625d36a + 7e52007 commit bac9955

6 files changed

Lines changed: 181 additions & 7 deletions

File tree

‎apps/studio/src/mhfu_studio/monster/clips.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
1313

1414
from __future__ import annotations
1515

16+
import functools
1617
import hashlib
1718
import re
1819
from collections.abc import Callable, Iterable, Mapping
1920
from dataclasses import dataclass, field
21+
from types import MappingProxyType
2022

2123
from mhfu_port import behaviour, motion, slots
2224
from mhfu_port import layout as layouts
@@ -77,8 +79,11 @@ def clip_table(anim: AnimPack) -> dict[int, Fingerprint]:
7779
return {s: (frames, bool(loop)) for s, (frames, loop) in slots.slot_prints(anim).items()}
7880

7981

80-
def pac_clip_table(pac: bytes) -> dict[int, Fingerprint]:
81-
return clip_table(slots.anim_of(pac))
82+
@functools.lru_cache(maxsize=2)
83+
def pac_clip_table(pac: bytes) -> Mapping[int, Fingerprint]:
84+
"""`clip_table` of a built PAC, parsed once per PAC: the findings ask every few seconds.
85+
Read-only, as the table is shared by every caller."""
86+
return MappingProxyType(clip_table(slots.anim_of(pac)))
8287

8388

8489
def source_clip_table(moveset: bytes) -> dict[int, Fingerprint]:

‎apps/studio/src/mhfu_studio/monster/validate.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ def _pac(m: Manifest, pac: bytes | None, sources: Mapping[int, int]) -> list[Fin
215215
return []
216216
out = []
217217
try:
218-
table: dict[int, clips.Fingerprint] | None = clips.pac_clip_table(pac)
218+
table: Mapping[int, clips.Fingerprint] | None = clips.pac_clip_table(pac)
219219
except ValueError as e:
220220
out.append(_f("error", "PAC_UNREADABLE", "", f"no clip table in the PAC: {e}"))
221221
table = None
@@ -232,7 +232,7 @@ def _pac(m: Manifest, pac: bytes | None, sources: Mapping[int, int]) -> list[Fin
232232

233233

234234
def _clips(
235-
m: Manifest, table: dict[int, clips.Fingerprint], sources: Mapping[int, int]
235+
m: Manifest, table: Mapping[int, clips.Fingerprint], sources: Mapping[int, int]
236236
) -> list[Finding]:
237237
out = []
238238
moved = {

‎apps/studio/src/mhfu_studio/monster/workspace.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
import dataclasses
1414
import tomllib
15-
from collections.abc import Callable, Hashable, Sequence
15+
from collections.abc import Callable, Hashable, Mapping, Sequence
1616
from concurrent.futures import Future, ThreadPoolExecutor
1717
from contextlib import AbstractContextManager
1818
from dataclasses import dataclass
@@ -955,7 +955,7 @@ def attack_session(self) -> AttackSession | None:
955955

956956
# clips
957957

958-
def clip_table(self) -> dict[int, clips.Fingerprint]:
958+
def clip_table(self) -> Mapping[int, clips.Fingerprint]:
959959
"""The open build's fingerprints."""
960960
sc = self.scene
961961
if sc is None:

‎apps/studio/src/mhfu_studio/shell/studio.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@
3333
from mhfu_studio.shell.viewport import Viewport
3434

3535

36+
SLOW_COST, SLOW_EVERY = 0.05, 60.0
37+
"""Seconds: a check dearer than the first is not repeated on its own for the second, since a stall
38+
is the check's whole length and the interval's share of the time does not bound that. A change
39+
(`Findings.stale`) re-checks at once."""
40+
41+
3642
class Findings:
3743
"""The active document's findings, re-checked on demand and, when `auto`, now and then.
3844
@@ -55,7 +61,8 @@ def get(self, doc: Any, now: float | None = None) -> list[Finding]:
5561
if doc is None:
5662
self.found, self._doc = [], None
5763
return self.found
58-
due = self.auto and now - self._at >= max(1.0, 10 * self._cost)
64+
wait = SLOW_EVERY if self._cost > SLOW_COST else max(1.0, 10 * self._cost)
65+
due = self.auto and now - self._at >= wait
5966
if self._stale or doc is not self._doc or due:
6067
t0 = time.monotonic()
6168
try:

‎apps/studio/tests/monster/test_clips.py‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,3 +237,52 @@ def test_zinogre(games, built, ports):
237237
def test_brute(built):
238238
table = C.pac_clip_table(built("brute_tigrex"))
239239
assert len(table) == 77 and table[61] == (382, False) and 24 not in table
240+
241+
242+
@pytest.fixture
243+
def parses(monkeypatch):
244+
"""The PACs `pac_clip_table` parsed, with a table that says which."""
245+
seen: list[bytes] = []
246+
monkeypatch.setattr(slots, "anim_of", lambda pac: seen.append(pac) or pac)
247+
monkeypatch.setattr(C, "clip_table", lambda pack: {1: (len(pack), True)})
248+
C.pac_clip_table.cache_clear()
249+
yield seen
250+
C.pac_clip_table.cache_clear()
251+
252+
253+
def test_a_pac_is_parsed_once_whoever_asks(parses):
254+
pac = b"a pac"
255+
first = C.pac_clip_table(pac)
256+
assert C.pac_clip_table(pac) is first and C.pac_clip_table(bytes(bytearray(pac))) is first
257+
assert parses == [pac]
258+
259+
260+
def test_other_bytes_are_another_table(parses):
261+
one, two = C.pac_clip_table(b"one"), C.pac_clip_table(b"three")
262+
assert one is not two and dict(one) == {1: (3, True)} and dict(two) == {1: (5, True)}
263+
assert parses == [b"one", b"three"]
264+
265+
266+
def test_the_table_is_shared_so_no_caller_can_change_it(parses):
267+
table = C.pac_clip_table(b"pac")
268+
with pytest.raises(TypeError):
269+
table[2] = (1, False)
270+
with pytest.raises(TypeError):
271+
del table[1]
272+
assert dict(C.pac_clip_table(b"pac")) == {1: (3, True)}
273+
274+
275+
def test_an_unreadable_pac_is_not_remembered(monkeypatch):
276+
tries = []
277+
278+
def broken(pac):
279+
tries.append(pac)
280+
raise ValueError("no clips")
281+
282+
monkeypatch.setattr(slots, "anim_of", broken)
283+
C.pac_clip_table.cache_clear()
284+
for _ in range(2):
285+
with pytest.raises(ValueError, match="no clips"):
286+
C.pac_clip_table(b"bad")
287+
assert len(tries) == 2
288+
C.pac_clip_table.cache_clear()
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# SPDX-License-Identifier: GPL-3.0-or-later
2+
# SPDX-FileCopyrightText: 2026 sp00ktober
3+
"""The findings' own clock: how often a re-check runs, and what a slow one does to it."""
4+
5+
from __future__ import annotations
6+
7+
import pytest
8+
from mhfu_studio.shell import studio
9+
from mhfu_studio.shell.findings import Finding
10+
from mhfu_studio.shell.studio import SLOW_COST, SLOW_EVERY, Findings
11+
12+
13+
class Clock:
14+
"""`time.monotonic`, moved by the checks themselves."""
15+
16+
def __init__(self) -> None:
17+
self.t = 0.0
18+
19+
def __call__(self) -> float:
20+
return self.t
21+
22+
23+
class Doc:
24+
"""A document whose check costs `cost` seconds on `clock`."""
25+
26+
def __init__(self, clock: Clock, cost: float) -> None:
27+
self.clock, self.cost, self.calls = clock, cost, 0
28+
29+
def findings(self) -> list[Finding]:
30+
self.calls += 1
31+
self.clock.t += self.cost
32+
return []
33+
34+
35+
@pytest.fixture
36+
def clock(monkeypatch: pytest.MonkeyPatch) -> Clock:
37+
c = Clock()
38+
monkeypatch.setattr(studio.time, "monotonic", c)
39+
return c
40+
41+
42+
def run(fs: Findings, doc: Doc, secs: float, step: float = 0.25) -> list[float]:
43+
"""The times in the next `secs` seconds (the window ticks every `step`) a check ran."""
44+
ran, t = [], 0.0
45+
while t <= secs:
46+
before = doc.calls
47+
fs.get(doc, now=t)
48+
if doc.calls > before:
49+
ran.append(t)
50+
t += step
51+
return ran
52+
53+
54+
def test_a_cheap_check_repeats_every_second(clock: Clock) -> None:
55+
doc = Doc(clock, 0.005)
56+
assert run(Findings(), doc, 5) == [0.0, 1.0, 2.0, 3.0, 4.0, 5.0]
57+
58+
59+
def test_a_check_dearer_than_a_tenth_of_its_interval_waits_longer_as_before(clock: Clock) -> None:
60+
doc = Doc(clock, 0.04)
61+
assert SLOW_COST > 0.04 and run(Findings(), doc, 3) == [0.0, 1.0, 2.0, 3.0]
62+
63+
64+
def test_a_slow_check_is_not_repeated_for_a_minute(clock: Clock) -> None:
65+
doc = Doc(clock, 0.4)
66+
assert SLOW_COST < 0.4
67+
ran = run(Findings(), doc, 130)
68+
assert ran == [0.0, SLOW_EVERY, 2 * SLOW_EVERY]
69+
70+
71+
def test_a_change_still_re_checks_at_once_however_slow_the_last_was(clock: Clock) -> None:
72+
fs, doc = Findings(), Doc(clock, 0.4)
73+
fs.get(doc, now=0.0)
74+
fs.get(doc, now=10.0)
75+
assert doc.calls == 1
76+
fs.stale()
77+
fs.get(doc, now=10.25)
78+
assert doc.calls == 2
79+
80+
81+
def test_a_cheap_check_after_a_slow_one_brings_the_interval_back(clock: Clock) -> None:
82+
fs, doc = Findings(), Doc(clock, 0.4)
83+
fs.get(doc, now=0.0)
84+
doc.cost = 0.005 # the cache is warm
85+
fs.stale()
86+
fs.get(doc, now=1.0)
87+
assert doc.calls == 2
88+
fs.get(doc, now=1.5)
89+
fs.get(doc, now=2.0)
90+
assert doc.calls == 3
91+
92+
93+
def test_another_document_is_checked_at_once(clock: Clock) -> None:
94+
fs = Findings()
95+
fs.get(Doc(clock, 0.4), now=0.0)
96+
other = Doc(clock, 0.4)
97+
fs.get(other, now=1.0)
98+
assert other.calls == 1
99+
100+
101+
def test_a_check_that_fails_says_so_and_a_slow_failure_backs_off_too(clock: Clock) -> None:
102+
class Bad(Doc):
103+
def findings(self) -> list[Finding]:
104+
self.calls += 1
105+
self.clock.t += self.cost
106+
raise ValueError("no")
107+
108+
fs, bad = Findings(), Bad(clock, 0.4)
109+
assert [f.code for f in fs.get(bad, now=0.0)] == ["check-failed"]
110+
fs.get(bad, now=5.0)
111+
assert bad.calls == 1
112+
fs.get(bad, now=SLOW_EVERY)
113+
assert bad.calls == 2

0 commit comments

Comments
 (0)