Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-05-14 - Eager Evaluation in dict.setdefault

**Learning:** When `dict.setdefault(key, complex_default)` is used inside a loop, Python eagerly evaluates the `complex_default` argument on *every* iteration before calling the method, even if the key already exists. In `src/geometry/hull_kv.py`, this caused a list of `Fraction(0)` objects to be instantiated repeatedly, leading to significant overhead in the `_rebuild_if_needed` loop.
**Action:** Avoid `setdefault` for complex default values (lists, objects, function calls) in hot paths. Instead, use an explicit membership check (`if key not in dict: dict[key] = complex_default()`) to ensure lazy evaluation and avoid unnecessary allocations.
9 changes: 4 additions & 5 deletions src/geometry/hull_kv.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
HardmaxResult,
NumberLike,
ValueLike,
_as_fraction,
_coerce_key,
_coerce_value,
_normalize_number,
Expand Down Expand Up @@ -205,10 +204,10 @@ def _rebuild_if_needed(self) -> None:
total_value_sum = [Fraction(0) for _ in range(self._value_width or 0)]

for index, (key, value) in enumerate(self._entries):
bucket = aggregates.setdefault(
key,
{"value_sum": [Fraction(0) for _ in value], "count": 0, "entry_indices": []},
)
# Performance: avoid eager evaluation of complex default (Fraction allocations) on every iteration
if key not in aggregates:
aggregates[key] = {"value_sum": [Fraction(0) for _ in value], "count": 0, "entry_indices": []}
bucket = aggregates[key]
Comment on lines +208 to +210

Copilot AI Apr 16, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change avoids the eager default allocation, but if key not in aggregates: followed by aggregates[key] does two dict lookups in the common case where the key already exists. In this hot loop, consider using a single-lookup pattern (e.g., try/except KeyError or bucket = aggregates.get(key) with a sentinel) to keep the allocation win without adding an extra hash table probe per iteration.

Suggested change
if key not in aggregates:
aggregates[key] = {"value_sum": [Fraction(0) for _ in value], "count": 0, "entry_indices": []}
bucket = aggregates[key]
# while also avoiding a second dict probe in the common case where the bucket exists.
try:
bucket = aggregates[key]
except KeyError:
bucket = {"value_sum": [Fraction(0) for _ in value], "count": 0, "entry_indices": []}
aggregates[key] = bucket

Copilot uses AI. Check for mistakes.
for coord_index, coord in enumerate(value):
bucket["value_sum"][coord_index] += coord
total_value_sum[coord_index] += coord
Expand Down