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-04-05 - Avoid eager default allocation in dictionary lookups

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

This entry is being added now, but the header date is 2024-04-05. If this is meant to be a chronological log of learnings, consider updating the date to match the actual change/PR date to avoid confusing ordering when more entries are added later.

Suggested change
## 2024-04-05 - Avoid eager default allocation in dictionary lookups
## 2026-04-05 - Avoid eager default allocation in dictionary lookups

Copilot uses AI. Check for mistakes.
**Learning:** In hot loops, using `dict.setdefault(key, complex_default)` unconditionally allocates the default value (e.g. lists, fraction objects) on every iteration, even when the key already exists. This creates massive memory overhead.
**Action:** Replace `dict.setdefault` with explicit membership checks (`if key not in dict: dict[key] = complex_default`) inside high-frequency loops.
28 changes: 20 additions & 8 deletions src/geometry/hull_kv.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,19 @@ def is_breakpoint(self, slope: Fraction) -> bool:
return index < len(self.breakpoints) and self.breakpoints[index] == slope


def _add_vectors(left: tuple[Fraction, ...], right: tuple[Fraction, ...]) -> tuple[Fraction, ...]:
def _add_vectors(
left: tuple[Fraction, ...], right: tuple[Fraction, ...]
) -> tuple[Fraction, ...]:
return tuple(a + b for a, b in zip(left, right, strict=True))


def _intersection_x(left: _EnvelopeLine, right: _EnvelopeLine) -> Fraction:
return Fraction(left.intercept - right.intercept, right.slope - left.slope)


def _build_upper_envelope(points: Sequence[_PointAggregate], negate: bool = False) -> _Envelope:
def _build_upper_envelope(
points: Sequence[_PointAggregate], negate: bool = False
) -> _Envelope:
sign = Fraction(-1 if negate else 1)
best_by_slope: dict[Fraction, _EnvelopeLine] = {}

Expand All @@ -83,7 +87,9 @@ def _build_upper_envelope(points: Sequence[_PointAggregate], negate: bool = Fals
if existing is None or candidate.intercept > existing.intercept:
best_by_slope[candidate.slope] = candidate

ordered = sorted(best_by_slope.values(), key=lambda line: (line.slope, line.intercept))
ordered = sorted(
best_by_slope.values(), key=lambda line: (line.slope, line.intercept)
)
if not ordered:
return _Envelope(lines=(), breakpoints=())

Expand Down Expand Up @@ -172,7 +178,9 @@ def query(self, query: Sequence[NumberLike]) -> HardmaxResult:
if qx == 0 and qy == 0:
divisor = Fraction(self._total_count)
averaged = tuple(coord / divisor for coord in self._total_value_sum)
all_indices = tuple(index for point in self._points for index in point.entry_indices)
all_indices = tuple(
index for point in self._points for index in point.entry_indices
)
return HardmaxResult(
score=0,
value=_restore_value(averaged, self._scalar_mode),
Expand Down Expand Up @@ -205,10 +213,14 @@ 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": []},
)
# Optimization: avoid eager default allocation in hot loop
if key not in aggregates:
aggregates[key] = {
"value_sum": [Fraction(0) for _ in value],
"count": 0,
"entry_indices": [],
}
bucket = aggregates[key]
Comment on lines +216 to +223

Copilot AI Apr 5, 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 eager default allocation, but if key not in aggregates: followed by bucket = aggregates[key] performs two dictionary lookups (and hashes) per iteration. Since this is a hot loop, consider using a single-lookup pattern (e.g., try/except KeyError or bucket = aggregates.get(key) + initialize) to keep the optimization focused on minimizing per-iteration overhead.

Suggested change
# Optimization: avoid eager default allocation in hot loop
if key not in aggregates:
aggregates[key] = {
"value_sum": [Fraction(0) for _ in value],
"count": 0,
"entry_indices": [],
}
bucket = aggregates[key]
# Optimization: avoid eager default allocation and redundant lookups in hot loop
bucket = aggregates.get(key)
if bucket is None:
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