From 332d95cd802153f8064bf8dd6e805a0b872614a9 Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Tue, 20 Oct 2026 21:51:31 +0800 Subject: [PATCH 01/10] fix(params): recognise scad.var(name=..., default=...) keyword form The editor only read the default from the second positional argument, so keyword declarations were silently ignored. Co-Authored-By: Claude Sonnet 5.5 --- src/simplecadapi/runtime/params.py | 18 +++++++++++++++--- test/test_notebook_params.py | 5 +++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/simplecadapi/runtime/params.py b/src/simplecadapi/runtime/params.py index 57e565ed..21ce600d 100644 --- a/src/simplecadapi/runtime/params.py +++ b/src/simplecadapi/runtime/params.py @@ -193,9 +193,9 @@ def _read_value(node: ast.expr) -> _Value | None: if ( isinstance(node, ast.Call) and _callee(node.func) == "var" - and len(node.args) >= 2 + and _var_default_node(node) is not None ): - default = _literal(node.args[1]) + default = _literal(_var_default_node(node)) if default is None or isinstance(default, bool): return None unit = comment = None @@ -212,6 +212,17 @@ def _read_value(node: ast.expr) -> _Value | None: return None +def _var_default_node(call: ast.Call) -> ast.expr | None: + """The default argument of ``var(name, default, ...)``, positional or ``default=``.""" + + if len(call.args) >= 2: + return call.args[1] + for keyword in call.keywords: + if keyword.arg == "default": + return keyword.value + return None + + def _callee(node: ast.expr) -> str | None: if isinstance(node, ast.Name): return node.id @@ -595,7 +606,8 @@ def commit_value(code: str, name: str, value: float | int | bool) -> str | None: node = statement.value if declared.declared == "var": assert isinstance(node, ast.Call) - node = node.args[1] + node = _var_default_node(node) + assert node is not None text = _format_value(declared.default, value) if text is None or node.end_lineno != node.lineno: return None diff --git a/test/test_notebook_params.py b/test/test_notebook_params.py index 42a2e973..769e9b8b 100644 --- a/test/test_notebook_params.py +++ b/test/test_notebook_params.py @@ -112,6 +112,11 @@ def test_var_default_is_replaced_not_the_name(self) -> None: assert out is not None self.assertIn('OD = scad.var("od", 120.0, comment="é", unit="mm") # 外径', out) + def test_var_with_keyword_name_and_default_is_recognised(self) -> None: + code = 'J = scad.var(name="j1", default=0.0, comment="joint")\n' + out = commit_value(code, "J", 15) + self.assertEqual(out, 'J = scad.var(name="j1", default=15.0, comment="joint")\n') + def test_refuses_what_does_not_fit(self) -> None: self.assertIsNone(commit_value(self.CODE, "MISSING", 1)) self.assertIsNone(commit_value(self.CODE, "F", 1.0)) From 63704c1ca93a073a26b2733ed2f30206699df4a8 Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Tue, 20 Oct 2026 21:51:53 +0800 Subject: [PATCH 02/10] release: 2.2.6.post1 Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 8 ++++++++ README.md | 16 ++++++---------- README.zh-CN.md | 14 ++++++-------- docs/skill/SKILL.md | 4 ++-- docs/updates/2.2.6.post1.md | 15 +++++++++++++++ docs/updates/2.2.6.post1.zh-CN.md | 13 +++++++++++++ pyproject.toml | 2 +- uv.lock | 2 +- 8 files changed, 52 insertions(+), 22 deletions(-) create mode 100644 docs/updates/2.2.6.post1.md create mode 100644 docs/updates/2.2.6.post1.zh-CN.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 28fd320d..f12fbf10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ user-facing notes live in [`docs/updates/`](docs/updates/) (English and ## Unreleased +## 2.2.6.post1 — 2026-10-20 + +### Fixed + +- Editor: the parameter panel reads `scad.var(name="j1", default=0.0, ...)` — the keyword + form — as well as `scad.var("j1", 0.0, ...)`. Before, only the second positional argument + was taken as the default, so keyword declarations never showed up as parameters. + ## 2.2.6 — 2026-10-19 ### Added diff --git a/README.md b/README.md index ca41bcbf..68f3a6a1 100644 --- a/README.md +++ b/README.md @@ -149,20 +149,15 @@ the full crank cycle sweeps clean in the same sim · --- -## Update Notes (2.2.6) +## Update Notes (2.2.6.post1) > Validate generated definitions, assembly constraints, and manufacturing > geometry before production use. -- **Switch notebooks without leaving the CLI.** `sca cell open NOTEBOOK` moves the editor - and waits for its kernel; the write commands take `--open` to switch and write in one - step, and a refused write names the way out. -- **`sca viewport` answers what the person sees.** The image's path, age and size, copied - out with `--out`, instead of reading `.sca-editor/viewport.png` by hand. -- **Cache entries are signed and verified again.** `cryptography` ships as a dependency, - so marimo's cache verification no longer silently degrades to off. +- **Keyword `scad.var` is a parameter again.** `scad.var(name="j1", default=0.0)` is read + by the editor's parameter panel like the positional form; before, it was skipped. -Full notes for this version: [`docs/updates/2.2.6.md`](docs/updates/2.2.6.md). +Full notes for this version: [`docs/updates/2.2.6.post1.md`](docs/updates/2.2.6.post1.md). Earlier versions: the [changelog](CHANGELOG.md) and [`docs/updates/`](docs/updates/). --- @@ -183,7 +178,7 @@ in a compact public API for creating solids, applying features, tagging semantic intent, querying topology, exporting manufacturing files, and translating recorded models into FreeCAD workflows. -Current release: `simplecadapi==2.2.6`. +Current release: `simplecadapi==2.2.6.post1`. ## What It Provides @@ -698,6 +693,7 @@ and the [STEP reconstruction workflow](docs/skill/references/workflows/step-reco ## Documentation +- 2.2.6.post1 update notes: [`docs/updates/2.2.6.post1.md`](docs/updates/2.2.6.post1.md) - 2.2.6 update notes: [`docs/updates/2.2.6.md`](docs/updates/2.2.6.md) - 2.2.5 update notes: [`docs/updates/2.2.5.md`](docs/updates/2.2.5.md) - 2.2.4 update notes: [`docs/updates/2.2.4.md`](docs/updates/2.2.4.md) diff --git a/README.zh-CN.md b/README.zh-CN.md index d3c4a005..00125b46 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -133,17 +133,14 @@ CalculiX 静力 FEM（边界面按保真的 `interface.*` 标签选取，仿真 --- -## 更新日志（2.2.6） +## 更新日志（2.2.6.post1） > 投入生产前请验证生成的定义、装配约束和制造几何。 -- **不离开 CLI 就能切换 notebook。** `sca cell open NOTEBOOK` 切换编辑器并等 kernel 就绪； - 写命令加 `--open` 切换 + 写入一步完成，被拒绝的写入会直接指路。 -- **`sca viewport` 直接回答"用户看到什么"。** 图片的路径、age、大小，`--out` 复制出去， - 不用手读 `.sca-editor/viewport.png`。 -- **缓存条目重新有了签名验证。** `cryptography` 随包安装，marimo 缓存验证不再静默降级。 +- **关键字写法的 `scad.var` 重新被识别为参数。** `scad.var(name="j1", default=0.0)` 现在和 + 位置参数写法一样出现在参数面板；之前会被跳过。 -本版本的完整说明：[`docs/updates/2.2.6.zh-CN.md`](docs/updates/2.2.6.zh-CN.md)。 +本版本的完整说明：[`docs/updates/2.2.6.post1.zh-CN.md`](docs/updates/2.2.6.post1.zh-CN.md)。 更早的版本：见[更新日志](CHANGELOG.md)和 [`docs/updates/`](docs/updates/)。 --- @@ -161,7 +158,7 @@ CalculiX 静力 FEM（边界面按保真的 `interface.*` 标签选取，仿真 SimpleCADAPI 是一个基于 OCP 的 Python CAD SDK，提供清晰的函数式建模操作和可重放的模型图。它在 OpenCascade 几何内核之上提供精简的公共 API，可用于创建实体、应用特征、添加语义标签、查询拓扑、导出制造文件，以及将记录的模型转换为 FreeCAD 工作流。 -当前版本：`simplecadapi==2.2.6`。 +当前版本：`simplecadapi==2.2.6.post1`。 ## 核心能力 @@ -587,6 +584,7 @@ uv run --extra fem python examples/ap242_gmsh_volume_mesh/run_calculix.py ## 文档 +- 2.2.6.post1 更新说明：[`docs/updates/2.2.6.post1.zh-CN.md`](docs/updates/2.2.6.post1.zh-CN.md) - 2.2.6 更新说明：[`docs/updates/2.2.6.zh-CN.md`](docs/updates/2.2.6.zh-CN.md) - 2.2.5 更新说明：[`docs/updates/2.2.5.zh-CN.md`](docs/updates/2.2.5.zh-CN.md) - 2.2.4 更新说明：[`docs/updates/2.2.4.zh-CN.md`](docs/updates/2.2.4.zh-CN.md) diff --git a/docs/skill/SKILL.md b/docs/skill/SKILL.md index e120c541..ee641c37 100644 --- a/docs/skill/SKILL.md +++ b/docs/skill/SKILL.md @@ -4,9 +4,9 @@ description: Build, assemble, inspect, reconstruct, and export parametric CAD mo license: Apache-2.0 metadata: project: simplecadapi - version: 2.2.6 + version: 2.2.6.post1 package-name: simplecadapi - package-version: 2.2.6 + package-version: 2.2.6.post1 --- # SimpleCAD SDK Skill diff --git a/docs/updates/2.2.6.post1.md b/docs/updates/2.2.6.post1.md new file mode 100644 index 00000000..8bb1fa86 --- /dev/null +++ b/docs/updates/2.2.6.post1.md @@ -0,0 +1,15 @@ +# 2.2.6.post1 Update Notes + +A hotfix for the editor's parameter panel. + +## `scad.var(name=..., default=...)` is a parameter again + +The editor found a parameter's default only as the second *positional* argument of +`scad.var`. A declaration written with keywords — + +```python +J1 = scad.var(name="j1", default=0.0, comment="Joint 1") +``` + +— was skipped: no slider, no commit back to the file. Both forms are read now, and +committing a value rewrites the right argument in either. diff --git a/docs/updates/2.2.6.post1.zh-CN.md b/docs/updates/2.2.6.post1.zh-CN.md new file mode 100644 index 00000000..246171ac --- /dev/null +++ b/docs/updates/2.2.6.post1.zh-CN.md @@ -0,0 +1,13 @@ +# 2.2.6.post1 更新说明 + +编辑器参数面板的热修复。 + +## `scad.var(name=..., default=...)` 重新被识别为参数 + +编辑器只把 `scad.var` 的第二个**位置参数**当作默认值。用关键字写的声明—— + +```python +J1 = scad.var(name="j1", default=0.0, comment="Joint 1") +``` + +——会被跳过：没有滑块，也无法写回文件。现在两种写法都能读取，提交数值时也会改写对应的那个参数。 diff --git a/pyproject.toml b/pyproject.toml index 719f2579..527ee33e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "simplecadapi" -version = "2.2.6" +version = "2.2.6.post1" description = "A simplified OCP-native CAD modeling Python API" readme = "README.md" license = { file = "LICENSE" } diff --git a/uv.lock b/uv.lock index 5634b28b..59754e79 100644 --- a/uv.lock +++ b/uv.lock @@ -3237,7 +3237,7 @@ wheels = [ [[package]] name = "simplecadapi" -version = "2.2.6" +version = "2.2.6.post1" source = { editable = "." } dependencies = [ { name = "cadquery-ocp" }, From 19381d86fd4b9e477ff42c385faf0e73c8404dbb Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Tue, 20 Oct 2026 22:10:41 +0800 Subject: [PATCH 03/10] feat(cache): depend on the names a cell imports, not the whole module file from lib.dims import A now keys the cell on the definition of A and what it reads (across local modules); anything unprovable falls back to the file. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 8 ++ src/simplecadapi/runtime/cache.py | 10 +- src/simplecadapi/runtime/deps.py | 102 ++++++++++++++-- src/simplecadapi/runtime/names.py | 191 ++++++++++++++++++++++++++++++ test/test_name_digests.py | 119 +++++++++++++++++++ 5 files changed, 416 insertions(+), 14 deletions(-) create mode 100644 src/simplecadapi/runtime/names.py create mode 100644 test/test_name_digests.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 28fd320d..7d21c962 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ user-facing notes live in [`docs/updates/`](docs/updates/) (English and ## Unreleased +### Changed + +- Cache: a cell that does `from lib.dims import A` now depends on the definition of `A` (and + what it reads, across local modules), not on the whole file. Editing another name of + `lib/dims.py`, a comment, or the formatting leaves the cell cached. A module the analysis + cannot prove (calls or loops at top level, star imports, a name defined twice, a plain + `import lib.dims`) still depends on the whole file. + ## 2.2.6 — 2026-10-19 ### Added diff --git a/src/simplecadapi/runtime/cache.py b/src/simplecadapi/runtime/cache.py index 8cb3f083..5ac058be 100644 --- a/src/simplecadapi/runtime/cache.py +++ b/src/simplecadapi/runtime/cache.py @@ -41,6 +41,7 @@ Skip, ) from .config import NotebookConfig +from .names import names_tag from .deps import ( CellSource, ancestors, @@ -116,13 +117,14 @@ def __init__( def digest(self, cell_id: CellId_t) -> str | None: """The digest for *cell_id*, ``None`` for a cell that is not cached.""" - files = self._files.get(cell_id) - if files is None: + deps = self._files.get(cell_id) + if deps is None: return None # the setup cell if self._kernel and cell_id in self._uses: return None - tag = ("use" if cell_id in self._uses else "") + self._saved_tag.get(cell_id, "") - return cell_digest(self.config, files, tag=tag) + named, fallback = names_tag(self.config.directory, deps.names) + tag = ("use" if cell_id in self._uses else "") + self._saved_tag.get(cell_id, "") + named + return cell_digest(self.config, deps.files | fallback, tag=tag) class CellCache(CachedLifecycle): diff --git a/src/simplecadapi/runtime/deps.py b/src/simplecadapi/runtime/deps.py index fdc2d547..f955444d 100644 --- a/src/simplecadapi/runtime/deps.py +++ b/src/simplecadapi/runtime/deps.py @@ -18,7 +18,10 @@ *D* digests the current content of all these files, together with the versions of the software that writes the cache, and a run uses the cell cache stored under *D* only. Any change to any of these files therefore -invalidates the notebook's whole cache: coarse, but never stale. +invalidates the cache of the cells that depend on it: coarse, but never +stale. A name a cell imports with ``from m import x`` is the exception: it +is a dependency on the definition of ``x`` (:mod:`.names`), and falls back +to the file when that cannot be digested. """ from __future__ import annotations @@ -289,13 +292,42 @@ class CellSource: """The names the cell defines (marimo's ``cell.defs``).""" +@dataclass(frozen=True) +class NameDep: + """A dependency on one name a cell imports from a local module. + + ``from lib.dims import A`` depends on the definition of ``A`` (see + :mod:`.names`), not on the file; *fallback* is the files it depends on + instead when that definition cannot be digested. + """ + + module: str + name: str + fallback: frozenset[Path] + + +@dataclass(frozen=True) +class CellDeps: + """What a cell depends on outside its code.""" + + files: frozenset[Path] + names: frozenset[NameDep] = frozenset() + + def __or__(self, other: CellDeps) -> CellDeps: + return CellDeps(self.files | other.files, self.names | other.names) + + def cell_module_files( config: NotebookConfig, setup: str | None, cells: Sequence[CellSource], recorded: Iterable[Path] = (), -) -> dict[str, frozenset[Path]]: - """Return, per cell key, the local files whose content its result depends on. +) -> dict[str, CellDeps]: + """Return, per cell key, what its result depends on in local files. + + Names imported with ``from m import x`` from a local module are + dependencies on that name (:class:`NameDep`); every other dependency is + on whole files. What follows describes the files. A cell depends on a local module when it reads a name that an ``import`` in the notebook (the setup cell, or any cell) bound to that module, or @@ -319,6 +351,7 @@ def cell_module_files( for name, modules in _import_bindings(root, source).items(): bound.setdefault(name, set()).update(modules) star = bound.pop("*", set()) + fine = _from_imports(root, sources) closures: dict[tuple[str, Path], frozenset[Path]] = {} @@ -356,7 +389,7 @@ def reach(name: str, seen: set[str]) -> set[str]: star_files = frozenset(path for module in star for path in closure(module)) recorded_files = frozenset(recorded) - result: dict[str, frozenset[Path]] = {} + result: dict[str, CellDeps] = {} for cell in cells: names = set(cell.refs) | (cell.defs & bound.keys()) | _loaded_names(cell.code) needed: set[str] = set() @@ -368,21 +401,31 @@ def reach(name: str, seen: set[str]) -> set[str]: for name in cell.defs & reads.keys(): needed |= reach(name, set()) files: set[Path] = set(star_files) + named: set[NameDep] = set() for name in needed: + coarse: set[Path] = set() for module in bound.get(name, ()): - files |= closure(module) + coarse |= closure(module) + if name in fine: + module_name, original = fine[name] + named.add(NameDep(module_name, original, frozenset(coarse))) + else: + files |= coarse if calls_use(cell.code): files |= recorded_files - result[cell.key] = frozenset(files) + result[cell.key] = CellDeps(frozenset(files), frozenset(named)) # marimo keys a cell that reads an object (a solid, a part) by the code of # the cells that made it, not by its value: a file an ancestor depends on # is one this cell depends on. reached = ancestors(cells) - return { - cell.key: frozenset(result[cell.key].union(*(result[key] for key in reached[cell.key]))) - for cell in cells - } + merged: dict[str, CellDeps] = {} + for cell in cells: + deps = result[cell.key] + for key in reached[cell.key]: + deps = deps | result[key] + merged[cell.key] = deps + return merged def ancestors(cells: Sequence[CellSource]) -> dict[str, frozenset[str]]: @@ -456,6 +499,43 @@ def bind(name: str, modules: Iterable[tuple[str, Path]]) -> None: return bindings +def _from_imports(root: Path, sources: Sequence[str]) -> dict[str, tuple[str, str]]: + """The names bound exactly once, by ``from m import x`` of a local module. + + Name -> (m, x). A name bound any other way, or to two things, or to a + submodule, is left out: it is a dependency on whole files. + """ + + found: dict[str, tuple[str, str] | None] = {} + for source in sources: + try: + tree = ast.parse(source) + except (SyntaxError, ValueError): + continue + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + found[alias.asname or alias.name.split(".")[0]] = None + elif isinstance(node, ast.ImportFrom): + base = _absolute(node.module, node.level, "") + for alias in node.names: + bound = alias.asname or alias.name + if base is None or alias.name == "*" or not _is_local_module(root, base): + found[bound] = None + elif _is_local_module(root, f"{base}.{alias.name}"): + found[bound] = None # a submodule, not a name + elif found.get(bound, (base, alias.name)) != (base, alias.name): + found[bound] = None + else: + found[bound] = (base, alias.name) + return {name: target for name, target in found.items() if target is not None} + + +def _is_local_module(root: Path, module: str) -> bool: + files = _module_files(root, module) + return bool(files) and files[-1][0] == module + + def _definitions(source: str) -> dict[str, set[str]]: """The names each top-level function and class in *source* reads.""" @@ -583,7 +663,9 @@ def _display(config: NotebookConfig, path: Path) -> str: __all__ = [ + "CellDeps", "CellSource", + "NameDep", "ancestors", "calls_use", "cell_digest", diff --git a/src/simplecadapi/runtime/names.py b/src/simplecadapi/runtime/names.py new file mode 100644 index 00000000..3e39fdab --- /dev/null +++ b/src/simplecadapi/runtime/names.py @@ -0,0 +1,191 @@ +"""Variable-level dependencies on local modules. + +``from lib.dims import A`` depends on what ``A`` is, not on every other name +in ``lib/dims.py``. :func:`definition_digest` digests the definition of one +top-level name together with every definition it reads, in its own module and +through ``from m import x`` into other local modules. Comments and +formatting do not count: the digest is over the syntax tree. + +It answers only what it can prove. A module with a statement that can do +more than define a name (a call, a loop, a conditional import, an augmented +assignment), a star import, a name defined twice, a plain ``import m`` of a +local module that the definition reads, or a name the module does not +define: the digest is ``None``, and the caller depends on the whole file as +before. Never stale, at worst coarse. +""" + +from __future__ import annotations + +import ast +import hashlib +import json +import threading +from dataclasses import dataclass +from pathlib import Path + +from .deps import NameDep, _absolute, _loaded_names_of, _module_files, file_digest + + +@dataclass(frozen=True) +class _Module: + name: str + defs: dict[str, ast.stmt] + """Top-level name -> the statement that defines it.""" + imports: dict[str, tuple[str, str]] + """Name bound by ``from m import x [as y]`` of a local module -> (m, x).""" + local_plain: frozenset[str] + """Names bound by ``import m`` of a local module.""" + + +def definition_digest(root: Path, module: str, name: str) -> str | None: + """The digest of *name* defined in local *module* under *root*, or ``None``.""" + + parsed: dict[str, _Module | None] = {} + + def load(mod: str) -> _Module | None: + if mod not in parsed: + parsed[mod] = _parse(root, mod) + return parsed[mod] + + payload: dict[str, str] = {} + packages: dict[str, str | None] = {} + pending = [(module, name)] + while pending: + mod, wanted = pending.pop() + key = f"{mod}:{wanted}" + if key in payload: + continue + info = load(mod) + if info is None: + return None + # Importing the module runs the packages on the way. + for package, path in _module_files(root, mod)[:-1]: + packages[f"pkg:{package}"] = file_digest(path) + if wanted in info.imports: # a re-export + payload[key] = f"-> {info.imports[wanted][0]}:{info.imports[wanted][1]}" + pending.append(info.imports[wanted]) + continue + statement = info.defs.get(wanted) + if statement is None: + return None + payload[key] = ast.dump(statement, include_attributes=False) + for read in _loaded_names_of(statement): + if read in info.local_plain: + return None + if read in info.imports or read in info.defs: + pending.append((mod, read)) + payload.update({key: str(value) for key, value in packages.items()}) + encoded = json.dumps(payload, sort_keys=True, ensure_ascii=False).encode("utf-8") + return hashlib.sha256(encoded).hexdigest()[:16] + + +def _is_local(root: Path, module: str) -> bool: + """Whether *module* itself (not only a package on the way) is a local file.""" + + files = _module_files(root, module) + return bool(files) and files[-1][0] == module + + +_parsed: dict[tuple[str, str, str | None], _Module | None] = {} +_parsed_lock = threading.Lock() + + +def _parse(root: Path, module: str) -> _Module | None: + """:func:`_analyse`, remembered per file content: the kernel asks for the + digest of every cell it runs.""" + + files = _module_files(root, module) + if not files or files[-1][0] != module: + return None + key = (str(root), module, file_digest(files[-1][1])) + with _parsed_lock: + if key in _parsed: + return _parsed[key] + result = _analyse(root, module, files[-1][1]) + with _parsed_lock: + if len(_parsed) > 512: + _parsed.clear() + _parsed[key] = result + return result + + +def _analyse(root: Path, module: str, path: Path) -> _Module | None: + try: + tree = ast.parse(path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, SyntaxError, ValueError): + return None + package = module if path.name == "__init__.py" else module.rpartition(".")[0] + defs: dict[str, ast.stmt] = {} + imports: dict[str, tuple[str, str]] = {} + plain: set[str] = set() + + def define(name: str, statement: ast.stmt) -> bool: + if name in defs or name in imports or name in plain: + return False # rebound: which value a reader sees depends on order + defs[name] = statement + return True + + for statement in tree.body: + if isinstance(statement, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + if not define(statement.name, statement): + return None + elif isinstance(statement, ast.Assign) and all( + isinstance(target, ast.Name) for target in statement.targets + ): + for target in statement.targets: + assert isinstance(target, ast.Name) + if not define(target.id, statement): + return None + elif isinstance(statement, ast.AnnAssign) and isinstance(statement.target, ast.Name): + if statement.value is not None and not define(statement.target.id, statement): + return None + elif isinstance(statement, ast.Import): + for alias in statement.names: + if _is_local(root, alias.name): + bound = alias.asname or alias.name.split(".")[0] + if bound in defs or bound in imports: + return None + plain.add(bound) + elif isinstance(statement, ast.ImportFrom): + base = _absolute(statement.module, statement.level, package) + if base is None: + return None + for alias in statement.names: + if alias.name == "*": + if _is_local(root, base): + return None + continue + bound = alias.asname or alias.name + if bound in defs or bound in imports or bound in plain: + return None + if _is_local(root, f"{base}.{alias.name}"): + plain.add(bound) # a submodule: its content is a whole file + elif _is_local(root, base): + imports[bound] = (base, alias.name) + elif ( + isinstance(statement, ast.Expr) + and isinstance(statement.value, ast.Constant) + and isinstance(statement.value.value, str) + ): + continue # a docstring + else: + return None + return _Module(module, defs, imports, frozenset(plain)) + + +def names_tag(root: Path, names: frozenset[NameDep]) -> tuple[str, frozenset[Path]]: + """The digest of what *names* are now, and the files to depend on instead + for those that cannot be digested.""" + + parts: dict[str, str] = {} + files: set[Path] = set() + for dep in sorted(names, key=lambda item: (item.module, item.name)): + digest = definition_digest(root, dep.module, dep.name) + if digest is None: + files |= dep.fallback + else: + parts[f"{dep.module}:{dep.name}"] = digest + if not parts: + return "", frozenset(files) + encoded = json.dumps(parts, sort_keys=True).encode("utf-8") + return "n" + hashlib.sha256(encoded).hexdigest()[:12], frozenset(files) diff --git a/test/test_name_digests.py b/test/test_name_digests.py new file mode 100644 index 00000000..7cb2f28d --- /dev/null +++ b/test/test_name_digests.py @@ -0,0 +1,119 @@ +"""Variable-level dependencies on local modules. + +A cell that does ``from lib.dims import A`` depends on the definition of +``A`` (and what that definition reads), not on the whole file: editing +another name of ``lib/dims.py`` must leave the cell cached. Anything the +analysis cannot prove falls back to the whole file. +""" + +from __future__ import annotations + +from simplecadapi.runtime import run_notebook +from simplecadapi.runtime.names import definition_digest +from test_notebook_runtime import NotebookTestCase, cell, notebook_text + + +class TestDefinitionDigest(NotebookTestCase): + def digest(self, name: str, module: str = "lib.dims") -> str | None: + return definition_digest(self.root, module, name) + + def test_an_unrelated_name_does_not_move_the_digest(self) -> None: + self.write("lib/dims.py", "A = 1.0\nB = 2.0\n") + before = self.digest("A") + self.write("lib/dims.py", "A = 1.0\nB = 9.0\n") + self.assertEqual(self.digest("A"), before) + self.assertNotEqual(self.digest("B"), definition_digest(self.root, "lib.dims", "A")) + + def test_the_definition_itself_moves_it(self) -> None: + self.write("lib/dims.py", "A = 1.0\n") + before = self.digest("A") + self.write("lib/dims.py", "A = 1.5\n") + self.assertNotEqual(self.digest("A"), before) + + def test_formatting_and_comments_do_not_move_it(self) -> None: + self.write("lib/dims.py", "A = 1.0\n") + before = self.digest("A") + self.write("lib/dims.py", "# note\nA = 1.0 # same\n") + self.assertEqual(self.digest("A"), before) + + def test_names_it_reads_in_the_module_are_followed(self) -> None: + self.write("lib/dims.py", "BASE = 2.0\nA = BASE * 3\nOTHER = 1\n") + before = self.digest("A") + self.write("lib/dims.py", "BASE = 2.0\nA = BASE * 3\nOTHER = 5\n") + self.assertEqual(self.digest("A"), before) + self.write("lib/dims.py", "BASE = 4.0\nA = BASE * 3\nOTHER = 5\n") + self.assertNotEqual(self.digest("A"), before) + + def test_functions_and_classes_are_definitions_too(self) -> None: + self.write("lib/dims.py", "K = 2\n\ndef half(x):\n return x / K\n\nZ = 0\n") + before = self.digest("half") + self.write("lib/dims.py", "K = 2\n\ndef half(x):\n return x / K\n\nZ = 1\n") + self.assertEqual(self.digest("half"), before) + self.write("lib/dims.py", "K = 4\n\ndef half(x):\n return x / K\n\nZ = 1\n") + self.assertNotEqual(self.digest("half"), before) + + def test_a_name_imported_from_another_local_module_is_followed(self) -> None: + self.write("lib/base.py", "W = 1.0\nV = 2.0\n") + self.write("lib/dims.py", "from lib.base import W\nA = W * 2\n") + before = self.digest("A") + self.write("lib/base.py", "W = 1.0\nV = 3.0\n") + self.assertEqual(self.digest("A"), before) + self.write("lib/base.py", "W = 2.0\nV = 3.0\n") + self.assertNotEqual(self.digest("A"), before) + + def test_what_it_cannot_prove_falls_back_to_the_whole_file(self) -> None: + for text in ( + "from lib.base import *\nA = 1\n", # unknown bindings + "A = 1\nprint('side effect')\n", # a statement with effects + "import os\nA = 1\nA += os.getpid() * 0\n", # name rebound + "A = 1\n", # (control: provable) + ): + self.write("lib/base.py", "W = 1\n") + self.write("lib/dims.py", text) + expected_none = text != "A = 1\n" + self.assertEqual(self.digest("A") is None, expected_none, text) + self.assertIsNone(self.digest("MISSING")) + self.assertIsNone(definition_digest(self.root, "lib.nope", "A")) + + +TWO_NAMES = notebook_text( + cell("from lib.dims import A", returns="A"), + cell("from lib.dims import B", returns="B"), + cell("box = scad.make_box_rsolid(A, 4.0, 4.0)", reads="A", returns="box"), + cell("plate = scad.make_box_rsolid(B, B, 1.0)", reads="B", returns="plate"), + cell( + "part = scad.Part(part_id='names', body=scad.union_rsolid(box, plate))", + reads="box, plate", + returns="part", + ), + table='id = "names"', +) + + +class TestVariableLevelCache(NotebookTestCase): + def setUp(self) -> None: + super().setUp() + self.write("lib/dims.py", "A = 10.0\nB = 6.0\n") + self.nb = self.write("names.py", TWO_NAMES) + + def test_editing_one_name_reruns_only_what_reads_it(self) -> None: + run_notebook(self.nb) + self.write("lib/dims.py", "A = 12.0\nB = 6.0\n") + changed = run_notebook(self.nb) + # setup, A, B (its name is unchanged), box, plate (cached), part + self.assertEqual(self.statuses(changed), ["ran", "ran", "cached", "ran", "cached", "ran"]) + + def test_an_edit_to_a_name_nobody_reads_reruns_nothing(self) -> None: + run_notebook(self.nb) + self.write("lib/dims.py", "A = 10.0\nB = 6.0\nUNUSED = 1\n") + self.assertEqual(self.statuses(run_notebook(self.nb)), ["ran"] + ["cached"] * 5) + + def test_the_result_is_what_a_fresh_run_gives(self) -> None: + run_notebook(self.nb) + for text in ("A = 12.0\nB = 6.0\n", "A = 12.0\nB = 8.0\nC = 1\n", "A = 10.0\nB = 6.0\n"): + self.write("lib/dims.py", text) + self.assertEqual( + run_notebook(self.nb).definition.content_hash, + run_notebook(self.nb, cache=False).definition.content_hash, + text, + ) From 999439d939d58060c6022296b2fc2c95d6e9f52d Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Tue, 20 Oct 2026 23:18:40 +0800 Subject: [PATCH 04/10] perf(cache): cheaper shape restore, no private values, compressed geometry Adding a child no longer refreshes the subtree's tag cache unless a tag can pass down; restore indexes entities by position and refreshes tag caches once; the cell cache stores no underscore names' values; BinTools bytes are zlib'd. Desktop-arm benchmark: no-change run 23.6s -> 7.0s. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 6 + src/simplecadapi/_internal/shape_pickle.py | 78 +++++++- src/simplecadapi/core.py | 21 ++- src/simplecadapi/runtime/cache.py | 12 ++ test/test_name_digests.py | 28 +++ test/test_shape_restore_cost.py | 107 +++++++++++ tools/profile_arm6.py | 52 ++++++ tools/profile_use_cascade.py | 202 +++++++++++++++++++++ 8 files changed, 495 insertions(+), 11 deletions(-) create mode 100644 test/test_shape_restore_cost.py create mode 100644 tools/profile_arm6.py create mode 100644 tools/profile_use_cascade.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d21c962..ef403f9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ user-facing notes live in [`docs/updates/`](docs/updates/) (English and ### Changed +- Cache and pickling: restoring a cached shape is about 40% faster (adding a child to a + shape no longer refreshes its whole subtree's tag cache unless a tag can pass down; the + restore finds entities by position instead of scanning, and refreshes each tag cache + once). The cache no longer stores a cell's private (`_name`) variables, and the geometry + bytes in a pickle are compressed (zlib, level 1); entries written before are still read. + On the desktop-arm benchmark a no-change run drops from 23.6 s to 7.0 s. - Cache: a cell that does `from lib.dims import A` now depends on the definition of `A` (and what it reads, across local modules), not on the whole file. Editing another name of `lib/dims.py`, a comment, or the formatting leaves the cell cached. A module the analysis diff --git a/src/simplecadapi/_internal/shape_pickle.py b/src/simplecadapi/_internal/shape_pickle.py index 6ff73abe..98241dff 100644 --- a/src/simplecadapi/_internal/shape_pickle.py +++ b/src/simplecadapi/_internal/shape_pickle.py @@ -19,6 +19,7 @@ import io import re +import zlib from dataclasses import dataclass from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Set, Tuple @@ -93,6 +94,20 @@ def _write_shape(shape: TopoDS_Shape) -> bytes: return stream.getvalue() +_PACKED = b"SCZ1" +"""Starts a payload stored compressed; entries written before it was +introduced hold the BinTools bytes as they are, and are still read.""" + + +def _pack(raw: bytes) -> bytes: + # BinTools is bit-exact doubles: level 1 gets most of the ~10x for little time. + return _PACKED + zlib.compress(raw, 1) + + +def _unpack(payload: bytes) -> bytes: + return zlib.decompress(payload[len(_PACKED) :]) if payload.startswith(_PACKED) else payload + + def _read_shape(payload: bytes) -> TopoDS_Shape: shape = TopoDS_Shape() BinTools.Read_s(shape, io.BytesIO(payload)) @@ -156,15 +171,55 @@ def _capture_states(wrapper: "AnyShape") -> Tuple[_EntityState, ...]: return tuple(states) -def _find_entity( - cache: "_TopologyEntityCache", kind: str, shape: TopoDS_Shape -) -> "_TopoEntity | None": - """Look an entity up without creating one (``cache.get`` would).""" +def _entities_by_position( + cache: "_TopologyEntityCache", kind: str, shapes: TopTools_IndexedMapOfShape +) -> Dict[int, "_TopoEntity"]: + """The entity of each position in *shapes* (``FindIndex`` is ``IsSame``), + found in one pass rather than one scan per position.""" + found: Dict[int, "_TopoEntity"] = {} for entity in cache.entities(kind): - if entity.representative.IsSame(shape): - return entity - return None + position = shapes.FindIndex(entity.representative) + if position > 0: + found.setdefault(position, entity) + return found + + +def _refresh_restored_tags(wrapper: "AnyShape") -> None: + """Rebuild the tag cache of every wrapper under *wrapper*, once each. + + The restored tree has no ancestors above *wrapper*. When none of its + bindings is passed downward, an entity's effective tags are its own + (local and legacy) and need no walk up through its parents; otherwise + the general refresh runs. + """ + + from ..core import TaggedMixin, TopologyPropagation + + members: List[Any] = [] + seen: Set[int] = set() + stack: List[Any] = [wrapper] + while stack: + item = stack.pop() + if id(item) in seen: + continue + seen.add(id(item)) + members.append(item) + stack.extend(item.get_children()) + tagged = [item for item in members if isinstance(item, TaggedMixin)] + if any( + binding.propagation.topology == TopologyPropagation.DOWNWARD + for item in tagged + for binding in item._local_tag_bindings() + ): + wrapper._refresh_tag_cache(recursive=True) + return + for item in tagged: + item._tag_cache.clear() + item._tag_cache.update( + binding.tag + for binding in (*item._local_tag_bindings(), *item._legacy_effective_bindings()) + ) _TOPO_ID_RE = re.compile(r"^(?P[a-z]+)_(?P\d+)$") @@ -195,16 +250,18 @@ def _renumber(cache: "_TopologyEntityCache", restored: Set[int]) -> None: def _apply_states(wrapper: "AnyShape", states: Tuple[_EntityState, ...]) -> None: cache = wrapper._topology_cache maps: Dict[str, TopTools_IndexedMapOfShape] = {} + by_index: Dict[str, Dict[int, "_TopoEntity"]] = {} restored: Set[int] = set() for state in states: shapes = maps.get(state.kind) if shapes is None: shapes = maps[state.kind] = _index_map(wrapper.wrapped, state.kind) + by_index[state.kind] = _entities_by_position(cache, state.kind, shapes) if state.index > shapes.Extent(): raise ValueError( f"pickled {state.kind} #{state.index} is outside the restored shape" ) - entity = _find_entity(cache, state.kind, shapes.FindKey(state.index)) + entity = by_index[state.kind].get(state.index) if entity is None: continue entity.topo_id = state.topo_id @@ -228,13 +285,13 @@ def _apply_states(wrapper: "AnyShape", states: Tuple[_EntityState, ...]) -> None entity.incident_edge_ids.update(state.incident_edge_ids) restored.add(id(entity)) _renumber(cache, restored) - wrapper._refresh_tag_cache(recursive=True) + _refresh_restored_tags(wrapper) def reduce_shape(wrapper: "AnyShape") -> Tuple[Any, Tuple[Any, ...]]: """``__reduce__`` implementation shared by every topology wrapper.""" - payload = source_bytes(wrapper) or _write_shape(wrapper.wrapped) + payload = _pack(source_bytes(wrapper) or _write_shape(wrapper.wrapped)) return (restore_shape, (type(wrapper).__name__, payload, _capture_states(wrapper))) @@ -246,6 +303,7 @@ def restore_shape( from .. import core wrapper_type = getattr(core, class_name) + payload = _unpack(payload) wrapper = wrapper_type(_read_shape(payload)) remember_source_bytes(wrapper, payload) _apply_states(wrapper, states) diff --git a/src/simplecadapi/core.py b/src/simplecadapi/core.py index 363f0541..07a6148d 100644 --- a/src/simplecadapi/core.py +++ b/src/simplecadapi/core.py @@ -940,9 +940,28 @@ def add_child(self, child: "AnyShape") -> None: if child not in self.children: self.children.append(child) child.set_parent(self.self_shape_ref) - if isinstance(child, TaggedMixin): + if isinstance(child, TaggedMixin) and self._may_pass_tags_down(): child._refresh_tag_cache(recursive=True) + def _may_pass_tags_down(self) -> bool: + """Whether a new child of this entity can inherit any tag from it. + + A child's effective tags are its own plus what its ancestors pass + down; an entity that holds no downward binding, and has no ancestor + that does, adds nothing, and the child's tag cache stays right. A + shape being built (or restored) has no ancestors yet, so this spares + refreshing the whole subtree for every child added. + """ + + if not isinstance(self, TaggedMixin): + return True + if any( + binding.propagation.topology == TopologyPropagation.DOWNWARD + for binding in self._local_tag_bindings() + ): + return True + return bool(self._inherited_tag_bindings_with_paths()) + def get_children(self) -> List["AnyShape"]: return self.children diff --git a/src/simplecadapi/runtime/cache.py b/src/simplecadapi/runtime/cache.py index 5ac058be..d3e167e9 100644 --- a/src/simplecadapi/runtime/cache.py +++ b/src/simplecadapi/runtime/cache.py @@ -186,6 +186,18 @@ def teardown(self, cell: CellImpl, glbls: MutableGlobals, run_result: RunResult) if loader is None: return self._loader = loader + # A name that starts with an underscore belongs to its cell: no other + # cell reads it, so storing its value only costs disk and restore time + # (a cell that builds an assembly in `_a` stored it twice). marimo + # insists that an entry has a value for every name the cell defines, + # so the name stays, with nothing in it. + attempt = self._attempts.get(cell.cell_id) + if attempt is not None and not attempt.hit: + glbls = {**glbls} + for name in attempt.defs: + if name.startswith("_"): + glbls[name] = None + glbls[f"_cell_{cell.cell_id}_{name[1:]}"] = None super().teardown(cell, glbls, run_result) def _invalidate(self, cell_id: CellId_t) -> None: diff --git a/test/test_name_digests.py b/test/test_name_digests.py index 7cb2f28d..cac2065b 100644 --- a/test/test_name_digests.py +++ b/test/test_name_digests.py @@ -9,6 +9,7 @@ from __future__ import annotations from simplecadapi.runtime import run_notebook +from simplecadapi.runtime.deps import state_dir from simplecadapi.runtime.names import definition_digest from test_notebook_runtime import NotebookTestCase, cell, notebook_text @@ -117,3 +118,30 @@ def test_the_result_is_what_a_fresh_run_gives(self) -> None: run_notebook(self.nb, cache=False).definition.content_hash, text, ) + + +class TestWhatTheCacheStores(NotebookTestCase): + """A cell's private (underscore) names are invisible to every other cell: + storing them only costs disk and restore time.""" + + def test_private_names_are_not_stored_and_the_cell_is_still_restored(self) -> None: + text = notebook_text( + cell( + """ + _scratch = scad.make_box_rsolid(30.0, 30.0, 30.0) + width = 20.0 + """, + returns="width", + ), + cell("block = scad.make_box_rsolid(width, 4.0, 4.0)", reads="width", returns="block"), + cell("part = scad.Part(part_id='priv', body=block)", reads="block", returns="part"), + table='id = "priv"', + ) + path = self.write("priv.py", text) + first = run_notebook(path) + stored = {item.stem for item in (state_dir(first.config) / "cache").rglob("*.pickle")} + self.assertIn("block", stored) + self.assertNotIn("_scratch", stored) + second = run_notebook(path) + self.assertEqual(self.statuses(second), ["ran", "cached", "cached", "cached"]) + self.assertEqual(first.definition.content_hash, second.definition.content_hash) diff --git a/test/test_shape_restore_cost.py b/test/test_shape_restore_cost.py new file mode 100644 index 00000000..d14cb5bb --- /dev/null +++ b/test/test_shape_restore_cost.py @@ -0,0 +1,107 @@ +"""Restoring a pickled shape: same tags as before, in time linear in its size. + +A cell's value is stored as a pickle (the cell cache), and restoring it is the +main cost of a cached run. The tag cache of every entity is rebuilt on the +way; it must be rebuilt once per entity, not once per entity per ancestor. +""" + +from __future__ import annotations + +import pickle +import unittest + +import simplecadapi as scad +from simplecadapi.core import TaggedMixin, TagScope + + +def entities(root): + """Every wrapper below *root* (and *root*), once.""" + + seen, order, stack = set(), [], [root] + while stack: + item = stack.pop() + if id(item) in seen: + continue + seen.add(id(item)) + order.append(item) + stack.extend(item.get_children()) + return order + + +def effective_tags(root): + return {item.topo_id: frozenset(item._list_tags(TagScope.EFFECTIVE)) for item in entities(root)} + + +def tagged_box(): + box = scad.make_box_rsolid( + 20.0, 10.0, 5.0, tag_prefix="t.box", result_tag="part.box" + ) + cut = scad.cut_rsolid( + box, scad.make_cylinder_rsolid(radius=2.0, height=20.0, bottom_face_center=(10.0, 5.0, -5.0)) + ) + cut._apply_tag("inherited.down", propagate=True) + return cut + + +class TestShapeRestore(unittest.TestCase): + def test_every_entity_keeps_its_effective_tags(self) -> None: + shape = tagged_box() + before = effective_tags(shape) + self.assertTrue(any("inherited.down" in tags for tags in before.values())) + restored = pickle.loads(pickle.dumps(shape)) + self.assertEqual(effective_tags(restored), before) + + def test_the_tag_cache_is_rebuilt_a_bounded_number_of_times(self) -> None: + shape = tagged_box() + payload = pickle.dumps(shape) + count = len(entities(shape)) + calls = 0 + original = TaggedMixin._refresh_tag_cache + + def counted(self, *args, **kwargs): + nonlocal calls + calls += 1 + return original(self, *args, **kwargs) + + TaggedMixin._refresh_tag_cache = counted + try: + pickle.loads(payload) + finally: + TaggedMixin._refresh_tag_cache = original + self.assertLessEqual(calls, 3 * count, f"{calls} refreshes for {count} entities") + + +if __name__ == "__main__": + unittest.main() + + +def drilled_plate(): + plate = scad.make_box_rsolid(60.0, 40.0, 6.0) + for index in range(24): + hole = scad.make_cylinder_rsolid(radius=1.5, height=20.0, bottom_face_center=(5.0 + index % 6 * 9, 5.0 + index // 6 * 9, -5.0)) + plate = scad.cut_rsolid(plate, hole) + return plate + + +class TestShapePayload(unittest.TestCase): + def test_the_stored_geometry_is_compressed(self) -> None: + from simplecadapi._internal.shape_pickle import _write_shape + + shape = drilled_plate() + raw = _write_shape(shape.wrapped) + self.assertGreater(len(raw), 10_000) # something to compress + payload = shape.__reduce__()[1][1] + self.assertLess(len(payload), len(raw) // 2) + + def test_an_entry_stored_before_compression_is_still_read(self) -> None: + from simplecadapi._internal.shape_pickle import _write_shape, restore_shape + + shape = drilled_plate() + before = effective_tags(shape) + name, payload, states = shape.__reduce__()[1] + raw = _write_shape(shape.wrapped) + self.assertEqual(name, type(shape).__name__) + legacy = restore_shape(name, raw, states) + self.assertEqual(effective_tags(legacy), before) + self.assertAlmostEqual(legacy.get_volume(), shape.get_volume(), places=6) + diff --git a/tools/profile_arm6.py b/tools/profile_arm6.py new file mode 100644 index 00000000..d6b3e98c --- /dev/null +++ b/tools/profile_arm6.py @@ -0,0 +1,52 @@ +"""Benchmark: the desktop 6-DOF arm (sandbox-envs/desktop_arm_sources/arm6.py). + +Copies the case to a temporary directory (the source stays clean), then runs +the top notebook headlessly through a series of edits and prints the tree of +nested notebook runs with wall seconds and ran/cached cell counts. + + uv run python tools/profile_arm6.py [--keep DIR] + +Baseline before cache work (2026-10-20, 8-core Apple silicon): cold 80.5s, +warm (no change) 23.6s, J1 edit 18.2s, UPPER_LENGTH edit 35.1s, BASE_D edit +22.8s, OPEN_STROKE edit 10.1s; cache directory 1.7 GB. +""" + +from __future__ import annotations + +import argparse +import shutil +import sys +import tempfile +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parent)) +import profile_use_cascade as P # noqa: E402 + +CASE = Path(__file__).resolve().parent.parent / "sandbox-envs" / "desktop_arm_sources" + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--keep", type=Path, help="copy the case here and keep it") + args = parser.parse_args() + temporary = None if args.keep else tempfile.TemporaryDirectory() + root = (args.keep or Path(temporary.name)).resolve() / "arm" # type: ignore[union-attr] + shutil.copytree(CASE, root, ignore=shutil.ignore_patterns("__marimo__", "__pycache__", "out")) + top = root / "arm6.py" + P.install_probe() + P.scenario("cold", top) + P.scenario("warm (nothing changed)", top) + P.edit(top, 'J1=scad.var("j1", 0.0,unit="deg")', 'J1=scad.var("j1", 15.0,unit="deg")') + P.scenario("edit J1 0->15 (solve only)", top) + P.edit(top, 'upper_length", 90.0', 'upper_length", 95.0') + P.scenario("edit UPPER_LENGTH 90->95", top) + P.edit(root / "yaw_base.py", "default=90.0", "default=92.0") + P.scenario("edit yaw_base BASE_D", top) + P.edit(top, 'open_stroke", 3.0', 'open_stroke", 4.0') + P.scenario("edit OPEN_STROKE (gripper)", top) + if temporary is not None: + temporary.cleanup() + + +if __name__ == "__main__": + main() diff --git a/tools/profile_use_cascade.py b/tools/profile_use_cascade.py new file mode 100644 index 00000000..07d481af --- /dev/null +++ b/tools/profile_use_cascade.py @@ -0,0 +1,202 @@ +"""Profile what one edit makes the notebook runtime rebuild, in a nested-assembly project. + +The project: a top assembly ``use``s assemblies A, B, C; each of those ``use``s +three parts (A also a "giant" part, whose one big cell builds two independent +things). Every part has parameters ``p`` and ``q``. Ideally changing ``p`` +of one part rebuilds that part's cells, the ``use`` of it, and what reads it +in the assembly above, and nothing else. + + uv run python tools/profile_use_cascade.py [--holes N] [--keep DIR] + +Each scenario edits the project, runs the top notebook headlessly, and prints +the tree of notebook runs: wall seconds (inclusive of children) and how many +of the notebook's cells ran / were restored. Every run uses the cache. +""" + +from __future__ import annotations + +import argparse +import re +import tempfile +import textwrap +import threading +import time +from pathlib import Path + +HEADER = '# /// script\n# [tool.simplecadapi]\n# id = "{id}"\n# ///\nimport marimo\n\napp = marimo.App()\n\nwith app.setup:\n import simplecadapi as scad\n\n\n' +FOOTER = '\nif __name__ == "__main__":\n app.run()\n' + + +def heavy(var: str, size: str, holes: int) -> str: + return textwrap.dedent( + f"""\ + _s = scad.make_box_rsolid({size}, 20.0, 20.0) + for _i in range({holes}): + _c = scad.make_cylinder_rsolid(1.5, 30.0) + _s = scad.cut_rsolid(_s, scad.translate_shape(_c, (_i % 6 * 3 + 1, _i // 6 * 3 + 1, -5.0))) + {var} = _s + """ + ) + + +def cell(reads: str, body: str, returns: str) -> str: + code = textwrap.indent(textwrap.dedent(body).strip(), " ") + return f"@app.cell\ndef _({reads}):\n{code}\n return ({returns},)\n\n\n" + + +def part(name: str, holes: int) -> str: + return ( + HEADER.format(id=name) + + cell("", "# ---- params ----\np = 40.0\nq = 3.0", "p, q") + + cell("p", heavy("body", "p", holes), "body") + + cell("q, body", "rim = scad.cut_rsolid(body, scad.make_cylinder_rsolid(q, 50.0))", "rim") + + cell("rim", f"part = scad.Part(part_id='{name}', body=rim)", "part") + + FOOTER + ) + + +def giant(name: str, holes: int) -> str: + """One big cell that reads both p and q and builds two independent things.""" + + body = heavy("a", "p", holes) + "\n" + heavy("b", "q * 10.0", holes) + "\nbody = scad.union_rsolid(a, scad.translate_shape(b, (0.0, 20.0, 0.0)))\n" + return ( + HEADER.format(id=name) + + cell("", "# ---- params ----\np = 40.0\nq = 3.0", "p, q") + + cell("p, q", body, "body") + + cell("body", f"part = scad.Part(part_id='{name}', body=body)", "part") + + FOOTER + ) + + +def assembly(name: str, children: list[tuple[str, str]]) -> str: + """*children*: (variable, relative path).""" + + uses = "".join(cell("", f'{var} = scad.use("{path}")', var) for var, path in children) + lines = [ + "_identity = scad.identity_placement_rplacement()", + f'_assembly = scad.make_assembly_rassembly(assembly_id="{name}", name="{name}")', + ] + for var, _ in children: + lines.append( + f'_assembly = scad.add_component_rassembly(assembly=_assembly, item={var}, component_id="{var}", placement=_identity)' + ) + lines.append(f"{name} = _assembly") + return ( + HEADER.format(id=name) + + uses + + cell(", ".join(var for var, _ in children), "\n".join(lines), name) + + FOOTER + ) + + +def build_project(root: Path, holes: int) -> Path: + parts = root / "parts" + asm = root / "asm" + parts.mkdir(parents=True) + asm.mkdir() + layout = {"a": ["a1", "a2", "a3", "giant"], "b": ["b1", "b2", "b3"], "c": ["c1", "c2", "c3"]} + for names in layout.values(): + for name in names: + text = giant(name, holes) if name == "giant" else part(name, holes) + (parts / f"{name}.py").write_text(text, encoding="utf-8") + for key, names in layout.items(): + (asm / f"asm_{key}.py").write_text( + assembly(f"asm_{key}", [(name, f"../parts/{name}.py") for name in names]), encoding="utf-8" + ) + top = root / "top.py" + top.write_text(assembly("top", [(f"asm_{k}", f"asm/asm_{k}.py") for k in layout]), encoding="utf-8") + return top + + +# --------------------------------------------------------------------------- +# Probe +# --------------------------------------------------------------------------- + +_depth = 0 +_lock = threading.Lock() +_rows: list[tuple[int, str, float, int, int]] = [] + + +def install_probe() -> None: + from simplecadapi.runtime import runner + + original = runner.run_notebook + + def probed(path, *args, **kwargs): # type: ignore[no-untyped-def] + global _depth + with _lock: + depth = _depth + _depth += 1 + slot = len(_rows) + _rows.append((depth, Path(path).name, 0.0, 0, 0)) + start = time.perf_counter() + try: + run = original(path, *args, **kwargs) + finally: + with _lock: + _depth -= 1 + ran = sum(1 for c in run.cells if c.status == "ran" and c.name != "setup") + cached = sum(1 for c in run.cells if c.status == "cached") + with _lock: + _rows[slot] = (depth, Path(path).name, time.perf_counter() - start, ran, cached) + return run + + runner.run_notebook = probed + + +def scenario(title: str, top: Path) -> float: + from simplecadapi.runtime import runner + + _rows.clear() + start = time.perf_counter() + runner.run_notebook(top) + total = time.perf_counter() - start + print(f"\n== {title}: {total:.2f}s") + for depth, name, seconds, ran, cached in _rows: + mark = "" if ran == 0 else f" <- {ran} cell(s) ran" + print(f" {' ' * depth}{name:<14} {seconds:6.2f}s ran {ran:>2} cached {cached:>2}{mark}") + return total + + +def edit(path: Path, old: str, new: str) -> None: + text = path.read_text(encoding="utf-8") + assert old in text, (path, old) + path.write_text(text.replace(old, new, 1), encoding="utf-8") + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--holes", type=int, default=30, help="cuts per heavy build (cost knob)") + parser.add_argument("--keep", type=Path, help="build the project here and keep it") + args = parser.parse_args() + + temporary = None + if args.keep: + root = args.keep.resolve() + else: + temporary = tempfile.TemporaryDirectory() + root = Path(temporary.name).resolve() / "project" + top = build_project(root, args.holes) + print(f"project: {root}") + install_probe() + + parts = root / "parts" + scenario("cold", top) + scenario("warm (nothing changed)", top) + edit(parts / "a1.py", "p = 40.0", "p = 41.0") + scenario("edit p of a1 (a leaf under asm_a)", top) + edit(parts / "a1.py", "p = 41.0", "p = 41.0 # a comment") + scenario("comment only in a1", top) + edit(parts / "giant.py", "q = 3.0", "q = 4.0") + scenario("edit q of giant (one big cell, two independent builds)", top) + edit(parts / "b2.py", "q = 3.0", "q = 3.5") + scenario("edit q of b2 (a leaf under asm_b)", top) + edit(parts / "a1.py", "p = 41.0 # a comment", "p = 40.0") + scenario("revert p of a1", top) + + if temporary is not None: + temporary.cleanup() + + +if __name__ == "__main__": + main() From 3f5ee25e1c852e6363a6ac8a1a8efbe4440d00ae Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Tue, 20 Oct 2026 23:45:59 +0800 Subject: [PATCH 05/10] perf(cache): build restored shapes on first use; compress the cell cache Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 4 + src/simplecadapi/_internal/shape_pickle.py | 102 +++++++++++++++++++-- src/simplecadapi/runtime/cache.py | 32 ++++++- test/test_cell_cache_store.py | 56 +++++++++++ test/test_shape_restore_cost.py | 55 +++++++++++ tools/profile_arm6.py | 7 ++ tools/profile_use_cascade.py | 6 +- 7 files changed, 254 insertions(+), 8 deletions(-) create mode 100644 test/test_cell_cache_store.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ef403f9e..0d5691e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ user-facing notes live in [`docs/updates/`](docs/updates/) (English and once). The cache no longer stores a cell's private (`_name`) variables, and the geometry bytes in a pickle are compressed (zlib, level 1); entries written before are still read. On the desktop-arm benchmark a no-change run drops from 23.6 s to 7.0 s. +- Cache: a restored shape is built when something first uses it (a cached value that no + running cell reads is never built, and pickling it again hands back what it was restored + from), and the cell cache compresses what it writes (zlib level 1; entries written before + are still read). The desktop-arm cache drops from 1.2 GB to 96 MB. - Cache: a cell that does `from lib.dims import A` now depends on the definition of `A` (and what it reads, across local modules), not on the whole file. Editing another name of `lib/dims.py`, a comment, or the formatting leaves the cell cached. A module the analysis diff --git a/src/simplecadapi/_internal/shape_pickle.py b/src/simplecadapi/_internal/shape_pickle.py index 98241dff..cf136019 100644 --- a/src/simplecadapi/_internal/shape_pickle.py +++ b/src/simplecadapi/_internal/shape_pickle.py @@ -19,6 +19,7 @@ import io import re +import threading import zlib from dataclasses import dataclass from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Set, Tuple @@ -295,19 +296,108 @@ def reduce_shape(wrapper: "AnyShape") -> Tuple[Any, Tuple[Any, ...]]: return (restore_shape, (type(wrapper).__name__, payload, _capture_states(wrapper))) +_PENDING = "_pending_restore" +"""The ``__dict__`` key under which a restored shape that is not built yet +keeps what it needs to be: ``(payload, states)``.""" + +_build_lock = threading.RLock() +_building = threading.local() +_lazy_classes: Dict[type, type] = {} + + +class _PendingShape: + """Behaviour of a restored shape that has not been built yet. + + The instance is empty. Its first attribute read or write builds it (the + kernel shape from its bytes, the wrapper tree, the saved state) and puts + its real class back, after which none of this is in the way. Pickling it + again hands back what it was restored from. + """ + + def __getattr__(self, name: str) -> Any: + if not name.startswith("__"): + _build(self) + try: + return object.__getattribute__(self, name) + except AttributeError: + pass + raise AttributeError(f"{type(self).__name__!r} object has no attribute {name!r}") + + def __setattr__(self, name: str, value: Any) -> None: + _build(self) + object.__setattr__(self, name, value) + + def __reduce__(self) -> Tuple[Any, Tuple[Any, ...]]: + pending = self.__dict__.get(_PENDING) + if pending is None: + _build(self) + return type(self).__reduce__(self) + return (restore_shape, (_real_class(type(self)).__name__, *pending)) + + +def _real_class(cls: type) -> type: + return cls.__mro__[2] if _PendingShape in cls.__mro__ else cls + + +def _lazy_class(wrapper_type: type) -> type: + lazy = _lazy_classes.get(wrapper_type) + if lazy is None: + # Same name as the real class, so a repr or a ``type(x).__name__`` + # does not tell the two apart. + lazy = _lazy_classes[wrapper_type] = type( + wrapper_type.__name__, (_PendingShape, wrapper_type), {"__module__": wrapper_type.__module__} + ) + return lazy + + +def is_pending(wrapper: "AnyShape") -> bool: + """Whether *wrapper* was restored and nothing has used it yet.""" + + return _PENDING in wrapper.__dict__ + + +def _build(wrapper: Any) -> None: + """Build a pending shape; a no-op for one already built (or being built + by this thread: its own constructor reads its attributes).""" + + if _PENDING not in wrapper.__dict__: + return + with _build_lock: + pending = wrapper.__dict__.get(_PENDING) + building = _building.__dict__.setdefault("ids", set()) + if pending is None or id(wrapper) in building: + return + building.add(id(wrapper)) + try: + payload, states = pending + payload = _unpack(payload) + real = _real_class(type(wrapper)) + real.__init__(wrapper, _read_shape(payload)) + remember_source_bytes(wrapper, payload) + _apply_states(wrapper, states) + del wrapper.__dict__[_PENDING] + wrapper.__class__ = real + finally: + building.discard(id(wrapper)) + + def restore_shape( class_name: str, payload: bytes, states: Tuple[_EntityState, ...] ) -> "AnyShape": - """Unpickle target: rebuild the wrapper and re-apply entity state.""" + """Unpickle target: a shape that is built when it is first used. + + Most values a cell cache restores are never read by the cells that run + (they are only there so that the cell counts as done), and building one + (decoding the kernel shape, the wrapper tree, the saved state) is most of + the time restoring costs. + """ from .. import core wrapper_type = getattr(core, class_name) - payload = _unpack(payload) - wrapper = wrapper_type(_read_shape(payload)) - remember_source_bytes(wrapper, payload) - _apply_states(wrapper, states) + wrapper = wrapper_type.__new__(_lazy_class(wrapper_type)) + object.__setattr__(wrapper, "__dict__", {_PENDING: (payload, states)}) return wrapper -__all__ = ["reduce_shape", "remember_source_bytes", "restore_shape", "source_bytes"] +__all__ = ["is_pending", "reduce_shape", "remember_source_bytes", "restore_shape", "source_bytes"] diff --git a/src/simplecadapi/runtime/cache.py b/src/simplecadapi/runtime/cache.py index d3e167e9..88093b97 100644 --- a/src/simplecadapi/runtime/cache.py +++ b/src/simplecadapi/runtime/cache.py @@ -24,6 +24,7 @@ import hashlib import json import shutil +import zlib from collections.abc import Callable, Mapping from pathlib import Path from typing import Any @@ -65,6 +66,35 @@ def cache_root(config: NotebookConfig) -> Path: return state_dir(config) / "cache" +_PACKED = b"SCZ2" +"""Starts an entry written compressed; one without it was written before +compression existed and is read as it is.""" + +_MIN_PACKED = 1024 + + +class CompressedFileStore(FileStore): + """marimo's file store, compressing what it writes. + + A cell's value carries the definitions of the parts in it (archives and + JSON documents, written uncompressed) and the geometry: an assembly is a + hundred megabytes of pickle that shrinks ten-fold at zlib level 1, and + reading less is most of what a restore costs. Decompressing happens on + the loader's reader threads, where zlib lets go of the interpreter lock. + """ + + def get(self, key: str) -> bytes | None: + data = super().get(key) + if data is not None and data.startswith(_PACKED): + return zlib.decompress(data[len(_PACKED) :]) + return data + + def put(self, key: str, value: bytes) -> bool: + if len(value) >= _MIN_PACKED: + value = _PACKED + zlib.compress(value, 1) + return super().put(key, value) + + class CellDigests: """The digest of each cell of a notebook, from what it depends on. @@ -162,7 +192,7 @@ def recent(self, limit: int) -> set[str]: def _store(self, digest: str) -> LazyLoader: if digest not in self._stores: self._stores[digest] = LazyLoader( - name=_CELLS, store=LazyStore(FileStore(str(self._root / digest))) + name=_CELLS, store=LazyStore(CompressedFileStore(str(self._root / digest))) ) return self._stores[digest] diff --git a/test/test_cell_cache_store.py b/test/test_cell_cache_store.py new file mode 100644 index 00000000..9b540e76 --- /dev/null +++ b/test/test_cell_cache_store.py @@ -0,0 +1,56 @@ +"""What the cell cache puts on disk: compressed, and still reading what it +wrote before it compressed.""" + +from __future__ import annotations + +import zlib + +from simplecadapi.runtime import run_notebook +from simplecadapi.runtime.deps import state_dir +from test_notebook_runtime import NotebookTestCase, cell, notebook_text + +DRILLED = notebook_text( + cell( + """ + plate = scad.make_box_rsolid(60.0, 40.0, 6.0) + for _i in range(24): + _hole = scad.make_cylinder_rsolid( + radius=1.5, height=20.0, bottom_face_center=(5.0 + _i % 6 * 9, 5.0 + _i // 6 * 9, -5.0) + ) + plate = scad.cut_rsolid(plate, _hole) + """, + returns="plate", + ), + cell("part = scad.Part(part_id='drilled', body=plate)", reads="plate", returns="part"), + table='id = "drilled"', +) + + +class TestCellCacheStore(NotebookTestCase): + def setUp(self) -> None: + super().setUp() + self.nb = self.write("drilled.py", DRILLED) + + def entries(self, run): + return [item for item in (state_dir(run.config) / "cache").rglob("*") if item.is_file() and item.name != "_runs.json"] + + def test_entries_are_stored_compressed(self) -> None: + run = run_notebook(self.nb) + stored = sum(item.stat().st_size for item in self.entries(run)) + plain = 0 + for item in self.entries(run): + data = item.read_bytes() + plain += len(zlib.decompress(data[4:])) if data.startswith(b"SCZ2") else len(data) + self.assertGreater(plain, 100_000) + self.assertLess(stored, plain // 2) + + def test_entries_written_before_compression_are_still_hit(self) -> None: + first = run_notebook(self.nb) + for item in self.entries(first): + data = item.read_bytes() + if data.startswith(b"SCZ2"): + item.write_bytes(zlib.decompress(data[4:])) + second = run_notebook(self.nb) + self.assertEqual(self.statuses(second), ["ran", "cached", "cached"]) + self.assertEqual(first.definition.content_hash, second.definition.content_hash) + diff --git a/test/test_shape_restore_cost.py b/test/test_shape_restore_cost.py index d14cb5bb..d7aef909 100644 --- a/test/test_shape_restore_cost.py +++ b/test/test_shape_restore_cost.py @@ -105,3 +105,58 @@ def test_an_entry_stored_before_compression_is_still_read(self) -> None: self.assertEqual(effective_tags(legacy), before) self.assertAlmostEqual(legacy.get_volume(), shape.get_volume(), places=6) + +class TestLazyRestore(unittest.TestCase): + """A restored shape is built when something first uses it: most cached + values are never read by the cells that run.""" + + def test_a_restored_shape_is_built_on_first_use(self) -> None: + from simplecadapi._internal.shape_pickle import is_pending + + shape = drilled_plate() + restored = pickle.loads(pickle.dumps(shape)) + self.assertIsInstance(restored, scad.Solid) + self.assertTrue(is_pending(restored)) + self.assertAlmostEqual(restored.get_volume(), shape.get_volume(), places=6) + self.assertFalse(is_pending(restored)) + self.assertEqual(effective_tags(restored), effective_tags(shape)) + + def test_pickling_a_shape_nobody_used_does_not_build_it(self) -> None: + from simplecadapi._internal.shape_pickle import is_pending + + shape = drilled_plate() + restored = pickle.loads(pickle.dumps(shape)) + again = pickle.loads(pickle.dumps(restored)) + self.assertTrue(is_pending(restored)) + self.assertTrue(is_pending(again)) + self.assertAlmostEqual(again.get_volume(), shape.get_volume(), places=6) + + def test_a_missing_attribute_is_still_an_attribute_error(self) -> None: + restored = pickle.loads(pickle.dumps(drilled_plate())) + self.assertFalse(hasattr(restored, "no_such_attribute")) + self.assertTrue(hasattr(restored, "wrapped")) + + def test_two_threads_first_using_a_shape_build_it_once(self) -> None: + import threading + + from simplecadapi._internal import shape_pickle + + restored = pickle.loads(pickle.dumps(drilled_plate())) + reads = 0 + original = shape_pickle._read_shape + + def counted(payload): + nonlocal reads + reads += 1 + return original(payload) + + shape_pickle._read_shape = counted + try: + volumes: list[float] = [] + threads = [threading.Thread(target=lambda: volumes.append(restored.get_volume())) for _ in range(4)] + [thread.start() for thread in threads] + [thread.join() for thread in threads] + finally: + shape_pickle._read_shape = original + self.assertEqual(reads, 1) + self.assertEqual(len(set(volumes)), 1) diff --git a/tools/profile_arm6.py b/tools/profile_arm6.py index d6b3e98c..22ca4ddc 100644 --- a/tools/profile_arm6.py +++ b/tools/profile_arm6.py @@ -26,6 +26,13 @@ def main() -> None: + import os + + if os.environ.get("SCA_NO_PACK"): # A/B: write cache entries uncompressed + from simplecadapi.runtime import cache + + cache._MIN_PACKED = 1 << 60 + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) parser.add_argument("--keep", type=Path, help="copy the case here and keep it") args = parser.parse_args() diff --git a/tools/profile_use_cascade.py b/tools/profile_use_cascade.py index 07d481af..d3d72527 100644 --- a/tools/profile_use_cascade.py +++ b/tools/profile_use_cascade.py @@ -149,9 +149,13 @@ def scenario(title: str, top: Path) -> float: _rows.clear() start = time.perf_counter() + cpu = time.process_time() runner.run_notebook(top) total = time.perf_counter() - start - print(f"\n== {title}: {total:.2f}s") + cpu = time.process_time() - cpu + # CPU seconds (all threads of this process) stay comparable between runs + # when the machine is busy; wall seconds do not. + print(f"\n== {title}: {total:.2f}s wall, {cpu:.2f}s cpu") for depth, name, seconds, ran, cached in _rows: mark = "" if ran == 0 else f" <- {ran} cell(s) ran" print(f" {' ' * depth}{name:<14} {seconds:6.2f}s ran {ran:>2} cached {cached:>2}{mark}") From 57418c27e0de2f08a0865b1f71ce41305ddef4c7 Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Tue, 20 Oct 2026 23:45:59 +0800 Subject: [PATCH 06/10] feat(editor): send a slider drag when it settles, not step by step A drag shows its own value and is sent after 250 ms without a change, or at once on release; clicks and number-box commits still go at once. Co-Authored-By: Claude Sonnet 5.5 --- editor-web/src/components/CellEditor.tsx | 24 ++++---- editor-web/src/components/ModuleView.tsx | 27 +++++---- editor-web/src/components/ParamControl.tsx | 26 ++++++-- editor-web/src/lib/settle.test.ts | 70 ++++++++++++++++++++++ editor-web/src/lib/settle.ts | 42 +++++++++++++ editor-web/tsconfig.app.json | 3 +- editor-web/tsconfig.json | 3 +- 7 files changed, 166 insertions(+), 29 deletions(-) create mode 100644 editor-web/src/lib/settle.test.ts create mode 100644 editor-web/src/lib/settle.ts diff --git a/editor-web/src/components/CellEditor.tsx b/editor-web/src/components/CellEditor.tsx index da77beca..ff5b1c7b 100644 --- a/editor-web/src/components/CellEditor.tsx +++ b/editor-web/src/components/CellEditor.tsx @@ -19,6 +19,7 @@ import { import { Badge } from "@/components/ui/badge"; import { ScrollArea } from "@/components/ui/scroll-area"; import { ParamControl, type ParamMeta, type ParamValue } from "@/components/ParamControl"; +import { createSettler, SETTLE_MS } from "@/lib/settle"; import { cadTheme } from "@/lib/cm-theme"; import { sdkCompletion } from "@/lib/sdk-complete"; import { cn } from "@/lib/utils"; @@ -205,17 +206,16 @@ export function CellEditor({ ); }; - // Forward slider bursts; the server coalesces (latest-wins rebuild). - const pendingParams = useRef(new Map()); - const paramTimer = useRef(null); - const forwardParam = (id: string, value: ParamValue) => { - pendingParams.current.set(id, value); - if (paramTimer.current !== null) return; - paramTimer.current = window.setTimeout(() => { - paramTimer.current = null; - socket?.send({ type: "param.set", values: Object.fromEntries(pendingParams.current) }); - pendingParams.current.clear(); - }, 60); + // A drag is sent when it pauses or ends, not step by step: every value the + // server gets is a rebuild. + const socketRef = useRef(socket); + socketRef.current = socket; + const settler = useRef( + createSettler(SETTLE_MS, (values) => socketRef.current?.send({ type: "param.set", values })), + ); + const forwardParam = (id: string, value: ParamValue, now = false) => { + settler.current.push(id, value); + if (now) settler.current.flush(); }; // Disarm delete when clicking anywhere outside the armed button. @@ -364,7 +364,7 @@ function CellBlock({ live: boolean; armed: boolean; socket: SocketLike; - forwardParam: (id: string, value: ParamValue) => void; + forwardParam: (id: string, value: ParamValue, now?: boolean) => void; onToggle: () => void; onRun: () => void; onInterrupt: () => void; diff --git a/editor-web/src/components/ModuleView.tsx b/editor-web/src/components/ModuleView.tsx index 2f12221f..7ae7b509 100644 --- a/editor-web/src/components/ModuleView.tsx +++ b/editor-web/src/components/ModuleView.tsx @@ -8,6 +8,7 @@ import { Badge } from "@/components/ui/badge"; import { ScrollArea } from "@/components/ui/scroll-area"; import { ParamControl, type ParamMeta, type ParamValue } from "@/components/ParamControl"; import { cadTheme } from "@/lib/cm-theme"; +import { createSettler, SETTLE_MS } from "@/lib/settle"; import { sdkCompletion } from "@/lib/sdk-complete"; /** A local module of the workspace (shared dimensions, helpers): what the @@ -41,8 +42,17 @@ export function ModuleView({ const [draft, setDraft] = useState(module.code); const dirty = useRef(false); const saveTimer = useRef(null); - const paramTimer = useRef(null); - const pending = useRef(new Map()); + const socketRef = useRef(socket); + socketRef.current = socket; + const pathRef = useRef(module.path); + pathRef.current = module.path; + const settler = useRef( + createSettler(SETTLE_MS, (values) => + socketRef.current?.send({ type: "module.param", path: pathRef.current, values }), + ), + ); + // values of the module that was left are not the next one's + useEffect(() => () => settler.current.cancel(), [module.path]); // A different module, an external edit or a parameter edit replaces the // text; our own autosave echo must not clobber what is being typed. @@ -68,14 +78,11 @@ export function ModuleView({ editRef.current = edit; const handleChange = useCallback((value: string) => editRef.current(value), []); - const forward = (id: string, value: ParamValue) => { - pending.current.set(id, value); - if (paramTimer.current !== null) return; - paramTimer.current = window.setTimeout(() => { - paramTimer.current = null; - socket?.send({ type: "module.param", path: module.path, values: Object.fromEntries(pending.current) }); - pending.current.clear(); - }, 120); + // A drag is sent when it pauses or ends, not step by step: every value the + // server gets is a rebuild. + const forward = (id: string, value: ParamValue, now = false) => { + settler.current.push(id, value); + if (now) settler.current.flush(); }; const groups = new Map(); diff --git a/editor-web/src/components/ParamControl.tsx b/editor-web/src/components/ParamControl.tsx index 74f1dd22..7aa87fe6 100644 --- a/editor-web/src/components/ParamControl.tsx +++ b/editor-web/src/components/ParamControl.tsx @@ -30,8 +30,16 @@ export function ParamControl({ forwardParam, }: { param: ParamMeta; - forwardParam: (id: string, value: ParamValue) => void; + /** `now`: send at once (a click, a release) instead of when the changes settle. */ + forwardParam: (id: string, value: ParamValue, now?: boolean) => void; }) { + // A drag shows its own value: the server only hears of it when it settles, + // so the value it echoes back cannot move the thumb meanwhile. + const [drag, setDrag] = useState(null); + const dragging = useRef(false); + useEffect(() => { + if (!dragging.current) setDrag(null); + }, [param.value]); const label = param.label || param.id; const title = [param.comment, ...(param.issues ?? [])].filter(Boolean).join("\n") || undefined; const ink = param.modified ? "text-focus" : "text-name"; @@ -42,7 +50,7 @@ export function ParamControl({ type="button" className="flex items-center justify-between gap-2 text-left text-[11px]" title={title} - onClick={() => forwardParam(param.id, !on)} + onClick={() => forwardParam(param.id, !on, true)} > {label} @@ -51,7 +59,7 @@ export function ParamControl({ ); } - const value = Number(param.value); + const value = drag ?? Number(param.value); return (
@@ -63,7 +71,7 @@ export function ParamControl({ min={param.min} max={param.max} step={param.step ?? (param.kind === "int" ? 1 : null)} - onCommit={(next) => forwardParam(param.id, next)} + onCommit={(next) => forwardParam(param.id, next, true)} /> ) : ( {value} @@ -78,7 +86,15 @@ export function ParamControl({ max={param.max} modified={param.modified} step={param.step ?? (param.kind === "int" ? 1 : (param.max - param.min) / 200)} - onValueChange={([next]) => forwardParam(param.id, next)} + onValueChange={([next]) => { + dragging.current = true; + setDrag(next); + forwardParam(param.id, next); + }} + onValueCommit={([next]) => { + dragging.current = false; + forwardParam(param.id, next, true); + }} /> )}
diff --git a/editor-web/src/lib/settle.test.ts b/editor-web/src/lib/settle.test.ts new file mode 100644 index 00000000..a37acd97 --- /dev/null +++ b/editor-web/src/lib/settle.test.ts @@ -0,0 +1,70 @@ +// Run with: node --test src/lib/settle.test.ts (Node >= 22.6 strips the types) +import assert from "node:assert/strict"; +import { mock, test } from "node:test"; + +import { createSettler } from "./settle.ts"; + +function setup(delay = 250) { + mock.timers.enable({ apis: ["setTimeout"] }); + const sent: Record[] = []; + const settler = createSettler(delay, (values) => sent.push(values)); + return { sent, settler }; +} + +test("a burst of changes is sent once, after it stops", () => { + const { sent, settler } = setup(); + for (let step = 1; step <= 20; step++) { + settler.push("j1", step); + mock.timers.tick(100); // faster than the delay: the drag never pauses + } + assert.equal(sent.length, 0); + mock.timers.tick(250); + assert.deepEqual(sent, [{ j1: 20 }]); + mock.reset(); +}); + +test("a pause in the middle of a drag sends what is there", () => { + const { sent, settler } = setup(); + settler.push("j1", 5); + mock.timers.tick(300); + settler.push("j1", 9); + mock.timers.tick(300); + assert.deepEqual(sent, [{ j1: 5 }, { j1: 9 }]); + mock.reset(); +}); + +test("flush sends at once and cancels the wait", () => { + const { sent, settler } = setup(); + settler.push("j1", 7); + settler.flush(); + assert.deepEqual(sent, [{ j1: 7 }]); + mock.timers.tick(1000); + assert.equal(sent.length, 1); + mock.reset(); +}); + +test("the latest value of each parameter wins, and all of them go together", () => { + const { sent, settler } = setup(); + settler.push("j1", 1); + settler.push("j2", 2); + settler.push("j1", 3); + settler.flush(); + assert.deepEqual(sent, [{ j1: 3, j2: 2 }]); + mock.reset(); +}); + +test("flush with nothing pending sends nothing", () => { + const { sent, settler } = setup(); + settler.flush(); + assert.equal(sent.length, 0); + mock.reset(); +}); + +test("cancel drops what is pending", () => { + const { sent, settler } = setup(); + settler.push("j1", 1); + settler.cancel(); + mock.timers.tick(1000); + assert.equal(sent.length, 0); + mock.reset(); +}); diff --git a/editor-web/src/lib/settle.ts b/editor-web/src/lib/settle.ts new file mode 100644 index 00000000..a786d0fe --- /dev/null +++ b/editor-web/src/lib/settle.ts @@ -0,0 +1,42 @@ +/** How long a drag has to pause before its value is sent (ms). */ +export const SETTLE_MS = 250; + +/** Collect changes and send them once they settle. + * + * A slider reports every step of a drag, and each step the server is given + * is a rebuild of the model. `push` records the latest value of a key and + * restarts the wait; the values go out together when `delay` ms pass without + * another change (the drag paused), or at once on `flush` (the pointer was + * released, a toggle was clicked). Not a throttle: a drag that never pauses + * sends nothing until it ends. + */ +export function createSettler(delay: number, send: (values: Record) => void) { + const pending = new Map(); + let timer: ReturnType | null = null; + + const stop = () => { + if (timer !== null) clearTimeout(timer); + timer = null; + }; + + const flush = () => { + stop(); + if (pending.size === 0) return; + const values = Object.fromEntries(pending); + pending.clear(); + send(values); + }; + + return { + push(key: string, value: V) { + pending.set(key, value); + stop(); + timer = setTimeout(flush, delay); + }, + flush, + cancel() { + stop(); + pending.clear(); + }, + }; +} diff --git a/editor-web/tsconfig.app.json b/editor-web/tsconfig.app.json index 6830b6f7..2a0e8bc3 100644 --- a/editor-web/tsconfig.app.json +++ b/editor-web/tsconfig.app.json @@ -22,5 +22,6 @@ "erasableSyntaxOnly": true, "noFallthroughCasesInSwitch": true }, - "include": ["src"] + "include": ["src"], + "exclude": ["src/**/*.test.ts"] } diff --git a/editor-web/tsconfig.json b/editor-web/tsconfig.json index 2da88b0d..5943e4be 100644 --- a/editor-web/tsconfig.json +++ b/editor-web/tsconfig.json @@ -17,5 +17,6 @@ "@/*": ["./src/*"] } }, - "include": ["src"] + "include": ["src"], + "exclude": ["src/**/*.test.ts"] } From 54016e9a97ee2959177d61b2446004e371365cef Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Tue, 20 Oct 2026 23:55:27 +0800 Subject: [PATCH 07/10] perf(cache): key scad.var and expressions by content Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 5 +++ src/simplecadapi/runtime/_marimo.py | 3 ++ src/simplecadapi/runtime/cache.py | 1 + src/simplecadapi/runtime/stubs.py | 49 +++++++++++++++++++++++++ test/test_cell_cache_store.py | 56 +++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+) create mode 100644 src/simplecadapi/runtime/stubs.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d5691e6..3b8441b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,11 @@ user-facing notes live in [`docs/updates/`](docs/updates/) (English and running cell reads is never built, and pickling it again hands back what it was restored from), and the cell cache compresses what it writes (zlib level 1; entries written before are still read). The desktop-arm cache drops from 1.2 GB to 96 MB. +- Cache: `scad.var(...)` values and the expressions made from them are keyed by content, so a + cell that reads one parameter is no longer redone because another one, defined in the same + cell, changed (marimo keyed any object it cannot hash by the code of the cell that made it). +- Editor: a slider drag is sent when it pauses (250 ms) or ends, not step by step; clicks and + number-box commits still go at once. - Cache: a cell that does `from lib.dims import A` now depends on the definition of `A` (and what it reads, across local modules), not on the whole file. Editing another name of `lib/dims.py`, a comment, or the formatting leaves the cell cached. A module the analysis diff --git a/src/simplecadapi/runtime/_marimo.py b/src/simplecadapi/runtime/_marimo.py index 9874a967..0b7db9f6 100644 --- a/src/simplecadapi/runtime/_marimo.py +++ b/src/simplecadapi/runtime/_marimo.py @@ -37,6 +37,7 @@ from marimo._runtime.runner.result import RunResult from marimo._save.loaders.lazy import LazyLoader, LazyStore from marimo._save.stores.file import FileStore +from marimo._save.stubs.stubs import CustomStub, register_stub from marimo._types.globals import MutableGlobals from marimo._types.ids import CellId_t from marimo._utils.paths import notebook_output_dir @@ -47,6 +48,7 @@ "CachedLifecycle", "CellId_t", "CellImpl", + "CustomStub", "DefaultExecutor", "DirectedGraph", "Evaluator", @@ -68,6 +70,7 @@ "load_notebook", "notebook_output_dir", "parse_notebook", + "register_stub", "read_pyproject_from_script", "runtime_context_installed", "safe_get_context", diff --git a/src/simplecadapi/runtime/cache.py b/src/simplecadapi/runtime/cache.py index 88093b97..115e94ef 100644 --- a/src/simplecadapi/runtime/cache.py +++ b/src/simplecadapi/runtime/cache.py @@ -43,6 +43,7 @@ ) from .config import NotebookConfig from .names import names_tag +from .stubs import register_scalar_stubs # noqa: F401 (registers on import) from .deps import ( CellSource, ancestors, diff --git a/src/simplecadapi/runtime/stubs.py b/src/simplecadapi/runtime/stubs.py new file mode 100644 index 00000000..b335a568 --- /dev/null +++ b/src/simplecadapi/runtime/stubs.py @@ -0,0 +1,49 @@ +"""What the cell cache makes of the values a notebook's parameters are. + +``scad.var(...)`` is an object, not a number. marimo keys a cell by the +values it reads, but only for types it can hash; for any other object it +keys by the *code of the cell that made it*, and a params cell that defines +seven variables then invalidates every cell that reads any one of them +whenever one of the seven changes. A scalar expression has a content digest +of its own (``expr_id``: the declaration, or the operation over its operands' +digests), so registering it as a type marimo can hash makes a reader depend +on what it reads. +""" + +from __future__ import annotations + +from typing import Any + +from ..params.expr import Const, Expr, ScalarExprBase, Var, _expr_signature +from ._marimo import CustomStub, register_stub + + +class ScalarExprStub(CustomStub): + """Stands for a scalar expression in the cache: hashed by content, stored + as the (small, picklable) expression itself.""" + + __slots__ = ("expr",) + + def __init__(self, obj: Any) -> None: + self.expr = obj + + def load(self, glbls: dict[str, Any]) -> Any: + return self.expr + + @staticmethod + def get_type() -> type: + return ScalarExprBase + + def to_bytes(self) -> bytes: + expr = self.expr + return repr((type(expr).__name__, expr.expr_id, _expr_signature(expr))).encode("utf-8") + + +def register_scalar_stubs() -> None: + for kind in (Const, Var, Expr): + register_stub(kind, ScalarExprStub) + + +register_scalar_stubs() + +__all__ = ["ScalarExprStub", "register_scalar_stubs"] diff --git a/test/test_cell_cache_store.py b/test/test_cell_cache_store.py index 9b540e76..0784e92d 100644 --- a/test/test_cell_cache_store.py +++ b/test/test_cell_cache_store.py @@ -54,3 +54,59 @@ def test_entries_written_before_compression_are_still_hit(self) -> None: self.assertEqual(self.statuses(second), ["ran", "cached", "cached"]) self.assertEqual(first.definition.content_hash, second.definition.content_hash) + +def _params_notebook(params: str) -> str: + """A params cell, a cell that reads `a` only, and the part made from it.""" + + return notebook_text( + cell(params, returns="a, b, w" if "w =" in params else "a, b"), + cell("block = scad.make_box_rsolid(float(w), 4.0, 4.0)", reads="w", returns="block") + if "w =" in params + else cell("block = scad.make_box_rsolid(float(a), 4.0, 4.0)", reads="a", returns="block"), + cell("part = scad.Part(part_id='vars', body=block)", reads="block", returns="part"), + table='id = "vars"', + ) + + +class TestParametersAreKeyedByValue(NotebookTestCase): + """`scad.var` is a value, not a piece of its cell's code: a cell that reads + one parameter is not redone because another one, defined next to it, + changed (marimo keys what it cannot hash by the code that made it).""" + + def run_edit(self, before: str, after: str): + path = self.write("vars.py", _params_notebook(before)) + run_notebook(path) + self.write("vars.py", _params_notebook(after)) + edited = run_notebook(path) + fresh = run_notebook(path, cache=False) + self.assertEqual(edited.definition.content_hash, fresh.definition.content_hash) + return self.statuses(edited) + + A, B = 'a = scad.var("a", 10.0, unit="mm")', 'b = scad.var("b", {}, unit="mm")' + + def test_editing_a_sibling_parameter_leaves_the_reader_cached(self) -> None: + statuses = self.run_edit(f"{self.A}\n{self.B.format(1.0)}", f"{self.A}\n{self.B.format(2.0)}") + self.assertEqual(statuses, ["ran", "ran", "cached", "cached"]) + + def test_editing_the_parameter_it_reads_redoes_it(self) -> None: + statuses = self.run_edit( + f"{self.A}\n{self.B.format(1.0)}", f'a = scad.var("a", 11.0, unit="mm")\n{self.B.format(1.0)}' + ) + self.assertEqual(statuses, ["ran", "ran", "ran", "ran"]) + + def test_what_else_a_declaration_says_also_redoes_it(self) -> None: + for after in ( + 'a = scad.var("a", 10.0, unit="mm", comment="new comment")', + 'a = scad.var("a2", 10.0, unit="mm")', + 'a = scad.var("a", 10.0, unit="mm", tolerance=0.1)', + ): + with self.subTest(after=after): + statuses = self.run_edit(f"{self.A}\n{self.B.format(1.0)}", f"{after}\n{self.B.format(1.0)}") + self.assertEqual(statuses[2:], ["ran", "ran"]) + + def test_a_derived_expression_is_keyed_by_its_content_too(self) -> None: + derived = f"{self.A}\n{self.B.format(1.0)}\nw = a * 2.0 + 1.0" + statuses = self.run_edit(derived, derived.replace('b", 1.0', 'b", 2.0')) + self.assertEqual(statuses, ["ran", "ran", "cached", "cached"]) + changed = self.run_edit(derived, derived.replace("a * 2.0", "a * 3.0")) + self.assertEqual(changed[2:], ["ran", "ran"]) From df52326ec0458ac806d17a4aecc7af1f6a06f076 Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Wed, 21 Oct 2026 00:25:02 +0800 Subject: [PATCH 08/10] perf(runtime): static use digests, process-level use prefetch, state-dir lock A cell that uses notebooks by literal path is keyed by what each of them is, and the kernel caches such cells; the use calls a cell is about to make are started together in worker processes (SCA_JOBS), when that is worth the seconds a worker takes to start; a notebook's state directory is locked between processes. Desktop-arm benchmark: cold 80.5s -> 34.6s. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 14 + src/simplecadapi/runtime/_prefetch_worker.py | 41 ++ src/simplecadapi/runtime/cache.py | 30 +- src/simplecadapi/runtime/children.py | 115 +++++ src/simplecadapi/runtime/deps.py | 86 +++- src/simplecadapi/runtime/executor.py | 12 + src/simplecadapi/runtime/prefetch.py | 418 +++++++++++++++++++ src/simplecadapi/runtime/runner.py | 11 +- test/conftest.py | 5 + test/test_state_lock.py | 51 +++ test/test_use_digests.py | 110 +++++ test/test_use_prefetch.py | 158 +++++++ 12 files changed, 1044 insertions(+), 7 deletions(-) create mode 100644 src/simplecadapi/runtime/_prefetch_worker.py create mode 100644 src/simplecadapi/runtime/children.py create mode 100644 src/simplecadapi/runtime/prefetch.py create mode 100644 test/test_state_lock.py create mode 100644 test/test_use_digests.py create mode 100644 test/test_use_prefetch.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b8441b0..d8288fe7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,20 @@ user-facing notes live in [`docs/updates/`](docs/updates/) (English and cell, changed (marimo keyed any object it cannot hash by the code of the cell that made it). - Editor: a slider drag is sent when it pauses (250 ms) or ends, not step by step; clicks and number-box commits still go at once. +- Cache: a cell that runs a notebook with `use("child.py")` (a literal path) is keyed by what + that child is (its text, the modules it imports, the notebooks it uses in turn) instead of + by everything a run recorded: editing one child redoes the cell that uses it and not those + that use its siblings, and the editor kernel caches such cells too (it ran them live + before). A path computed at run time still depends on what a run recorded. +- Runtime: the `use` calls a cell is about to make (literal path; arguments made of values it + reads, `float(...)`, `notebook_id()`) are started together in worker processes, which fill + the notebooks' caches; the cell then finds them built. OpenCASCADE keeps the interpreter + lock, so these are processes, not threads. `SCA_JOBS` sets how many (default: the smaller of + 4 and the cores; 1 turns it off). Workers are only started when the builds they take off + the cell's hands are worth the seconds a worker takes to start (a notebook remembers how + long it last took to build), and a notebook that has not changed since it last ran is not + started at all. A notebook's state directory is locked between processes while it runs. + Desktop-arm benchmark: cold 80.5 s to 34.6 s. - Cache: a cell that does `from lib.dims import A` now depends on the definition of `A` (and what it reads, across local modules), not on the whole file. Editing another name of `lib/dims.py`, a comment, or the formatting leaves the cell cached. A module the analysis diff --git a/src/simplecadapi/runtime/_prefetch_worker.py b/src/simplecadapi/runtime/_prefetch_worker.py new file mode 100644 index 00000000..9bfdc4dd --- /dev/null +++ b/src/simplecadapi/runtime/_prefetch_worker.py @@ -0,0 +1,41 @@ +"""A worker process of :mod:`.prefetch`: runs the notebooks it is sent. + +Jobs and answers are length-prefixed pickles on stdin and the original stdout; +file descriptor 1 itself is pointed at the null device, so whatever a notebook +prints (from Python or from C) cannot get in the way of the protocol. +""" + +from __future__ import annotations + +import os +import pickle +import struct +import sys +from pathlib import Path + + +def main() -> int: + reply = os.fdopen(os.dup(1), "wb") + null = os.open(os.devnull, os.O_WRONLY) + os.dup2(null, 1) + sys.stdout = open(os.devnull, "w") # noqa: SIM115 + jobs = sys.stdin.buffer + from simplecadapi.runtime.runner import run_notebook + + while True: + header = jobs.read(8) + if len(header) < 8: + return 0 + path, child_id, overrides = pickle.loads(jobs.read(struct.unpack(">Q", header)[0])) + try: + run_notebook(Path(path), id=child_id, overrides=overrides) + answer: tuple[str, str] = ("ok", "") + except BaseException as error: # noqa: BLE001 - reported, the parent runs it for itself + answer = ("error", f"{type(error).__name__}: {error}"[-2000:]) + data = pickle.dumps(answer) + reply.write(struct.pack(">Q", len(data)) + data) + reply.flush() + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/simplecadapi/runtime/cache.py b/src/simplecadapi/runtime/cache.py index 115e94ef..c2f2b685 100644 --- a/src/simplecadapi/runtime/cache.py +++ b/src/simplecadapi/runtime/cache.py @@ -41,6 +41,7 @@ RunResult, Skip, ) +from .children import notebook_digest from .config import NotebookConfig from .names import names_tag from .stubs import register_scalar_stubs # noqa: F401 (registers on import) @@ -50,7 +51,6 @@ cell_digest, cell_module_files, read_dependencies, - recorded_dependents, state_dir, ) @@ -133,7 +133,8 @@ def __init__( # A cell that runs a child notebook, or reads what one makes, depends # on what the last run recorded; its entries are kept apart so they # can follow the record. - self._uses = recorded_dependents(sources) + self._uses = frozenset(key for key, deps in self._files.items() if deps.dynamic) + self._recorded = read_dependencies(config) self._kernel = kernel self._saved_tag: dict[str, str] = {} if saved: @@ -154,9 +155,32 @@ def digest(self, cell_id: CellId_t) -> str | None: if self._kernel and cell_id in self._uses: return None named, fallback = names_tag(self.config.directory, deps.names) - tag = ("use" if cell_id in self._uses else "") + self._saved_tag.get(cell_id, "") + named + used, unknown = self._used_tag(deps.uses) + if unknown: + # A notebook this one uses has a part of its own that only a run + # can name: depend on what the last run recorded. + if self._kernel: + return None + fallback = fallback | self._recorded + tag = ("use" if cell_id in self._uses or unknown else "") + self._saved_tag.get(cell_id, "") + named + used return cell_digest(self.config, deps.files | fallback, tag=tag) + @staticmethod + def _used_tag(paths: frozenset[Path]) -> tuple[str, bool]: + """The digest of the notebooks a cell uses by a literal path, and + whether one of them cannot be digested.""" + + if not paths: + return "", False + digests: dict[str, str] = {} + for path in sorted(paths): + digest = notebook_digest(path) + if digest is None: + return "", True + digests[path.name] = digest + encoded = json.dumps(digests, sort_keys=True).encode("utf-8") + return "u" + hashlib.sha256(encoded).hexdigest()[:12], False + class CellCache(CachedLifecycle): """marimo's cell cache, a store per digest, reporting its hits.""" diff --git a/src/simplecadapi/runtime/children.py b/src/simplecadapi/runtime/children.py new file mode 100644 index 00000000..7791f8de --- /dev/null +++ b/src/simplecadapi/runtime/children.py @@ -0,0 +1,115 @@ +"""What a notebook is, to the notebook that uses it. + +``use("child.py")`` makes a cell depend on the child. When the path is a +literal that is known before the cell runs, so the cell is keyed by a digest +of the child, and each ``use`` is followed on its own: editing one child +recomputes the cells that use it, not those that use its siblings. + +The digest is of what the child *is*: its text, the local modules it imports, +the project files it reads, and, in turn, the digest of every notebook it +uses. Its text, not its syntax tree: the definition a run makes records where +in the file each step is, so a comment or a blank line changes the definition +of every notebook that embeds the child, and a cached parent must equal a +fresh one. +A child that computes a path itself has no such digest (``None``): the caller +then depends on whatever a run recorded. +""" + +from __future__ import annotations + +import hashlib +import json +import os +import threading +from dataclasses import dataclass +from pathlib import Path + +from .config import read_notebook_config +from .deps import file_digest, input_files, local_modules, use_calls + + +@dataclass(frozen=True) +class _Parts: + syntax: str + header: str + modules: tuple[Path, ...] + inputs: tuple[Path, ...] + children: tuple[Path, ...] + + +_parts_memo: dict[tuple[Path, str | None], _Parts | None] = {} +_memo_lock = threading.Lock() + + +def notebook_digest(path: Path) -> str | None: + """The digest of the notebook at *path* and what it depends on, or ``None`` + when that cannot be told without running it.""" + + return _digest(path.resolve(), frozenset()) + + +def _digest(path: Path, stack: frozenset[Path]) -> str | None: + if path in stack: + return "cycle" # a run reports it + parts = _parts(path) + if parts is None: + return None + here = path.parent + children: dict[str, str] = {} + for child in parts.children: + digest = _digest(child, stack | {path}) + if digest is None: + return None + children[_relative(child, here)] = digest + payload = { + "syntax": parts.syntax, + "header": parts.header, + "modules": {_relative(item, here): file_digest(item) for item in parts.modules}, + "inputs": {_relative(item, here): file_digest(item) for item in parts.inputs}, + "children": children, + } + encoded = json.dumps(payload, sort_keys=True).encode("utf-8") + return hashlib.sha256(encoded).hexdigest()[:16] + + +def _parts(path: Path) -> _Parts | None: + key = (path, file_digest(path)) + with _memo_lock: + if key in _parts_memo: + return _parts_memo[key] + parts = _analyse(path) + with _memo_lock: + if len(_parts_memo) > 512: + _parts_memo.clear() + _parts_memo[key] = parts + return parts + + +def _analyse(path: Path) -> _Parts | None: + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeError): + return None + config = read_notebook_config(path) + if config is None: + return None + calls = use_calls(text) + if any(call is None for call in calls): + return None + return _Parts( + syntax=hashlib.sha256(text.encode("utf-8")).hexdigest()[:16], + header=repr((config.id, config.revision, config.tolerance_profile, config.inputs)), + modules=tuple(sorted(local_modules(config, [text]).values())), + inputs=tuple(sorted(input_files(config))), + children=tuple(sorted({(path.parent / call).resolve() for call in calls if call is not None})), + ) + + +def _relative(path: Path, start: Path) -> str: + try: + return Path(os.path.relpath(path, start)).as_posix() + except ValueError: # another drive on Windows + return path.as_posix() + + +__all__ = ["notebook_digest"] diff --git a/src/simplecadapi/runtime/deps.py b/src/simplecadapi/runtime/deps.py index f955444d..26d74fdf 100644 --- a/src/simplecadapi/runtime/deps.py +++ b/src/simplecadapi/runtime/deps.py @@ -92,6 +92,49 @@ def state_dir(config: NotebookConfig) -> Path: return notebook_output_dir(config.path) / "simplecad" / config.path.name / config.id +_held: set[str] = set() +_held_lock = threading.Lock() + + +@contextmanager +def exclusive(config: NotebookConfig) -> Iterator[None]: + """Hold the notebook's state directory against other processes. + + Worker processes may run the same notebook at once (two notebooks that + use a third), and a run prunes the cache directories it did not use. The + lock is between processes only: a run of a notebook this process is + already running (a ``use`` cycle, an editor next to a headless run) goes + on without it, as it always did. Without ``fcntl`` (Windows) it is a + no-op. + """ + + directory = state_dir(config) + key = str(directory) + with _held_lock: + nested = key in _held + _held.add(key) + handle = None + try: + if not nested: + try: + import fcntl + + directory.mkdir(parents=True, exist_ok=True) + handle = open(directory / "run.lock", "a") # noqa: SIM115 + fcntl.flock(handle.fileno(), fcntl.LOCK_EX) + except (ImportError, OSError): + if handle is not None: + handle.close() + handle = None + yield + finally: + if handle is not None: + handle.close() # releases the lock + if not nested: + with _held_lock: + _held.discard(key) + + def deps_file(config: NotebookConfig) -> Path: return state_dir(config) / "deps.json" @@ -312,9 +355,18 @@ class CellDeps: files: frozenset[Path] names: frozenset[NameDep] = frozenset() + uses: frozenset[Path] = frozenset() + """Notebooks the cell runs with ``use``, by a literal path (see :mod:`.children`).""" + dynamic: bool = False + """The cell runs a notebook whose path is computed: only a run knows which.""" def __or__(self, other: CellDeps) -> CellDeps: - return CellDeps(self.files | other.files, self.names | other.names) + return CellDeps( + self.files | other.files, + self.names | other.names, + self.uses | other.uses, + self.dynamic or other.dynamic, + ) def cell_module_files( @@ -411,9 +463,14 @@ def reach(name: str, seen: set[str]) -> set[str]: named.add(NameDep(module_name, original, frozenset(coarse))) else: files |= coarse - if calls_use(cell.code): + calls = use_calls(cell.code) + dynamic = any(call is None for call in calls) + if dynamic: files |= recorded_files - result[cell.key] = CellDeps(frozenset(files), frozenset(named)) + uses = frozenset( + (root / call).resolve() for call in calls if call is not None + ) + result[cell.key] = CellDeps(frozenset(files), frozenset(named), uses, dynamic) # marimo keys a cell that reads an object (a solid, a part) by the code of # the cells that made it, not by its value: a file an ancestor depends on @@ -565,6 +622,27 @@ def _loaded_names_of(node: ast.AST) -> set[str]: } +def use_calls(source: str) -> list[str | None]: + """The paths of the ``use(...)`` calls in *source*: the literal, or ``None`` + where it is computed (or the source cannot be read).""" + + try: + tree = ast.parse(source) + except (SyntaxError, ValueError): + return [None] + paths: list[str | None] = [] + for node in ast.walk(tree): + if isinstance(node, ast.Call): + function = node.func + if (isinstance(function, ast.Name) and function.id == "use") or ( + isinstance(function, ast.Attribute) and function.attr == "use" + ): + first = node.args[0] if node.args else None + literal = isinstance(first, ast.Constant) and isinstance(first.value, str) + paths.append(first.value if literal else None) # type: ignore[union-attr] + return paths + + def calls_use(source: str) -> bool: try: tree = ast.parse(source) @@ -668,12 +746,14 @@ def _display(config: NotebookConfig, path: Path) -> str: "NameDep", "ancestors", "calls_use", + "use_calls", "cell_digest", "cell_module_files", "dependency_digest", "file_digest", "recorded_dependents", "deps_file", + "exclusive", "input_files", "local_modules", "read_dependencies", diff --git a/src/simplecadapi/runtime/executor.py b/src/simplecadapi/runtime/executor.py index d3e3900d..fc6d56fc 100644 --- a/src/simplecadapi/runtime/executor.py +++ b/src/simplecadapi/runtime/executor.py @@ -45,6 +45,7 @@ RunResult, safe_get_context, ) +from . import prefetch from .cache import CellCache, CellDigests, cache_root, prune from .cells import NotebookCells, bound_requirements from .config import NotebookConfig, read_notebook_config @@ -116,6 +117,7 @@ def execute_cell(self, cell: CellImpl, glbls: MutableGlobals) -> Any: if restored is not _MISS: return restored cache = cache if restored is _MISS else None + _prefetch(scope, cell, glbls) try: with _cell_recording(scope, cell, glbls): result = self._delegate.execute_cell(cell, glbls) @@ -134,6 +136,7 @@ async def execute_cell_async(self, cell: CellImpl, glbls: MutableGlobals) -> Any restored = _restore(cache, cell, glbls) if restored is not _MISS: return restored + _prefetch(scope, cell, glbls) try: with _cell_recording(scope, cell, glbls): result = await self._delegate.execute_cell_async(cell, glbls) @@ -160,6 +163,15 @@ def _cache_for(self, scope: NotebookScope, cell: CellImpl) -> CellCache | None: return _kernel_cache(scope, cell) +def _prefetch(scope: NotebookScope, cell: CellImpl, glbls: MutableGlobals) -> None: + """Start the notebooks the cell is about to ``use`` in worker processes.""" + + try: + prefetch.start(scope.config.directory, cell.code, glbls, scope.config.id) + except Exception: # noqa: BLE001 - an optimisation, never a reason for a cell to fail + pass + + _MISS = object() """Returned by :func:`_restore` when the cell must run.""" diff --git a/src/simplecadapi/runtime/prefetch.py b/src/simplecadapi/runtime/prefetch.py new file mode 100644 index 00000000..f0d39177 --- /dev/null +++ b/src/simplecadapi/runtime/prefetch.py @@ -0,0 +1,418 @@ +"""Run the notebooks a cell uses side by side. + +OpenCASCADE does not let go of the interpreter lock (two threads building +the same kind of model are no faster than one), so the parallelism is in +processes. When a cell is about to run, the ``use`` calls it is going to make +that can be told beforehand (:func:`plan_uses`: literal paths, arguments made +of values the cell reads) are handed to worker processes. A worker runs the +notebook headlessly, which fills its cell cache; the cell then makes its +``use`` calls as it always did, waits for the worker of each (:func:`wait`), +and finds the notebook built. Nothing about the result depends on the +workers: a failed or missing one only means the call runs the notebook itself. + +``SCA_JOBS`` sets how many workers (default: the smaller of 4 and the number +of cores; 1 turns this off). A notebook that has not changed since it last +ran, with the same overrides, is not started: it is in the cache already. +""" + +from __future__ import annotations + +import ast +import hashlib +import json +import os +import pickle +import struct +import subprocess +import sys +import threading +from collections.abc import Mapping +from concurrent.futures import Future, ThreadPoolExecutor +from dataclasses import dataclass, replace +from pathlib import Path +from typing import Any + +from ..artifacts.canonical import validate_logical_id +from .children import notebook_digest +from .config import read_notebook_config +from .deps import state_dir, use_calls + +# --------------------------------------------------------------------------- +# What a cell is going to use +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class UseRequest: + path: Path + id: str | None + overrides: Mapping[str, Any] + + @property + def key(self) -> tuple[str, str | None, str]: + return request_key(self.path, self.id, self.overrides) + + +def request_key(path: Path, id: str | None, overrides: Mapping[str, Any]) -> tuple[str, str | None, str]: + return (str(Path(path).resolve()), id, repr(sorted(overrides.items(), key=lambda item: item[0]))) + + +class _Unknown(Exception): + """An expression the plan cannot evaluate before the cell runs.""" + + +_CALLS = {"float": float, "int": int, "str": str, "bool": bool, "round": round, "abs": abs, "min": min, "max": max} +_BINARY = { + ast.Add: lambda a, b: a + b, + ast.Sub: lambda a, b: a - b, + ast.Mult: lambda a, b: a * b, + ast.Div: lambda a, b: a / b, + ast.FloorDiv: lambda a, b: a // b, + ast.Mod: lambda a, b: a % b, + ast.Pow: lambda a, b: a**b, +} +_COMPARE = { + ast.Eq: lambda a, b: a == b, + ast.NotEq: lambda a, b: a != b, + ast.Lt: lambda a, b: a < b, + ast.LtE: lambda a, b: a <= b, + ast.Gt: lambda a, b: a > b, + ast.GtE: lambda a, b: a >= b, +} +_NOT_RUN_HERE = ( + ast.IfExp, ast.If, ast.For, ast.AsyncFor, ast.While, ast.Try, ast.FunctionDef, + ast.AsyncFunctionDef, ast.ClassDef, ast.Lambda, ast.ListComp, ast.SetComp, + ast.DictComp, ast.GeneratorExp, ast.Match, +) # fmt: skip + + +def _evaluate(node: ast.expr, scope: Mapping[str, Any], notebook_id: str | None = None) -> Any: + """The value of *node*, for the few expression forms a plan can trust.""" + + if isinstance(node, ast.Constant): + return node.value + if ( + isinstance(node, ast.Call) + and not node.args + and not node.keywords + and ( + (isinstance(node.func, ast.Name) and node.func.id == "notebook_id") + or (isinstance(node.func, ast.Attribute) and node.func.attr == "notebook_id") + ) + ): + if notebook_id is None: + raise _Unknown + return notebook_id + if isinstance(node, ast.Name): + if node.id in scope: + return scope[node.id] + raise _Unknown + if isinstance(node, ast.UnaryOp) and isinstance(node.op, (ast.USub, ast.UAdd, ast.Not)): + value = _evaluate(node.operand, scope, notebook_id) + return {ast.USub: lambda: -value, ast.UAdd: lambda: +value, ast.Not: lambda: not value}[type(node.op)]() + if isinstance(node, ast.BinOp) and type(node.op) in _BINARY: + return _BINARY[type(node.op)](_evaluate(node.left, scope, notebook_id), _evaluate(node.right, scope, notebook_id)) + if isinstance(node, ast.Compare) and len(node.ops) == 1 and type(node.ops[0]) in _COMPARE: + return _COMPARE[type(node.ops[0])](_evaluate(node.left, scope, notebook_id), _evaluate(node.comparators[0], scope, notebook_id)) + if isinstance(node, ast.IfExp): + return _evaluate(node.body if _evaluate(node.test, scope, notebook_id) else node.orelse, scope, notebook_id) + if isinstance(node, (ast.Tuple, ast.List)): + items = [_evaluate(item, scope, notebook_id) for item in node.elts] + return tuple(items) if isinstance(node, ast.Tuple) else items + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id in _CALLS + and node.func.id not in scope + and not node.keywords + ): + return _CALLS[node.func.id](*[_evaluate(arg, scope, notebook_id) for arg in node.args]) + raise _Unknown + + +def plan_uses( + code: str, scope: Mapping[str, Any], directory: Path, notebook_id: str | None = None +) -> list[UseRequest]: + """The ``use`` calls *code* will make that are known before it runs. + + A call counts when its path is a literal and every argument can be worked + out from *scope* (the values the cell reads) with arithmetic and + ``float(...)``-like calls; and when the cell is certain to make it (not + under a condition, in a loop or in a function). + """ + + try: + tree = ast.parse(code) + except (SyntaxError, ValueError): + return [] + requests: dict[tuple[str, str | None, str], UseRequest] = {} + + def visit(node: ast.AST) -> None: + if isinstance(node, _NOT_RUN_HERE): + return + if isinstance(node, ast.Call) and _is_use(node): + request = _request(node, scope, directory, notebook_id) + if request is not None: + requests.setdefault(request.key, request) + for child in ast.iter_child_nodes(node): + visit(child) + + visit(tree) + return list(requests.values()) + + +def _is_use(node: ast.Call) -> bool: + function = node.func + return (isinstance(function, ast.Name) and function.id == "use") or ( + isinstance(function, ast.Attribute) and function.attr == "use" + ) + + +def _request( + node: ast.Call, scope: Mapping[str, Any], directory: Path, notebook_id: str | None +) -> UseRequest | None: + if not node.args or not (isinstance(node.args[0], ast.Constant) and isinstance(node.args[0].value, str)): + return None + if len(node.args) != 1 or any(keyword.arg is None for keyword in node.keywords): + return None + try: + values = {keyword.arg: _evaluate(keyword.value, scope, notebook_id) for keyword in node.keywords} + except Exception: # noqa: BLE001 - anything wrong here only means "not planned" + return None + child_id = values.pop("id", None) + if child_id is not None and not isinstance(child_id, str): + return None + path = Path(node.args[0].value) + return UseRequest((path if path.is_absolute() else directory / path).resolve(), child_id, values) + + +# --------------------------------------------------------------------------- +# Has the notebook changed since it last ran? +# --------------------------------------------------------------------------- + + +def _signature(path: Path, config_id: str, overrides: Mapping[str, Any]) -> str | None: + digest = notebook_digest(path) + if digest is None: + return None + payload = repr((digest, config_id, sorted(overrides.items(), key=lambda item: item[0]))) + return hashlib.sha256(payload.encode("utf-8")).hexdigest()[:20] + + +def _stamp_file(config: Any) -> Path: + return state_dir(config) / "stamp.json" + + +def _read_stamp(config: Any) -> dict[str, Any]: + try: + data = json.loads(_stamp_file(config).read_text(encoding="utf-8")) + except (OSError, ValueError): + return {} + return data if isinstance(data, dict) else {} + + +def write_stamp(config: Any, overrides: Mapping[str, Any], seconds: float | None = None) -> None: + """Record that *config*'s notebook has just run to the end with *overrides*. + + *seconds* is how long the run took when it built something; a run that + only restored keeps the figure of the last build (it says what the + notebook costs, which is what decides whether a worker is worth it). + """ + + signature = _signature(config.path, config.id, overrides) + target = _stamp_file(config) + try: + if signature is None: + target.unlink(missing_ok=True) + return + previous = _read_stamp(config) + if seconds is None: + seconds = previous.get("seconds") + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(json.dumps({"signature": signature, "seconds": seconds}), encoding="utf-8") + except OSError: + pass # the stamp only saves work + + +def _expected_seconds(request: UseRequest) -> float | None: + """How long the notebook has to be built, if it has to be: ``None`` when it + does not (it is in the cache), ``inf`` when nobody knows how long.""" + + config = read_notebook_config(request.path) + if config is None: + return None # the call itself will say what is wrong with it + try: + if request.id is not None: + config = replace(config, id=validate_logical_id(request.id, "/run/id")) + except Exception: # noqa: BLE001 + return None + signature = _signature(config.path, config.id, request.overrides) + stamp = _read_stamp(config) + if signature is not None and stamp.get("signature") == signature: + return None + seconds = stamp.get("seconds") + return float(seconds) if isinstance(seconds, (int, float)) else float("inf") + + +# --------------------------------------------------------------------------- +# Workers +# --------------------------------------------------------------------------- + +_COLD_WORTH = 4.0 +"""Seconds a start of the workers has to save to be done (a worker imports the +kernel libraries: a few seconds).""" +_WARM_WORTH = 0.5 +"""...and once they are running.""" + +_stats: dict[str, list[str]] = {"submitted": []} +_pending: dict[tuple[str, str | None, str], Future[None]] = {} +_lock = threading.Lock() +_pool: _Pool | None = None + + +def jobs() -> int: + raw = os.environ.get("SCA_JOBS") + if raw is not None: + try: + return max(1, int(raw)) + except ValueError: + return 1 + return max(1, min(4, os.cpu_count() or 1)) + + +def stats() -> dict[str, list[str]]: + return {"submitted": list(_stats["submitted"])} + + +def reset_stats() -> None: + _stats["submitted"].clear() + + +def start(directory: Path, code: str, scope: Mapping[str, Any], notebook_id: str | None = None) -> None: + """Hand the ``use`` calls of a cell about to run to the workers.""" + + global _pool + size = jobs() + if size < 2 or "use" not in code: + return + if not use_calls(code): + return + planned = [(request, _expected_seconds(request)) for request in plan_uses(code, scope, directory, notebook_id)] + wanted = [(request, seconds) for request, seconds in planned if seconds is not None] + if len(wanted) < 2: + return # nothing to do beside the cell's own first call + # The longest build goes on in this process; what the workers take off its + # hands has to be worth more than starting them. + seconds = sorted((value for _, value in wanted), reverse=True) + saved = sum(seconds[1:]) + if saved < (_WARM_WORTH if _pool is not None and _pool.size == size else _COLD_WORTH): + return + requests = [request for request, _ in wanted] + with _lock: + if _pool is None or _pool.size != size: + _pool = _Pool(size) + for request in requests: + existing = _pending.get(request.key) + if existing is not None and not existing.done(): + continue + _pending[request.key] = _pool.submit(request) + _stats["submitted"].append(str(request.path)) + + +def wait(path: Path, id: str | None, overrides: Mapping[str, Any]) -> None: + """Wait for the worker that is building this notebook, if one is.""" + + with _lock: + future = _pending.pop(request_key(path, id, overrides), None) + if future is not None: + try: + future.result() + except Exception: # noqa: BLE001 - the call runs the notebook and reports for itself + pass + + +def shutdown() -> None: + global _pool + with _lock: + pool, _pool = _pool, None + _pending.clear() + if pool is not None: + pool.close() + + +class _Pool: + """Threads that each own a worker process, started when first needed.""" + + def __init__(self, size: int) -> None: + self.size = size + self._executor = ThreadPoolExecutor(size, thread_name_prefix="sca-prefetch") + self._local = threading.local() + self._workers: list[subprocess.Popen[bytes]] = [] + self._workers_lock = threading.Lock() + + def submit(self, request: UseRequest) -> Future[None]: + return self._executor.submit(self._run, request) + + def _run(self, request: UseRequest) -> None: + worker = getattr(self._local, "worker", None) + if worker is None or worker.poll() is not None: + worker = self._local.worker = self._spawn() + try: + _send(worker.stdin, (str(request.path), request.id, dict(request.overrides))) # type: ignore[arg-type] + reply = _receive(worker.stdout) # type: ignore[arg-type] + except Exception: + self._local.worker = None + worker.kill() + raise + if reply[0] != "ok": + raise RuntimeError(reply[1]) + + def _spawn(self) -> subprocess.Popen[bytes]: + worker = subprocess.Popen( + [sys.executable, "-m", "simplecadapi.runtime._prefetch_worker"], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + env={**os.environ, "SCA_JOBS": "1"}, # a worker does not start workers + ) + with self._workers_lock: + self._workers.append(worker) + return worker + + def close(self) -> None: + self._executor.shutdown(wait=False, cancel_futures=True) + with self._workers_lock: + workers, self._workers = self._workers, [] + for worker in workers: + try: + worker.kill() + except OSError: + pass + + +def _send(stream: Any, value: Any) -> None: + data = pickle.dumps(value) + stream.write(struct.pack(">Q", len(data)) + data) + stream.flush() + + +def _receive(stream: Any) -> Any: + header = stream.read(8) + if len(header) < 8: + raise EOFError("the worker ended") + (size,) = struct.unpack(">Q", header) + return pickle.loads(stream.read(size)) + + +__all__ = [ + "UseRequest", + "jobs", + "plan_uses", + "request_key", + "reset_stats", + "shutdown", + "start", + "stats", + "wait", + "write_stamp", +] diff --git a/src/simplecadapi/runtime/runner.py b/src/simplecadapi/runtime/runner.py index b6532213..fa3ba513 100644 --- a/src/simplecadapi/runtime/runner.py +++ b/src/simplecadapi/runtime/runner.py @@ -24,6 +24,7 @@ import hashlib import sys +import time from collections.abc import Callable, Iterator, Mapping from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager @@ -53,11 +54,13 @@ safe_get_context, suppress_warnings_logs, ) +from . import prefetch from .cache import CellCache, CellDigests, cache_root, prune from .cells import NotebookCells from .config import NotebookConfig, NotebookConfigError, read_notebook_config from .deps import ( local_modules, + exclusive, read_dependencies, record_dependencies, recording_dependencies, @@ -163,7 +166,7 @@ def run() -> NotebookRun: # Errors still come through. "marimo" is the logger every marimo # module shares (lazy.py's record shows its file name, "lazy", which # is not the logger's name). - with suppress_warnings_logs("marimo"): + with suppress_warnings_logs("marimo"), exclusive(config): return _run(config, values, cache=cache, through=through, product=product) # marimo keeps its runtime context per thread, and a notebook run needs @@ -188,6 +191,7 @@ def use(path: str | Path, /, *, id: str | None = None, **overrides: Any) -> Prod target = Path(path) if not target.is_absolute(): target = _calling_directory() / target + prefetch.wait(target, id, overrides) # a worker may be building it child = run_notebook(target, id=id, overrides=overrides) # use() never asks for a partial run, so the product is always there. if child.product is None: @@ -209,6 +213,8 @@ def _run( through: int | None = None, product: bool = True, ) -> NotebookRun: + started = time.perf_counter() + given = dict(overrides) # (``overrides`` becomes the run's values below) app = load_notebook(config.path, cell_prefix=_cell_prefix(config)) if app is None: raise NotebookConfigError(f"{config.path}: the notebook has no cells") @@ -310,6 +316,9 @@ def _run( ) for cell in ordered ) + if product and through is None and cache: + built = any(report.status == "ran" and report.name != "setup" for report in reports) + prefetch.write_stamp(config, given, time.perf_counter() - started if built else None) return NotebookRun( config=config, product=product_value, diff --git a/test/conftest.py b/test/conftest.py index 2494f6e8..253028ce 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -11,3 +11,8 @@ # with the agent-facing default. os.environ.setdefault("SCA_NO_DIAGNOSTIC_RENDER", "1") os.environ.setdefault("SCA_DIAGNOSTIC_RENDER_BUDGET", "60") + +# Worker processes for the notebooks a cell uses are for the tests that are +# about them (they set SCA_JOBS themselves): starting several interpreters per +# run would only slow the rest of the suite down. +os.environ.setdefault("SCA_JOBS", "1") diff --git a/test/test_state_lock.py b/test/test_state_lock.py new file mode 100644 index 00000000..559cd8c1 --- /dev/null +++ b/test/test_state_lock.py @@ -0,0 +1,51 @@ +"""One process at a time in a notebook's state directory. + +Workers (see ``prefetch``) may run the same notebook at once, and a run +prunes the cache directories it did not use. The lock is between processes: +inside one, a nested run of the same notebook (a ``use`` cycle) goes on as it +always did, and is reported, not waited for. +""" + +from __future__ import annotations + +import subprocess +import sys +import textwrap +import time + +from simplecadapi.runtime.config import read_notebook_config +from simplecadapi.runtime.deps import exclusive +from test_notebook_runtime import NotebookTestCase, cell, notebook_text + + +class TestStateLock(NotebookTestCase): + def setUp(self) -> None: + super().setUp() + self.path = self.write("n.py", notebook_text(cell("x = 1", returns="x"))) + self.config = read_notebook_config(self.path) + + def test_a_nested_run_of_the_same_notebook_is_not_waited_for(self) -> None: + with exclusive(self.config): + with exclusive(self.config): + pass + + def test_another_process_waits_for_the_holder(self) -> None: + holder = textwrap.dedent( + f""" + import sys, time + from simplecadapi.runtime.config import read_notebook_config + from simplecadapi.runtime.deps import exclusive + with exclusive(read_notebook_config({str(self.path)!r})): + print("locked", flush=True) + time.sleep(1.5) + """ + ) + process = subprocess.Popen([sys.executable, "-c", holder], stdout=subprocess.PIPE, text=True) + try: + self.assertEqual(process.stdout.readline().strip(), "locked") + started = time.monotonic() + with exclusive(self.config): + waited = time.monotonic() - started + self.assertGreater(waited, 0.8) + finally: + process.wait(timeout=30) diff --git a/test/test_use_digests.py b/test/test_use_digests.py new file mode 100644 index 00000000..d4a18f4a --- /dev/null +++ b/test/test_use_digests.py @@ -0,0 +1,110 @@ +"""`use` in the cache: a cell depends on the notebooks it uses, one by one. + +The path of a ``use`` call is usually a literal, so which notebooks a cell +depends on is known before it runs: the cell is keyed by what each of them is +(its code, not its comments or layout; the modules it imports; the notebooks +it uses in turn). What is computed at run time is recorded by a run, as +before. +""" + +from __future__ import annotations + +from simplecadapi.runtime import run_notebook +from simplecadapi.runtime._marimo import InternalApp, load_notebook +from simplecadapi.runtime.cache import CellDigests +from simplecadapi.runtime.config import read_notebook_config +from simplecadapi.runtime.runner import _cell_prefix +from test_notebook_runtime import NotebookTestCase, cell, notebook_text + + +def child(name: str, width: float, note: str = "") -> str: + return notebook_text( + cell(f"width = {width}{note}", returns="width"), + cell("block = scad.make_box_rsolid(width, 4.0, 4.0)", reads="width", returns="block"), + cell(f"part = scad.Part(part_id='{name}', body=block)", reads="block", returns="part"), + table=f'id = "{name}"', + ) + + +ASSEMBLE = """ + _asm = scad.make_assembly_rassembly(assembly_id="top", name="top") + for _key, _item in (("a", a), ("b", b)): + _asm = scad.add_component_rassembly( + assembly=_asm, item=_item, component_id=_key, placement=scad.identity_placement_rplacement() + ) + top = _asm +""" + + +def parent(first: str = 'scad.use("child_a.py")', second: str = 'scad.use("child_b.py")', *, before: str = "") -> str: + cells = [cell(f"{before}\na = {first}" if before else f"a = {first}", returns="a")] + cells += [cell(f"b = {second}", returns="b"), cell(ASSEMBLE, reads="a, b", returns="top")] + return notebook_text(*cells, table='id = "top"') + + +class TestStaticUse(NotebookTestCase): + def setUp(self) -> None: + super().setUp() + self.write("child_a.py", child("child_a", 10.0)) + self.write("child_b.py", child("child_b", 6.0)) + self.top = self.write("top.py", parent()) + + def edited(self) -> list[str]: + run = run_notebook(self.top) + fresh = run_notebook(self.top, cache=False) + self.assertEqual(run.definition.content_hash, fresh.definition.content_hash) + return self.statuses(run) + + def test_editing_one_child_redoes_only_the_cell_that_uses_it(self) -> None: + run_notebook(self.top) + self.write("child_a.py", child("child_a", 12.0)) + # setup, use a, use b, the assembly + self.assertEqual(self.edited(), ["ran", "ran", "cached", "ran"]) + + def test_a_comment_in_a_child_is_a_change_like_any_other(self) -> None: + # The definition records where in the file each step is: a parent that + # embeds the child must equal a fresh run, comment or not. + run_notebook(self.top) + self.write("child_a.py", child("child_a", 10.0, note=" # the width, in mm")) + self.assertEqual(self.edited(), ["ran", "ran", "cached", "ran"]) + + def test_a_module_a_child_imports_is_followed(self) -> None: + self.write("lib/size.py", "SIZE = 5.0\n") + text = child("child_a", 10.0).replace( + "with app.setup:\n import simplecadapi as scad\n", + "with app.setup:\n import simplecadapi as scad\n from lib.size import SIZE\n", + ).replace("make_box_rsolid(width, 4.0, 4.0)", "make_box_rsolid(width, SIZE, 4.0)") + self.write("child_a.py", text) + run_notebook(self.top) + self.write("lib/size.py", "SIZE = 6.0\n") + self.assertEqual(self.edited(), ["ran", "ran", "cached", "ran"]) + + def test_a_grandchild_is_followed_through_the_child(self) -> None: + self.write("deep.py", child("deep", 3.0)) + text = child("child_a", 10.0).replace( + "block = scad.make_box_rsolid(width, 4.0, 4.0)", + 'deep = scad.use("deep.py")\n block = scad.make_box_rsolid(width, 4.0, 4.0)', + ) + self.write("child_a.py", text) + run_notebook(self.top) + self.write("deep.py", child("deep", 4.0)) + self.assertEqual(self.edited(), ["ran", "ran", "cached", "ran"]) + + def test_a_path_computed_at_run_time_is_still_followed(self) -> None: + self.write("top.py", parent(first='scad.use(_name + ".py")', before='_name = "child_a"')) + run_notebook(self.top) + self.write("child_a.py", child("child_a", 12.0)) + statuses = self.edited() + self.assertEqual(statuses[1], "ran") + self.assertEqual(statuses[3], "ran") + + def test_the_kernel_keys_a_static_use_cell_and_runs_a_dynamic_one_live(self) -> None: + for source, expected in ((parent(), True), (parent(first='scad.use(_name + ".py")', before='_name = "child_a"'), False)): + path = self.write("probe.py", source) + config = read_notebook_config(path) + internal = InternalApp(load_notebook(path, cell_prefix=_cell_prefix(config))) + ordered = [internal.graph.cells[i] for i in internal.cell_manager.cell_ids() if i in internal.graph.cells] + setup = internal.graph.cells.get(internal.cell_manager.setup_cell_id) + digests = CellDigests(config, ordered, setup, kernel=True) + first_use = next(c for c in ordered if "use(" in c.code) + self.assertEqual(digests.digest(first_use.cell_id) is not None, expected) diff --git a/test/test_use_prefetch.py b/test/test_use_prefetch.py new file mode 100644 index 00000000..18bb1fd3 --- /dev/null +++ b/test/test_use_prefetch.py @@ -0,0 +1,158 @@ +"""Running the notebooks a cell uses side by side. + +OpenCASCADE does not let go of the interpreter lock, so threads would not +help: the ``use`` calls of a cell that is about to run are started in worker +processes, which only fill the notebooks' cell caches. The cell then runs its +``use`` calls as it always did, and finds the work done. Whatever the workers +do (or fail to do), the result is the one a serial run gives. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +import unittest +from pathlib import Path +from unittest import mock + +from simplecadapi.runtime import prefetch, run_notebook +from test_notebook_runtime import NotebookTestCase, notebook_text +from test_use_digests import ASSEMBLE, child, parent + + +class TestPlan(unittest.TestCase): + root = Path("/proj") + + def plan(self, code: str, scope: dict | None = None): + return prefetch.plan_uses(code, scope or {}, self.root) + + def test_literal_paths_and_values_known_before_the_cell_runs(self) -> None: + requests = self.plan( + 'a = scad.use("child_a.py")\nb = scad.use("sub/b.py", id="x", WIDTH=float(W), N=3)', + {"W": 4}, + ) + self.assertEqual([r.path for r in requests], [self.root / "child_a.py", self.root / "sub/b.py"]) + self.assertEqual((requests[1].id, requests[1].overrides), ("x", {"WIDTH": 4.0, "N": 3})) + + def test_the_id_of_the_running_notebook_is_known(self) -> None: + requests = prefetch.plan_uses( + 'a = scad.use("c.py", id=scad.notebook_id() + "-body", LENGTH=float(L))', {"L": 2}, self.root, notebook_id="arm" + ) + self.assertEqual([(r.id, dict(r.overrides)) for r in requests], [("arm-body", {"LENGTH": 2.0})]) + self.assertEqual(self.plan('a = scad.use("c.py", id=scad.notebook_id())'), []) # not told which + + def test_what_cannot_be_known_is_left_to_the_cell(self) -> None: + for code in ( + 'a = scad.use(name + ".py")', # computed path + 'a = scad.use("c.py") if flag else None', # may not run + 'for n in names:\n scad.use("c.py")', # a loop + 'a = scad.use("c.py", LENGTH=other.size())', # a call the plan will not make + 'a = scad.use("c.py", LENGTH=missing)', # a name the cell defines itself + "def f():\n return scad.use('c.py')", # not run by the cell + ): + with self.subTest(code=code): + self.assertEqual(self.plan(code, {"flag": True, "names": [1], "other": object()}), []) + + def test_two_requests_for_one_run_are_one(self) -> None: + requests = self.plan('a = scad.use("c.py", W=1.0)\nb = scad.use("c.py", W=1.0)\nc = scad.use("c.py", W=2.0)') + self.assertEqual(len({r.key for r in requests}), 2) + + +class TestPrefetchedRun(NotebookTestCase): + def setUp(self) -> None: + super().setUp() + patcher = mock.patch.dict(os.environ, {"SCA_JOBS": "2"}) + patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(prefetch.shutdown) + prefetch.reset_stats() + self.write("child_a.py", child("child_a", 10.0)) + self.write("child_b.py", child("child_b", 6.0)) + self.top = self.write("top.py", self.parent_text()) + + @staticmethod + def parent_text() -> str: + # both uses in one cell, as in a real assembly + return notebook_text( + "@app.cell\ndef _():\n a = scad.use('child_a.py')\n b = scad.use('child_b.py')\n return a, b\n", + "@app.cell\ndef _(a, b):\n" + ASSEMBLE.rstrip("\n") + "\n return (top,)\n", + table='id = "top"', + ) + + def test_a_cell_with_two_uses_starts_them_together_and_gives_the_serial_result(self) -> None: + run = run_notebook(self.top) + self.assertEqual(sorted(Path(p).name for p in prefetch.stats()["submitted"]), ["child_a.py", "child_b.py"]) + serial = run_notebook(self.top, cache=False) + self.assertEqual(run.definition.content_hash, serial.definition.content_hash) + + def test_a_notebook_that_has_not_changed_since_it_last_ran_is_not_started_again(self) -> None: + run_notebook(self.top) + prefetch.reset_stats() + run_notebook(self.top, cache=False) # the cell runs again; the children are as they were + self.assertEqual(prefetch.stats()["submitted"], []) + + def stamped(self, seconds: float) -> None: + """Make both children look like they last took *seconds* to build.""" + + from simplecadapi.runtime.config import read_notebook_config + + for name in ("child_a.py", "child_b.py"): + stamp = prefetch._stamp_file(read_notebook_config(self.root / name)) + stamp.parent.mkdir(parents=True, exist_ok=True) + stamp.write_text(json.dumps({"signature": "stale", "seconds": seconds})) + + def test_notebooks_that_build_in_moments_are_not_worth_a_worker(self) -> None: + # a worker takes seconds to start; two builds of a fifth of a second do not repay it + self.stamped(0.2) + run_notebook(self.top) + self.assertEqual(prefetch.stats()["submitted"], []) + + def test_notebooks_that_took_long_are_started_together(self) -> None: + self.stamped(20.0) + run_notebook(self.top) + self.assertEqual(len(prefetch.stats()["submitted"]), 2) + + def test_a_run_remembers_how_long_it_took_to_build(self) -> None: + from simplecadapi.runtime.config import read_notebook_config + + run_notebook(self.root / "child_a.py") + stamp = json.loads(prefetch._stamp_file(read_notebook_config(self.root / "child_a.py")).read_text()) + self.assertGreater(stamp["seconds"], 0.0) + # a run that only restores does not make it look cheap + run_notebook(self.root / "child_a.py", cache=True) + again = json.loads(prefetch._stamp_file(read_notebook_config(self.root / "child_a.py")).read_text()) + self.assertEqual(again["seconds"], stamp["seconds"]) + + def test_a_failing_child_is_reported_by_the_run_that_uses_it(self) -> None: + self.write("child_b.py", child("child_b", 6.0).replace("make_box_rsolid(width, 4.0, 4.0)", "make_box_rsolid(width, -4.0, 4.0)")) + with self.assertRaises(Exception) as caught: + run_notebook(self.top) + # the error of the child itself, not something about a worker + self.assertIn("greater than zero", str(caught.exception)) + + def test_one_use_is_not_worth_a_worker(self) -> None: + one = notebook_text( + "@app.cell\ndef _():\n a = scad.use('child_a.py')\n return (a,)\n", + "@app.cell\ndef _(a):\n" + ' _asm = scad.make_assembly_rassembly(assembly_id="single", name="single")\n' + ' single = scad.add_component_rassembly(assembly=_asm, item=a, component_id="a", ' + "placement=scad.identity_placement_rplacement())\n" + " return (single,)\n", + table='id = "single"', + ) + run_notebook(self.write("single.py", one)) + self.assertEqual(prefetch.stats()["submitted"], []) + + def test_the_command_line_prints_only_its_report(self) -> None: + chatty = child("child_a", 10.0).replace("width = 10.0", "print('from the child')\n width = 10.0") + self.write("child_a.py", chatty) + done = subprocess.run( + [sys.executable, "-c", "import sys; from simplecadapi.cli import main; sys.exit(main(['run', sys.argv[1]]))", str(self.top)], + capture_output=True, text=True, env={**os.environ, "SCA_JOBS": "2"}, timeout=300, + ) + self.assertEqual(done.returncode, 0, done.stderr[-2000:]) + lines = [line for line in done.stdout.splitlines() if line.strip()] + self.assertEqual(json.loads(lines[-1])["definition_id"], "top") + self.assertNotIn("from the child", done.stdout) # a worker's output goes nowhere From 302e62195d889c9b8c18d2d146b5614ba2ab0a90 Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Wed, 21 Oct 2026 00:34:40 +0800 Subject: [PATCH 09/10] docs: 2.2.6.post1 notes cover the cache and use speed-up Co-Authored-By: Claude Sonnet 5.5 --- README.md | 5 +++++ README.zh-CN.md | 4 ++++ docs/updates/2.2.6.post1.md | 31 ++++++++++++++++++++++++++++++- docs/updates/2.2.6.post1.zh-CN.md | 23 ++++++++++++++++++++++- 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 68f3a6a1..58a1b55f 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,11 @@ the full crank cycle sweeps clean in the same sim · - **Keyword `scad.var` is a parameter again.** `scad.var(name="j1", default=0.0)` is read by the editor's parameter panel like the positional form; before, it was skipped. +- **A cached run costs much less.** On the desktop arm: cold 80 s to 35 s, nothing changed + 24 s to 4 s, one joint edited 18 s to 3 s, cache 1.7 GB to 96 MB. Restored shapes are built + on first use, parameters and imported names are keyed by value, and the `use` calls of a + cell run in worker processes (`SCA_JOBS`, 1 turns it off). +- **Slider drags are sent when they settle**, not step by step. Full notes for this version: [`docs/updates/2.2.6.post1.md`](docs/updates/2.2.6.post1.md). Earlier versions: the [changelog](CHANGELOG.md) and [`docs/updates/`](docs/updates/). diff --git a/README.zh-CN.md b/README.zh-CN.md index 00125b46..24d9dc35 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -139,6 +139,10 @@ CalculiX 静力 FEM（边界面按保真的 `interface.*` 标签选取，仿真 - **关键字写法的 `scad.var` 重新被识别为参数。** `scad.var(name="j1", default=0.0)` 现在和 位置参数写法一样出现在参数面板；之前会被跳过。 +- **缓存运行便宜得多。** 桌面机械臂：冷启动 80 秒到 35 秒，什么都没改 24 秒到 4 秒，改一个关节 18 秒到 3 秒， + 缓存 1.7 GB 到 96 MB。形状在第一次用到时才构建，参数和导入的名字按值定键，cell 里的 `use` 调用在 worker + 进程里并行（`SCA_JOBS`，设为 1 关闭）。 +- **滑块拖动在停下来时才发送**，不再逐步发送。 本版本的完整说明：[`docs/updates/2.2.6.post1.zh-CN.md`](docs/updates/2.2.6.post1.zh-CN.md)。 更早的版本：见[更新日志](CHANGELOG.md)和 [`docs/updates/`](docs/updates/)。 diff --git a/docs/updates/2.2.6.post1.md b/docs/updates/2.2.6.post1.md index 8bb1fa86..1197a515 100644 --- a/docs/updates/2.2.6.post1.md +++ b/docs/updates/2.2.6.post1.md @@ -1,6 +1,7 @@ # 2.2.6.post1 Update Notes -A hotfix for the editor's parameter panel. +A hotfix for the editor's parameter panel, and a round of speed work on the notebook cache +and on `use`, measured on the desktop six-axis arm. ## `scad.var(name=..., default=...)` is a parameter again @@ -13,3 +14,31 @@ J1 = scad.var(name="j1", default=0.0, comment="Joint 1") — was skipped: no slider, no commit back to the file. Both forms are read now, and committing a value rewrites the right argument in either. + +## What a cached run costs + +| desktop arm | before | now | +|---|---|---| +| cold run | 80.5 s | 34.6 s | +| nothing changed | 23.6 s | 4.0 s | +| one joint angle edited | 18.2 s | 3.0 s | +| a link length edited | 35.1 s | 19 s | +| cache on disk | 1.7 GB | 96 MB | + +- **Restoring is cheaper.** A restored shape is built when something first uses it, so a + cached value no running cell reads costs nothing; the tag caches are rebuilt once, not + once per ancestor. The cache no longer stores a cell's private (`_name`) values and + compresses what it writes; entries from before are still read. +- **Parameters are keyed by value.** A cell that reads one `scad.var` is not redone because a + neighbour defined in the same cell changed. +- **A name, not a file.** `from lib.dims import A` depends on the definition of `A`. +- **`use` is followed one call at a time.** A cell that uses a notebook by a literal path + depends on that notebook (its text, the modules it imports, the notebooks it uses): editing + one child redoes the cell that uses it, not its siblings. The editor kernel caches such + cells too. +- **`use` calls run side by side.** The `use` calls a cell is about to make are started in + worker processes (threads would not help: OpenCASCADE keeps the interpreter lock) and the + cell finds them built. `SCA_JOBS` sets the number (default: the smaller of 4 and the + cores; `1` turns it off). Workers are started only when that saves more than they take to + start, and a notebook is locked between processes while it runs. +- **Editor sliders.** A drag is sent when it pauses (250 ms) or ends, not step by step. diff --git a/docs/updates/2.2.6.post1.zh-CN.md b/docs/updates/2.2.6.post1.zh-CN.md index 246171ac..3be2ab34 100644 --- a/docs/updates/2.2.6.post1.zh-CN.md +++ b/docs/updates/2.2.6.post1.zh-CN.md @@ -1,6 +1,6 @@ # 2.2.6.post1 更新说明 -编辑器参数面板的热修复。 +编辑器参数面板的热修复，以及一轮针对 notebook 缓存和 `use` 的速度优化，数据来自桌面六轴机械臂。 ## `scad.var(name=..., default=...)` 重新被识别为参数 @@ -11,3 +11,24 @@ J1 = scad.var(name="j1", default=0.0, comment="Joint 1") ``` ——会被跳过：没有滑块，也无法写回文件。现在两种写法都能读取，提交数值时也会改写对应的那个参数。 + +## 缓存运行的代价 + +| 桌面机械臂 | 之前 | 现在 | +|---|---|---| +| 冷启动 | 80.5 s | 34.6 s | +| 什么都没改 | 23.6 s | 4.0 s | +| 改一个关节角 | 18.2 s | 3.0 s | +| 改一个连杆长度 | 35.1 s | 19 s | +| 磁盘缓存 | 1.7 GB | 96 MB | + +- **恢复更便宜。** 恢复出来的形状在第一次被用到时才构建，没有 cell 读取的缓存值不花时间；标签缓存只重建一次， + 不再每个祖先一次。缓存不再存 cell 私有（`_name`）变量的值，并压缩写入的内容；旧条目照样能读。 +- **参数按值定键。** 读取某个 `scad.var` 的 cell，不会因为同一个 cell 里的邻居参数变化而重跑。 +- **按名字而不是按文件。** `from lib.dims import A` 只依赖 `A` 的定义。 +- **`use` 按调用逐个跟踪。** 用字面量路径 `use` 一个 notebook 的 cell 依赖那个 notebook（文本、它导入的模块、 + 它再 `use` 的 notebook）：改一个子 notebook 只重跑用它的 cell，不动它的兄弟。编辑器内核也缓存这类 cell。 +- **`use` 并行。** cell 即将发出的 `use` 调用交给 worker 进程同时构建（线程没用：OpenCASCADE 不释放解释器锁）， + cell 随后发现已经建好。`SCA_JOBS` 设置数量（默认 4 与核数的较小值；`1` 关闭）。只有省下的时间大于 worker + 启动成本才会启动，并且 notebook 运行期间其状态目录在进程间加锁。 +- **编辑器滑块。** 拖动在停顿 250 ms 或松开时才发送，不再逐步发送。 From 96e9c080e62a0167b989758309ccb74e70dadeb7 Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Wed, 21 Oct 2026 00:47:46 +0800 Subject: [PATCH 10/10] fix(addon): accept PEP 440 post-release versions in compat ranges 2.2.6.post1 made every add-on command fail with VersionSpecError. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 2 ++ src/simplecadapi/addon/versions.py | 10 ++++++++++ test/test_addon_units.py | 21 ++++++++++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 139b9f4f..7a4ba008 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ user-facing notes live in [`docs/updates/`](docs/updates/) (English and - Editor: the parameter panel reads `scad.var(name="j1", default=0.0, ...)` — the keyword form — as well as `scad.var("j1", 0.0, ...)`. Before, only the second positional argument was taken as the default, so keyword declarations never showed up as parameters. +- Add-ons: `[compat] sca` ranges and the add-on commands accept a post-release version such as + `2.2.6.post1` (it sorts after `2.2.6` and before `2.2.7`); the parser rejected it. ### Changed diff --git a/src/simplecadapi/addon/versions.py b/src/simplecadapi/addon/versions.py index 5a866f73..3e227384 100644 --- a/src/simplecadapi/addon/versions.py +++ b/src/simplecadapi/addon/versions.py @@ -5,6 +5,9 @@ * versions are dot-separated numeric components with optional pre-release suffixes (``2.1.2``, ``2.0.4b2``); a suffix sorts before the same numeric component without one (``2.0.4b2 < 2.0.4``); +* a trailing ``.postN`` component is a post-release (PEP 440): it sorts + after the release it follows and before the next one + (``2.2.6 < 2.2.6.post1 < 2.2.6.post2 < 2.2.7``); * ranges are comma-separated clauses with the operators ``== != >= <= > <``; a bare version means exact equality; * clauses must be jointly satisfiable (``>=2.1,<2.0`` is rejected). @@ -21,6 +24,7 @@ _CLAUSE_RE = re.compile(r"^(==|!=|>=|<=|>|<)?\s*([0-9][0-9A-Za-z.]*)$") _COMPONENT_RE = re.compile(r"^([0-9]+)([a-zA-Z][0-9A-Za-z]*)?$") +_POST_RE = re.compile(r"^post([0-9]+)$") class VersionSpecError(AddonError): @@ -34,6 +38,12 @@ def version_key(text: str) -> tuple[tuple[int, int, str], ...]: raise VersionSpecError("empty version literal") components: list[tuple[int, int, str]] = [] for part in stripped.split("."): + post = _POST_RE.match(part) + if post is not None and components: + # after the release it follows (a missing component pads as (0, 1, "")), + # and in order among post-releases + components.append((int(post.group(1)), 2, "")) + continue match = _COMPONENT_RE.match(part) if match is None: raise VersionSpecError( diff --git a/test/test_addon_units.py b/test/test_addon_units.py index 7541444c..ab3b5f61 100644 --- a/test/test_addon_units.py +++ b/test/test_addon_units.py @@ -50,8 +50,27 @@ def test_prerelease_suffix_sorts_before_release(): assert compare_versions("2.1.2", "2.1.2") == 0 +def test_post_release_sorts_after_its_release_and_before_the_next(): + # the SDK itself is released as 2.2.6.post1 (PEP 440): compat ranges are + # checked against that version + assert compare_versions("2.2.6.post1", "2.2.6") == 1 + assert compare_versions("2.2.6", "2.2.6.post1") == -1 + assert compare_versions("2.2.6.post2", "2.2.6.post1") == 1 + assert compare_versions("2.2.6.post10", "2.2.6.post2") == 1 + assert compare_versions("2.2.7", "2.2.6.post9") == 1 + assert compare_versions("2.2.6.post1", "2.2.6.post1") == 0 + assert compare_versions("2.2.6b2", "2.2.6.post1") == -1 + + +def test_ranges_hold_for_a_post_release(): + assert VersionRange.parse(">=2.2.6,<2.2.7").matches("2.2.6.post1") + assert VersionRange.parse("==2.2.6.post1").matches("2.2.6.post1") + assert not VersionRange.parse("<=2.2.6").matches("2.2.6.post1") + assert not VersionRange.parse(">=2.2.7").matches("2.2.6.post1") + + @pytest.mark.parametrize( - "spec", ["", " ", "*", "2.*", ">=2.1,,3", ">=", "==x.y", "v2.1"] + "spec", ["", " ", "*", "2.*", ">=2.1,,3", ">=", "==x.y", "v2.1", "2.2.post", "2.2.postx"] ) def test_invalid_specs_are_rejected_with_named_error(spec): with pytest.raises(VersionSpecError):