Skip to content

Commit 7a21543

Browse files
committed
Fix condition CNF explosion by using flat symbols for condition names
When conditions reference other conditions via !Condition, the CNF expression was recursively expanded into the full boolean tree. For templates with many conditions (e.g., 5 groups of 10 Fn::Equals each plus a DeployLambdas = Not(Or(G1..G5))), this created massive expressions where sympy's CNF conversion (_or()) exploded exponentially. Fix: Give each condition name its own Symbol and express relationships as implication constraints added once to the EncodedCNF. Nested !Condition references now resolve to the simple Symbol instead of expanding the full expression. The SAT solver handles implications through the constraints. Both the template-level (conditions/conditions.py) and context-level (context/conditions/_conditions.py) classes are updated. Performance on reproduction template: 20+ seconds -> 0.33 seconds. Fixes #4406
1 parent d67d05d commit 7a21543

5 files changed

Lines changed: 78 additions & 108 deletions

File tree

src/cfnlint/conditions/_condition.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,14 @@ def build_true_cnf(self, params: dict[str, Symbol]) -> Any:
233233
Returns:
234234
Any: A SymPy CNF clause
235235
"""
236-
return self.build_cnf(params)
236+
if self._name in params:
237+
return params[self._name]
238+
return super().build_cnf(params)
239+
240+
def build_cnf(self, params: dict[str, Symbol]) -> Any:
241+
if self._name in params:
242+
return params[self._name]
243+
return super().build_cnf(params)
237244

238245
def build_false_cnf(self, params: dict[str, Symbol]) -> Any:
239246
"""Build a SymPy CNF for a False based scenario

src/cfnlint/conditions/conditions.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,31 @@ def _build_equal_vars(c_equals: list[Equal]):
187187
if prop is not None:
188188
cnf.add_prop(Not(prop))
189189

190+
# Create a Symbol per condition name and add implication constraints
191+
# so that build_true_cnf/build_false_cnf can use the simple Symbol
192+
# instead of the full boolean expression (avoids CNF explosion).
193+
# We add condition symbols to equal_vars FIRST so that when
194+
# build_cnf is called, nested !Condition references resolve to
195+
# the simple Symbol instead of expanding the full expression.
196+
cond_symbols: dict[str, Symbol] = {}
197+
for condition_name in condition_names:
198+
cond_symbols[condition_name] = Symbol(f"__cond_{condition_name}")
199+
200+
# Build equivalences using cond_symbols for nested condition refs
201+
# but NOT for the condition being defined (to avoid circularity)
202+
for condition_name in condition_names:
203+
cond_sym = cond_symbols[condition_name]
204+
# Merge equal_vars with other condition symbols (excluding self)
205+
build_params = dict(equal_vars)
206+
for other_name, other_sym in cond_symbols.items():
207+
if other_name != condition_name:
208+
build_params[other_name] = other_sym
209+
cond_expr = self._conditions[condition_name].build_cnf(build_params)
210+
cnf.add_prop(Implies(cond_sym, cond_expr))
211+
cnf.add_prop(Implies(cond_expr, cond_sym))
212+
213+
equal_vars.update(cond_symbols)
214+
190215
for rule in self._rules:
191216
cnf.add_prop(rule.build_cnf(equal_vars))
192217

src/cfnlint/context/conditions/_condition.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from dataclasses import dataclass, field
99
from typing import Any
1010

11-
from sympy import And, Not, Or
11+
from sympy import And, Not, Or, Symbol
1212
from sympy.logic.boolalg import BooleanFunction
1313

1414
from cfnlint.conditions._utils import get_hash
@@ -20,19 +20,16 @@
2020
class Condition:
2121
instance: Any = field(init=True)
2222
status: bool | None = field(init=True, default=None)
23-
hash: str = field(init=True, default="") # Changed to init=True with default
23+
hash: str = field(init=True, default="")
2424

2525
fn_equals: Equal | None = field(init=True, default=None)
2626
condition: list["Condition"] | "Condition" | None = field(init=True, default=None)
2727
cnf: BooleanFunction = field(init=True, default_factory=BooleanFunction)
2828

29-
# Removed __post_init__ since hash is now passed in
30-
3129
@classmethod
3230
def create_from_instance(
3331
cls, instance: Any, all_conditions: dict[str, Any]
3432
) -> "Condition":
35-
# Calculate hash here once
3633
instance_hash = get_hash(instance)
3734

3835
fn_k, fn_v = is_function(instance)
@@ -73,6 +70,7 @@ def create_from_instance(
7370
if fn_k == "Condition":
7471
if not isinstance(fn_v, str):
7572
raise ValueError(f"Condition value {fn_v!r} must be a string")
73+
# Build the sub-condition for is_region/equals traversal
7674
sub_condition = all_conditions.get(fn_v)
7775
try:
7876
sub_all_conditions = all_conditions.copy()
@@ -82,7 +80,14 @@ def create_from_instance(
8280
c = Condition.create_from_instance(
8381
{"Fn::Equals": [None, None]}, all_conditions
8482
)
85-
return cls(instance=instance, condition=c, cnf=c.cnf, hash=instance_hash)
83+
# Use a Symbol for the condition name instead of the expanded CNF
84+
# The equivalence constraint is added in Conditions.create_from_instance
85+
return cls(
86+
instance=instance,
87+
condition=c,
88+
cnf=Symbol(fn_v),
89+
hash=instance_hash,
90+
)
8691

8792
raise ValueError(f"Unknown key {fn_k!r} in condition")
8893

src/cfnlint/context/conditions/_conditions.py

Lines changed: 33 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@
66
from __future__ import annotations
77

88
import itertools
9-
from collections import OrderedDict
109
from dataclasses import dataclass, field
1110
from typing import TYPE_CHECKING, Any, Iterator
1211

13-
from sympy import Not, Or
14-
from sympy.logic.boolalg import BooleanFunction
12+
from sympy import Equivalent, Not, Or, Symbol
13+
from sympy.assumptions.cnf import EncodedCNF
1514
from sympy.logic.inference import satisfiable
1615

1716
from cfnlint.conditions._utils import get_hash
@@ -26,39 +25,21 @@
2625
from cfnlint.context.context import Context, Parameter
2726

2827

29-
# Use OrderedDict for LRU-like behavior
30-
_satisfiable_cache: OrderedDict[str, bool] = OrderedDict()
31-
_MAX_CACHE_SIZE = 10000 # Limit cache size to prevent memory issues
32-
33-
34-
def _get_from_satisfiable_cache(cnf_hash: str) -> bool | None:
35-
"""Get result from cache with LRU behavior"""
36-
if cnf_hash in _satisfiable_cache:
37-
# Move to end (most recently used)
38-
value = _satisfiable_cache.pop(cnf_hash)
39-
_satisfiable_cache[cnf_hash] = value
40-
return value
41-
return None
42-
43-
44-
def _add_to_satisfiable_cache(cnf_hash: str, result: bool) -> None:
45-
"""Add result to cache with size management"""
46-
if len(_satisfiable_cache) >= _MAX_CACHE_SIZE:
47-
# Remove oldest item (first in OrderedDict)
48-
_satisfiable_cache.popitem(last=False)
49-
_satisfiable_cache[cnf_hash] = result
50-
51-
5228
@dataclass(frozen=True)
5329
class Conditions:
54-
# Template level condition management
5530
conditions: dict[str, Condition] = field(init=True, default_factory=dict)
56-
cnf: BooleanFunction | None = field(init=True, default=None)
31+
cnf: EncodedCNF = field(init=True, default_factory=EncodedCNF, compare=False)
32+
_condition_symbols: dict[str, Symbol] = field(
33+
init=True, default_factory=dict, compare=False
34+
)
5735
_max_scenarios: int = field(init=False, default=128)
5836

5937
@classmethod
6038
def create_from_instance(
61-
cls, conditions: Any, rules: dict[str, dict], parameters: dict[str, "Parameter"]
39+
cls,
40+
conditions: Any,
41+
rules: dict[str, dict],
42+
parameters: dict[str, "Parameter"],
6243
) -> "Conditions":
6344
obj: dict[str, Condition] = {}
6445
if not isinstance(conditions, dict):
@@ -69,13 +50,20 @@ def create_from_instance(
6950
del other_conditions[k]
7051
obj[k] = Condition.create_from_instance(v, other_conditions)
7152
except ValueError:
72-
# this is a default condition so we can keep the name but it will
73-
# not associate with another condition and will always be true/false
7453
obj[k] = Condition.create_from_instance(
7554
{"Fn::Equals": [None, None]}, conditions
7655
)
7756

78-
cnf = None
57+
# Build EncodedCNF with a Symbol per condition name
58+
cnf = EncodedCNF()
59+
condition_symbols: dict[str, Symbol] = {}
60+
for name, cond in obj.items():
61+
sym = Symbol(name)
62+
condition_symbols[name] = sym
63+
# Add equivalence: Symbol(name) <-> condition's boolean expression
64+
cnf.add_prop(Equivalent(sym, cond.cnf))
65+
66+
# Add parameter AllowedValues constraints
7967
for p_k, p_v in parameters.items():
8068
if not p_v.allowed_values:
8169
continue
@@ -90,21 +78,17 @@ def create_from_instance(
9078
if i.left.instance in allowed_values:
9179
allowed_values.remove(i.left.instance)
9280

93-
if not allowed_values:
94-
if cnf is None:
95-
cnf = Or(*equals_cnfs)
96-
else:
97-
cnf = cnf & Or(*equals_cnfs)
81+
if not allowed_values and equals_cnfs:
82+
cnf.add_prop(Or(*equals_cnfs))
9883

99-
return cls(conditions=obj, cnf=cnf)
84+
return cls(conditions=obj, cnf=cnf, _condition_symbols=condition_symbols)
10085

10186
def evolve(self, status: dict[str, bool]) -> "Conditions":
10287
cls = self.__class__
10388

10489
if not status:
10590
return self
10691

107-
# Check if we're trying to set the same status
10892
all_same = True
10993
for condition, condition_status in status.items():
11094
if (
@@ -117,50 +101,30 @@ def evolve(self, status: dict[str, bool]) -> "Conditions":
117101
return self
118102

119103
conditions: dict[str, Condition] = {}
120-
cnf = self.cnf
104+
cnf = self.cnf.copy()
121105
for condition, value in self.conditions.items():
122106
s = status.get(condition, value.status)
123107
try:
124108
conditions[condition] = value.evolve(status=s)
125-
if s is not None:
126-
if cnf:
127-
cnf = (
128-
cnf & conditions[condition].cnf
129-
if s
130-
else cnf & Not(conditions[condition].cnf)
131-
)
132-
else:
133-
cnf = (
134-
conditions[condition].cnf
135-
if s
136-
else Not(conditions[condition].cnf)
137-
)
109+
if s is not None and condition in self._condition_symbols:
110+
sym = self._condition_symbols[condition]
111+
cnf.add_prop(sym if s else Not(sym))
138112
except ValueError as e:
139113
raise Unsatisfiable(
140114
new_status=status,
141115
current_status=self.status,
142116
) from e
143117

144-
cnf_hash = get_hash(str(cnf))
145-
cached_result = _get_from_satisfiable_cache(cnf_hash)
146-
if cached_result is not None:
147-
if not cached_result:
148-
raise Unsatisfiable(
149-
new_status=status,
150-
current_status=self.status,
151-
)
152-
else:
153-
is_sat = satisfiable(cnf)
154-
_add_to_satisfiable_cache(cnf_hash, bool(is_sat))
155-
if not is_sat:
156-
raise Unsatisfiable(
157-
new_status=status,
158-
current_status=self.status,
159-
)
118+
if not satisfiable(cnf):
119+
raise Unsatisfiable(
120+
new_status=status,
121+
current_status=self.status,
122+
)
160123

161124
return cls(
162125
conditions=conditions,
163126
cnf=cnf,
127+
_condition_symbols=self._condition_symbols,
164128
)
165129

166130
def _build_conditions(self, conditions: set[str]) -> Iterator["Conditions"]:

test/unit/module/context/conditions/test_conditions.py

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,7 @@
66
import pytest
77

88
from cfnlint.context import create_context_for_template
9-
from cfnlint.context.conditions._conditions import (
10-
_MAX_CACHE_SIZE,
11-
Conditions,
12-
_add_to_satisfiable_cache,
13-
_satisfiable_cache,
14-
)
9+
from cfnlint.context.conditions._conditions import Conditions
1510
from cfnlint.context.conditions.exceptions import Unsatisfiable
1611
from cfnlint.template import Template
1712

@@ -302,29 +297,3 @@ def test_evolve_from_instance(current_status, instance, expected):
302297
def test_condition_failures():
303298
with pytest.raises(ValueError):
304299
Conditions.create_from_instance([], {}, {})
305-
306-
307-
def test_add_to_satisfiable_cache():
308-
"""Test _add_to_satisfiable_cache function with cache size management"""
309-
# Clear the cache before testing
310-
_satisfiable_cache.clear()
311-
312-
# Add items to fill the cache
313-
for i in range(_MAX_CACHE_SIZE):
314-
_add_to_satisfiable_cache(f"test_key_{i}", True)
315-
316-
# Verify cache size
317-
assert len(_satisfiable_cache) == _MAX_CACHE_SIZE
318-
319-
# Add one more item to trigger cache management (line 48)
320-
_add_to_satisfiable_cache("overflow_key", False)
321-
322-
# Verify cache size remains at max
323-
assert len(_satisfiable_cache) == _MAX_CACHE_SIZE
324-
325-
# Verify the first item was removed (oldest item)
326-
assert "test_key_0" not in _satisfiable_cache
327-
328-
# Verify the new item was added
329-
assert "overflow_key" in _satisfiable_cache
330-
assert _satisfiable_cache["overflow_key"] is False

0 commit comments

Comments
 (0)