From f34c60f7c617dc50c7a80f778b6ac6c4f2b15a54 Mon Sep 17 00:00:00 2001 From: Maxim Morozov Date: Fri, 14 Aug 2026 12:29:31 +0500 Subject: [PATCH 1/2] Skip revisiting cleared dependencies when validating resolution paths assert_valid_resolution_path detects cycles by walking the dependency graph depth-first, but it enumerates every distinct root-to-leaf path and never records that a node's subtree already came back clean. In a graph where many injectables share dependencies -- the normal shape once a few services sit on top of a common core -- each shared subgraph is re-walked once per incoming path, so the visit count grows with the number of paths through the graph rather than with its size. Container creation pays for this on every startup. Thread a set of cleared object ids through the walk and return early for any dependency already in it, adding to it only once the whole subtree has returned without raising. Skipping a cleared dependency is sound: were it part of a cycle, the walk would have come back to it while it was still on the current path, and that check -- along with the resulting error message -- is unchanged. Marking happens after the recursion, not before, so a node that is still on the stack is never treated as cleared. The set is created in validate_registry and lives for exactly one run. It is deliberately not module level: two registries describe two different graphs, and a node cleared in one says nothing about the other. On a synthetic graph of 316 injectables (fan-out 3, depth 9) the path check goes from 1,637,664 visits to 2,412, and validate_registry from 2612 ms to 2.6 ms. On a real ~320-type application container it goes from 418,891 visits to 550, cutting 1.40 s off a 1.97 s container build. The cycle validation this changes was added in #83 for #80. --- test/unit/test_container_creation.py | 126 +++++++++++++++++++++++++++ wireup/ioc/registry_validation.py | 23 ++++- 2 files changed, 146 insertions(+), 3 deletions(-) diff --git a/test/unit/test_container_creation.py b/test/unit/test_container_creation.py index 94051bdb..392343e4 100644 --- a/test/unit/test_container_creation.py +++ b/test/unit/test_container_creation.py @@ -10,6 +10,8 @@ import wireup from wireup._annotations import Inject, abstract, injectable from wireup.errors import WireupError +from wireup.ioc import registry_validation +from wireup.ioc.types import get_container_object_id from test.unit.services.no_annotations.random.random_service import RandomService @@ -208,6 +210,130 @@ def make_foo_no_dependency() -> Foo: wireup.create_sync_container(injectables=[make_foo, make_bar, make_foo_no_dependency]) +def test_validates_container_walks_shared_dependencies_once(monkeypatch: pytest.MonkeyPatch) -> None: + @wireup.injectable + class Leaf: ... + + @wireup.injectable + @dataclass + class Shared: + leaf: Leaf + + @wireup.injectable + @dataclass + class Foo: + shared: Shared + + @wireup.injectable + @dataclass + class Bar: + shared: Shared + + @wireup.injectable + @dataclass + class Baz: + foo: Foo + bar: Bar + shared: Shared + + reached: list[type] = [] + descended: list[type] = [] + walk = registry_validation.assert_valid_resolution_path + + def recording_walk(**kwargs) -> None: + dependency = kwargs["dependency"] + reached.append(dependency.klass) + # Dependencies known to be cycle-free return immediately, only record the rest. + if get_container_object_id(dependency.klass, dependency.qualifier_value) not in kwargs["cleared"]: + descended.append(dependency.klass) + walk(**kwargs) + + monkeypatch.setattr(registry_validation, "assert_valid_resolution_path", recording_walk) + wireup.create_sync_container(injectables=[Leaf, Shared, Foo, Bar, Baz]) + + assert reached.count(Shared) > 1 + # Every dependency in the graph is walked exactly once no matter how many paths reach it. + assert descended.count(Shared) == 1 + assert descended.count(Leaf) == 1 + assert descended.count(Foo) == 1 + assert descended.count(Bar) == 1 + + +def test_validates_container_raises_when_cycle_is_behind_a_walked_dependency() -> None: + class Shared: ... + + class Foo: + def __init__(self, shared, bar): ... + + class Bar: + def __init__(self, shared, foo): ... + + @wireup.injectable + def make_shared() -> Shared: + return Shared() + + # Walking 'shared' clears it, the cycle behind 'bar' must still be found. + @wireup.injectable + def make_foo(shared: Shared, bar: Bar) -> Foo: + return Foo(shared, bar) + + @wireup.injectable + def make_bar(shared: Shared, foo: Foo) -> Bar: + return Bar(shared, foo) + + with pytest.raises( + WireupError, + match=re.escape( + f"Circular dependency detected for {Bar!r} (created via {make_bar.__module__}.{make_bar.__name__})" + f"\n -> {Foo!r} (created via {make_foo.__module__}.{make_foo.__name__})" + f"\n -> {Bar!r} (created via {make_bar.__module__}.{make_bar.__name__})" + " ! Cycle here" + ), + ): + wireup.create_sync_container(injectables=[make_shared, make_foo, make_bar]) + + +def test_validates_container_does_not_reuse_walked_dependencies_between_containers() -> None: + class Foo: + def __init__(self, bar): ... + + class Bar: + def __init__(self, foo): ... + + class Baz: + def __init__(self, foo): ... + + @wireup.injectable + def make_foo() -> Foo: + return Foo(None) + + @wireup.injectable + def make_baz(foo: Foo) -> Baz: + return Baz(foo) + + # Foo is walked and cleared here, which says nothing about the graph of another container. + wireup.create_sync_container(injectables=[make_foo, make_baz]) + + @wireup.injectable + def make_cyclical_foo(bar: Bar) -> Foo: + return Foo(bar) + + @wireup.injectable + def make_bar(foo: Foo) -> Bar: + return Bar(foo) + + with pytest.raises( + WireupError, + match=re.escape( + f"Circular dependency detected for {Bar!r} (created via {make_bar.__module__}.{make_bar.__name__})" + f"\n -> {Foo!r} (created via {make_cyclical_foo.__module__}.{make_cyclical_foo.__name__})" + f"\n -> {Bar!r} (created via {make_bar.__module__}.{make_bar.__name__})" + " ! Cycle here" + ), + ): + wireup.create_sync_container(injectables=[make_cyclical_foo, make_bar]) + + def test_lifetimes_match_factories() -> None: class ScopedService: ... diff --git a/wireup/ioc/registry_validation.py b/wireup/ioc/registry_validation.py index 27e0ce5a..198436d2 100644 --- a/wireup/ioc/registry_validation.py +++ b/wireup/ioc/registry_validation.py @@ -13,11 +13,16 @@ if TYPE_CHECKING: from wireup.ioc.registry import ContainerRegistry, InjectableFactory - from wireup.ioc.types import Qualifier + from wireup.ioc.types import ContainerObjectIdentifier, Qualifier def validate_registry(registry: ContainerRegistry) -> None: """Assert that all required dependencies exist for this registry instance.""" + # Dependencies are shared between injectables, so the same subtree is reachable via many paths. + # Remember the ones already known to be cycle-free to avoid walking them again. + # Only valid for this run as a different registry describes a different graph. + cleared: set[ContainerObjectIdentifier] = set() + for obj_id, injectable_factory in registry.factories.items(): if isinstance(obj_id, tuple): impl, impl_qualifier = obj_id @@ -54,6 +59,7 @@ def validate_registry(registry: ContainerRegistry) -> None: dependencies=registry.dependencies, dependency=dependency, path=[], + cleared=cleared, ) for name in unknown_dependencies_with_default: @@ -118,18 +124,26 @@ def assert_dependency_exists( raise WireupError(msg) -def assert_valid_resolution_path( +def assert_valid_resolution_path( # noqa: PLR0913 *, interfaces: dict[type, dict[Qualifier | None, type]], factories: dict[Any, InjectableFactory], dependencies: dict[Any, dict[str, AnnotatedParameter]], dependency: AnnotatedParameter, path: list[tuple[AnnotatedParameter, Any]], + cleared: set[ContainerObjectIdentifier], ) -> None: """Assert that the resolution path for a dependency does not create a cycle.""" if dependency.klass in interfaces or dependency.is_parameter: return - dependency_injectable_factory = factories[get_container_object_id(dependency.klass, dependency.qualifier_value)] + object_id = get_container_object_id(dependency.klass, dependency.qualifier_value) + + # A dependency whose subtree came back clean cannot lead to a cycle. Were it part of one, + # the walk would have come back to it while it was still on the current path. + if object_id in cleared: + return + + dependency_injectable_factory = factories[object_id] new_path: list[tuple[AnnotatedParameter, Any]] = [*path, (dependency, dependency_injectable_factory)] if any(p.klass == dependency.klass and p.qualifier_value == dependency.qualifier_value for p, _ in path): @@ -154,4 +168,7 @@ def stringify_dependency(p: AnnotatedParameter, factory: Any) -> str: dependencies=dependencies, dependency=next_dependency, path=new_path, + cleared=cleared, ) + + cleared.add(object_id) From d2e15e105de085e8acb401fd17868d2a01f5bcd1 Mon Sep 17 00:00:00 2001 From: Maxim Morozov Date: Sun, 16 Aug 2026 04:31:37 +0500 Subject: [PATCH 2/2] Rename cleared to known_cycle_free_objects Per review. The set holds the objects already established to be cycle-free rather than every object that happens to be one, and the longer name says so without a reader having to infer it from the fact that entries are added only after the subtree returns. --- test/unit/test_container_creation.py | 5 +++-- wireup/ioc/registry_validation.py | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/test/unit/test_container_creation.py b/test/unit/test_container_creation.py index 392343e4..9b585076 100644 --- a/test/unit/test_container_creation.py +++ b/test/unit/test_container_creation.py @@ -243,8 +243,9 @@ class Baz: def recording_walk(**kwargs) -> None: dependency = kwargs["dependency"] reached.append(dependency.klass) + object_id = get_container_object_id(dependency.klass, dependency.qualifier_value) # Dependencies known to be cycle-free return immediately, only record the rest. - if get_container_object_id(dependency.klass, dependency.qualifier_value) not in kwargs["cleared"]: + if object_id not in kwargs["known_cycle_free_objects"]: descended.append(dependency.klass) walk(**kwargs) @@ -311,7 +312,7 @@ def make_foo() -> Foo: def make_baz(foo: Foo) -> Baz: return Baz(foo) - # Foo is walked and cleared here, which says nothing about the graph of another container. + # Foo is walked and recorded cycle-free here, which says nothing about another container's graph. wireup.create_sync_container(injectables=[make_foo, make_baz]) @wireup.injectable diff --git a/wireup/ioc/registry_validation.py b/wireup/ioc/registry_validation.py index 198436d2..14544701 100644 --- a/wireup/ioc/registry_validation.py +++ b/wireup/ioc/registry_validation.py @@ -21,7 +21,7 @@ def validate_registry(registry: ContainerRegistry) -> None: # Dependencies are shared between injectables, so the same subtree is reachable via many paths. # Remember the ones already known to be cycle-free to avoid walking them again. # Only valid for this run as a different registry describes a different graph. - cleared: set[ContainerObjectIdentifier] = set() + known_cycle_free_objects: set[ContainerObjectIdentifier] = set() for obj_id, injectable_factory in registry.factories.items(): if isinstance(obj_id, tuple): @@ -59,7 +59,7 @@ def validate_registry(registry: ContainerRegistry) -> None: dependencies=registry.dependencies, dependency=dependency, path=[], - cleared=cleared, + known_cycle_free_objects=known_cycle_free_objects, ) for name in unknown_dependencies_with_default: @@ -131,7 +131,7 @@ def assert_valid_resolution_path( # noqa: PLR0913 dependencies: dict[Any, dict[str, AnnotatedParameter]], dependency: AnnotatedParameter, path: list[tuple[AnnotatedParameter, Any]], - cleared: set[ContainerObjectIdentifier], + known_cycle_free_objects: set[ContainerObjectIdentifier], ) -> None: """Assert that the resolution path for a dependency does not create a cycle.""" if dependency.klass in interfaces or dependency.is_parameter: @@ -140,7 +140,7 @@ def assert_valid_resolution_path( # noqa: PLR0913 # A dependency whose subtree came back clean cannot lead to a cycle. Were it part of one, # the walk would have come back to it while it was still on the current path. - if object_id in cleared: + if object_id in known_cycle_free_objects: return dependency_injectable_factory = factories[object_id] @@ -168,7 +168,7 @@ def stringify_dependency(p: AnnotatedParameter, factory: Any) -> str: dependencies=dependencies, dependency=next_dependency, path=new_path, - cleared=cleared, + known_cycle_free_objects=known_cycle_free_objects, ) - cleared.add(object_id) + known_cycle_free_objects.add(object_id)