mirror of
https://github.com/openglow-org/forgefirm.git
synced 2026-09-28 01:01:12 -07:00
forgetest: the first /state parses each suite module once
The first GET /state after forgetest starts hashes every test's implementation, and it took from 81 s to more than 12 minutes on the bench reference. Two causes: - sibling_imports() read and parsed a test's module and every sibling it imports, transitively, once per test: 280 ast.parse calls for 26 suite modules. module_parts() kept its own parse, sibling_imports() kept none. - The page gives up on a poll after 20 s and polls again; the server thread goes on computing. Each new poll started the same cold work beside the first, all of it on the one CPU, so the longer the first took the more copies ran. That is the spread between 81 s and 12 minutes. Each module's text and tree are now read and parsed once and kept, as are its direct sibling imports, and the implementation hash is filled by one thread at a time: a poll that arrives while it is computed waits for it instead of repeating it. catalog.forget(path) drops what is kept about a file, for the unit tests that edit their modules. The hashes do not move: every test's implementation hash and domain fingerprint on the image manifest of 20260923220034 is byte-identical before and after, so no result is invalidated. On the host the cold computation went from 7.00 s to 0.29 s (280 parses to 25). On the bench reference (the file bind-mounted on image 20260923220034, the page open) the first /state answered in 8 to 12 s after a restart, where the same restart earlier in the evening had not answered after 7 minutes. tests/test_responsiveness.py pins both: each suite module parsed at most once while every test's implementation hash is computed, and three threads reading one test's hash compute it once. With the old behavior put back by a patch both fail (280 parses for 26 modules; the hash computed 3 times). forgetest's unit tests 454 OK. forgetest is the dev-only harness, outside the catalog's coverage; its catalog consequence is none, since no fingerprint moves.
This commit is contained in:
@@ -74,6 +74,10 @@ class Test:
|
|||||||
that test's earlier passes and no other; a change to a helper
|
that test's earlier passes and no other; a change to a helper
|
||||||
invalidates the tests of that module. A test the module does not
|
invalidates the tests of that module. A test the module does not
|
||||||
define in the @test form hashes its whole file."""
|
define in the @test form hashes its whole file."""
|
||||||
|
if self._source_sha is None:
|
||||||
|
# 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:
|
if self._source_sha is None:
|
||||||
path = inspect.getsourcefile(self.fn) or inspect.getfile(self.fn)
|
path = inspect.getsourcefile(self.fn) or inspect.getfile(self.fn)
|
||||||
self._source_sha = implementation_sha(path, self.id)
|
self._source_sha = implementation_sha(path, self.id)
|
||||||
@@ -140,8 +144,38 @@ def source_file_sha(path):
|
|||||||
return hashlib.sha256(data).hexdigest()
|
return hashlib.sha256(data).hexdigest()
|
||||||
|
|
||||||
|
|
||||||
_PARTS = {} # path -> (shared sha, {test id: own sha}); the files never change under a run
|
# The files never change under a run, so each is read and parsed once:
|
||||||
_PARTS_LOCK = threading.Lock()
|
# 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):
|
def module_parts(path):
|
||||||
@@ -154,14 +188,9 @@ def module_parts(path):
|
|||||||
hit = _PARTS.get(path)
|
hit = _PARTS.get(path)
|
||||||
if hit is not None:
|
if hit is not None:
|
||||||
return hit
|
return hit
|
||||||
with open(path, "rb") as f:
|
text, tree = _parsed(path)
|
||||||
text = f.read().replace(b"\r\n", b"\n").decode("utf-8")
|
|
||||||
lines = text.split("\n")
|
lines = text.split("\n")
|
||||||
spans = {}
|
spans = {}
|
||||||
try:
|
|
||||||
tree = ast.parse(text)
|
|
||||||
except SyntaxError:
|
|
||||||
tree = None
|
|
||||||
for node in (tree.body if tree is not None else []):
|
for node in (tree.body if tree is not None else []):
|
||||||
if not isinstance(node, ast.FunctionDef):
|
if not isinstance(node, ast.FunctionDef):
|
||||||
continue
|
continue
|
||||||
@@ -188,22 +217,30 @@ def sibling_imports(path, seen=None):
|
|||||||
test's fingerprint."""
|
test's fingerprint."""
|
||||||
if seen is None:
|
if seen is None:
|
||||||
seen = []
|
seen = []
|
||||||
with open(path, "rb") as f:
|
for sib in _direct_siblings(path):
|
||||||
text = f.read().replace(b"\r\n", b"\n").decode("utf-8")
|
if sib not in seen and sib != path:
|
||||||
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)
|
seen.append(sib)
|
||||||
sibling_imports(sib, seen)
|
sibling_imports(sib, seen)
|
||||||
return 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):
|
def implementation_sha(path, test_id):
|
||||||
"""The implementation hash of one test (see Test.source_sha): the
|
"""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
|
test's own text, the shared text of its module, and the shared text
|
||||||
|
|||||||
@@ -244,7 +244,7 @@ def two(ctx):
|
|||||||
shutil.rmtree(self.tmp, ignore_errors=True)
|
shutil.rmtree(self.tmp, ignore_errors=True)
|
||||||
|
|
||||||
def write(self, text):
|
def write(self, text):
|
||||||
catalog._PARTS.pop(self.path, None)
|
catalog.forget(self.path)
|
||||||
with open(self.path, "w", newline="\n") as f:
|
with open(self.path, "w", newline="\n") as f:
|
||||||
f.write(text)
|
f.write(text)
|
||||||
|
|
||||||
@@ -279,12 +279,12 @@ def two(ctx):
|
|||||||
sib = os.path.join(self.tmp, "judge.py")
|
sib = os.path.join(self.tmp, "judge.py")
|
||||||
with open(sib, "w", newline="\n") as f:
|
with open(sib, "w", newline="\n") as f:
|
||||||
f.write("def judge(x):\n return x > 1\n")
|
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)
|
self.write("from .judge import judge\n" + self.MODULE)
|
||||||
a1, b1 = self.shas()
|
a1, b1 = self.shas()
|
||||||
with open(sib, "w", newline="\n") as f:
|
with open(sib, "w", newline="\n") as f:
|
||||||
f.write("def judge(x):\n return x > 2\n")
|
f.write("def judge(x):\n return x > 2\n")
|
||||||
catalog._PARTS.pop(sib, None)
|
catalog.forget(sib)
|
||||||
a2, b2 = self.shas()
|
a2, b2 = self.shas()
|
||||||
self.assertNotEqual(a1, a2)
|
self.assertNotEqual(a1, a2)
|
||||||
self.assertNotEqual(b1, b2)
|
self.assertNotEqual(b1, b2)
|
||||||
@@ -296,7 +296,7 @@ def two(ctx):
|
|||||||
|
|
||||||
def test_line_endings_do_not_count(self):
|
def test_line_endings_do_not_count(self):
|
||||||
a1, b1 = self.shas()
|
a1, b1 = self.shas()
|
||||||
catalog._PARTS.pop(self.path, None)
|
catalog.forget(self.path)
|
||||||
with open(self.path, "wb") as f:
|
with open(self.path, "wb") as f:
|
||||||
f.write(self.MODULE.replace("\n", "\r\n").encode())
|
f.write(self.MODULE.replace("\n", "\r\n").encode())
|
||||||
self.assertEqual((a1, b1), self.shas())
|
self.assertEqual((a1, b1), self.shas())
|
||||||
|
|||||||
@@ -5,12 +5,16 @@
|
|||||||
|
|
||||||
"""What keeps the page answering the operator instead of the timer.
|
"""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,
|
- 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
|
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
|
its run log, so the file reaches megabytes over a campaign and the poll
|
||||||
grew with it. Both are now parsed and computed once.
|
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
|
- the wasted payload. An idle page polls an unchanged state; it now gets
|
||||||
a 304 instead of the whole thing.
|
a 304 instead of the whole thing.
|
||||||
|
|
||||||
@@ -171,6 +175,63 @@ class FingerprintCacheTests(unittest.TestCase):
|
|||||||
self.assertIsNone(self.man.files("no-such-component"))
|
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):
|
class PageTests(unittest.TestCase):
|
||||||
def test_page_never_rebuilds_what_the_operator_may_be_pressing(self):
|
def test_page_never_rebuilds_what_the_operator_may_be_pressing(self):
|
||||||
"""A poll must update rows, prompt buttons and tool entries in
|
"""A poll must update rows, prompt buttons and tool entries in
|
||||||
|
|||||||
Reference in New Issue
Block a user