Skip to content

Commit 9244884

Browse files
committed
perf: move BasicBlock type check from __iter__ to insertion methods
The isinstance check on every yielded item in BasicBlock.__iter__ was purely defensive — it detected invalid types that had already entered the list. Moving it to append/extend/insert/__setitem__ catches invalid types at the point of insertion instead, eliminating the check from the hot iteration path entirely. The structural checks (jump must be last, jump/TryBegin target must be a BasicBlock) remain in __iter__ as they depend on the full block context and cannot be verified at insertion time. Profiling data | Hotspot | Before | After | |---|---|---| | `BasicBlock.__iter__` own | 4.91% | 2.82% | | `ControlFlowGraph.from_bytecode` own | 4.61% | 4.04% | Throughput (Bytecode.from_code().to_code() on dis module's code object, 1 second timed window, 5 runs): | | r/s range | |---|---| | Before | 133–144 | | After | 142–144 |
1 parent 25bf1bc commit 9244884

2 files changed

Lines changed: 43 additions & 10 deletions

File tree

src/bytecode/cfg.py

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,50 @@ def __init__(
4242
if instructions:
4343
super().__init__(instructions)
4444

45+
_VALID_TYPES = (SetLineno, Instr, TryBegin, TryEnd)
46+
47+
@staticmethod
48+
def _check_instr(instr: Any) -> None:
49+
if not isinstance(instr, (SetLineno, Instr, TryBegin, TryEnd)):
50+
raise ValueError(
51+
"BasicBlock must only contain SetLineno and Instr objects, "
52+
"but %s was found" % instr.__class__.__name__
53+
)
54+
55+
def append(self, instr: Union[Instr, SetLineno, TryBegin, TryEnd]) -> None:
56+
self._check_instr(instr)
57+
super().append(instr)
58+
59+
def insert(
60+
self, index: SupportsIndex, instr: Union[Instr, SetLineno, TryBegin, TryEnd]
61+
) -> None:
62+
self._check_instr(instr)
63+
super().insert(index, instr)
64+
65+
def extend(
66+
self, instrs: Iterable[Union[Instr, SetLineno, TryBegin, TryEnd]]
67+
) -> None:
68+
instrs = list(instrs)
69+
for instr in instrs:
70+
self._check_instr(instr)
71+
super().extend(instrs)
72+
73+
def __setitem__(self, index, value):
74+
if isinstance(index, slice):
75+
values = list(value)
76+
for instr in values:
77+
self._check_instr(instr)
78+
super().__setitem__(index, values)
79+
else:
80+
self._check_instr(value)
81+
super().__setitem__(index, value)
82+
4583
def __iter__(self) -> Iterator[Union[Instr, SetLineno, TryBegin, TryEnd]]:
4684
index = 0
4785
while index < len(self):
4886
instr = self[index]
4987
index += 1
5088

51-
if not isinstance(instr, (SetLineno, Instr, TryBegin, TryEnd)):
52-
raise ValueError(
53-
"BasicBlock must only contain SetLineno and Instr objects, "
54-
"but %s was found" % instr.__class__.__name__
55-
)
56-
5789
if isinstance(instr, Instr) and instr.has_jump():
5890
if index < len(self) and any(
5991
isinstance(self[i], Instr) for i in range(index, len(self))

tests/test_cfg.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,14 @@ def disassemble(
5555

5656
class BlockTests(unittest.TestCase):
5757
def test_iter_invalid_types(self):
58-
# Labels are not allowed in basic blocks
58+
# Labels are not allowed in basic blocks — caught at insertion time
5959
block = BasicBlock()
60-
block.append(Label())
6160
with self.assertRaises(ValueError):
62-
list(block)
61+
block.append(Label())
6362
with self.assertRaises(ValueError):
64-
block.legalize(1)
63+
block.extend([Label()])
64+
with self.assertRaises(ValueError):
65+
block.insert(0, Label())
6566

6667
# Only one jump allowed and only at the end
6768
block = BasicBlock()

0 commit comments

Comments
 (0)