diff --git a/forgetest/forgetest/catalog.py b/forgetest/forgetest/catalog.py index 4fcf183..0a5132b 100644 --- a/forgetest/forgetest/catalog.py +++ b/forgetest/forgetest/catalog.py @@ -75,8 +75,12 @@ class Test: invalidates the tests of that module. A test the module does not define in the @test form hashes its whole file.""" if self._source_sha is None: - path = inspect.getsourcefile(self.fn) or inspect.getfile(self.fn) - self._source_sha = implementation_sha(path, self.id) + # One filler at a time: a second poll that arrives while the + # first computes waits for it rather than repeating the work. + with _IMPL_LOCK: + if self._source_sha is None: + path = inspect.getsourcefile(self.fn) or inspect.getfile(self.fn) + self._source_sha = implementation_sha(path, self.id) return self._source_sha def fingerprint(self, manifest): @@ -140,8 +144,38 @@ def source_file_sha(path): return hashlib.sha256(data).hexdigest() -_PARTS = {} # path -> (shared sha, {test id: own sha}); the files never change under a run -_PARTS_LOCK = threading.Lock() +# The files never change under a run, so each is read and parsed once: +# path -> (text, tree or None), path -> (shared sha, {test id: own sha}), +# path -> the sibling modules it imports itself. +_PARSED = {} +_PARTS = {} +_SIBLINGS = {} +_PARTS_LOCK = threading.RLock() +_IMPL_LOCK = threading.Lock() + + +def forget(path): + """Drop what is known about a module: its file changed (the unit tests + edit their modules between readings).""" + with _PARTS_LOCK: + for cache in (_PARSED, _PARTS, _SIBLINGS): + cache.pop(path, None) + + +def _parsed(path): + """(text, tree) of a suite module, line endings normalized; the tree + is None when the file does not parse.""" + with _PARTS_LOCK: + hit = _PARSED.get(path) + if hit is None: + with open(path, "rb") as f: + text = f.read().replace(b"\r\n", b"\n").decode("utf-8") + try: + tree = ast.parse(text) + except SyntaxError: + tree = None + hit = _PARSED[path] = (text, tree) + return hit def module_parts(path): @@ -154,14 +188,9 @@ def module_parts(path): hit = _PARTS.get(path) if hit is not None: return hit - with open(path, "rb") as f: - text = f.read().replace(b"\r\n", b"\n").decode("utf-8") + text, tree = _parsed(path) lines = text.split("\n") spans = {} - try: - tree = ast.parse(text) - except SyntaxError: - tree = None for node in (tree.body if tree is not None else []): if not isinstance(node, ast.FunctionDef): continue @@ -188,22 +217,30 @@ def sibling_imports(path, seen=None): test's fingerprint.""" if seen is None: seen = [] - with open(path, "rb") as f: - text = f.read().replace(b"\r\n", b"\n").decode("utf-8") - try: - tree = ast.parse(text) - except SyntaxError: - return seen - here = os.path.dirname(path) - for node in tree.body: - if isinstance(node, ast.ImportFrom) and node.level == 1 and node.module: - sib = os.path.join(here, node.module.replace(".", os.sep) + ".py") - if os.path.isfile(sib) and sib not in seen and sib != path: - seen.append(sib) - sibling_imports(sib, seen) + for sib in _direct_siblings(path): + if sib not in seen and sib != path: + seen.append(sib) + sibling_imports(sib, seen) return seen +def _direct_siblings(path): + """The suite modules one module imports itself, in import order.""" + with _PARTS_LOCK: + hit = _SIBLINGS.get(path) + if hit is None: + _text, tree = _parsed(path) + here = os.path.dirname(path) + hit = [] + for node in (tree.body if tree is not None else []): + if isinstance(node, ast.ImportFrom) and node.level == 1 and node.module: + sib = os.path.join(here, node.module.replace(".", os.sep) + ".py") + if os.path.isfile(sib): + hit.append(sib) + _SIBLINGS[path] = hit + return hit + + def implementation_sha(path, test_id): """The implementation hash of one test (see Test.source_sha): the test's own text, the shared text of its module, and the shared text diff --git a/forgetest/tests/test_manifest.py b/forgetest/tests/test_manifest.py index 068c96f..7d11b3d 100644 --- a/forgetest/tests/test_manifest.py +++ b/forgetest/tests/test_manifest.py @@ -244,7 +244,7 @@ def two(ctx): shutil.rmtree(self.tmp, ignore_errors=True) def write(self, text): - catalog._PARTS.pop(self.path, None) + catalog.forget(self.path) with open(self.path, "w", newline="\n") as f: f.write(text) @@ -279,12 +279,12 @@ def two(ctx): sib = os.path.join(self.tmp, "judge.py") with open(sib, "w", newline="\n") as f: f.write("def judge(x):\n return x > 1\n") - catalog._PARTS.pop(sib, None) + catalog.forget(sib) self.write("from .judge import judge\n" + self.MODULE) a1, b1 = self.shas() with open(sib, "w", newline="\n") as f: f.write("def judge(x):\n return x > 2\n") - catalog._PARTS.pop(sib, None) + catalog.forget(sib) a2, b2 = self.shas() self.assertNotEqual(a1, a2) self.assertNotEqual(b1, b2) @@ -296,7 +296,7 @@ def two(ctx): def test_line_endings_do_not_count(self): a1, b1 = self.shas() - catalog._PARTS.pop(self.path, None) + catalog.forget(self.path) with open(self.path, "wb") as f: f.write(self.MODULE.replace("\n", "\r\n").encode()) self.assertEqual((a1, b1), self.shas()) diff --git a/forgetest/tests/test_responsiveness.py b/forgetest/tests/test_responsiveness.py index 9dc091b..a79b777 100644 --- a/forgetest/tests/test_responsiveness.py +++ b/forgetest/tests/test_responsiveness.py @@ -5,12 +5,16 @@ """What keeps the page answering the operator instead of the timer. -Two things went wrong on the bench and are pinned here: +Three things went wrong on the bench and are pinned here: - the state cost. Every poll re-read and re-parsed the whole result log, and recomputed every test's domain fingerprint. A result record carries its run log, so the file reaches megabytes over a campaign and the poll grew with it. Both are now parsed and computed once. + - the first state's cost. Hashing the implementations parsed a test's + module and every module it imports once per test, and each poll the + page timed out on started the same work again beside the first. Each + module is now parsed once, and a poll waits for the work in progress. - the wasted payload. An idle page polls an unchanged state; it now gets a 304 instead of the whole thing. @@ -171,6 +175,63 @@ class FingerprintCacheTests(unittest.TestCase): self.assertIsNone(self.man.files("no-such-component")) +class ImplementationCostTests(unittest.TestCase): + """The first state after a start hashes every test's implementation. + It parses each suite module once, and a poll that arrives while it + runs waits for it: on the board a parse per test took minutes, and + the page's timed-out polls each started the work again beside it.""" + + def test_each_suite_module_is_parsed_once(self): + import ast + import inspect + reg = catalog.load_suite() + paths = sorted({inspect.getsourcefile(t.fn) for t in catalog.all_tests(reg)}) + for p in os.listdir(catalog.suite_dir()): + catalog.forget(os.path.join(catalog.suite_dir(), p)) + for p in paths: + catalog.forget(p) + real_parse, parsed = ast.parse, [] + + def counting_parse(source, *a, **kw): + parsed.append(source[:40]) + return real_parse(source, *a, **kw) + + ast.parse = counting_parse + try: + for t in catalog.all_tests(reg): + catalog.implementation_sha(inspect.getsourcefile(t.fn), t.id) + finally: + ast.parse = real_parse + modules = [p for p in os.listdir(catalog.suite_dir()) if p.endswith(".py")] + self.assertLessEqual(len(parsed), len(modules), + "%d parses for %d suite modules" % (len(parsed), len(modules))) + + def test_a_concurrent_fill_waits_instead_of_repeating(self): + import threading + import time + t = helpers.make_test("fake.impl", [("forgectrl", "src/ui.c")], fn=t_noop) + real, calls = catalog.implementation_sha, [] + + def slow(path, test_id): + calls.append(test_id) + time.sleep(0.3) + return real(path, test_id) + + catalog.implementation_sha = slow + try: + got = [] + threads = [threading.Thread(target=lambda: got.append(t.source_sha)) for _ in range(3)] + for th in threads: + th.start() + for th in threads: + th.join(10) + finally: + catalog.implementation_sha = real + self.assertEqual(calls, ["fake.impl"], "the implementation hash was computed %d times" % len(calls)) + self.assertEqual(len(set(got)), 1) + self.assertEqual(len(got), 3) + + class PageTests(unittest.TestCase): def test_page_never_rebuilds_what_the_operator_may_be_pressing(self): """A poll must update rows, prompt buttons and tool entries in