-
Notifications
You must be signed in to change notification settings - Fork 0
β‘ Bolt: replace dict.setdefault with explicit membership checks in hot loops #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ## 2024-05-24 - [Avoid `dict.setdefault` in hot loops] | ||
| **Learning:** `dict.setdefault(key, default)` evaluates the `default` expression eagerly on *every* loop iteration before performing the membership check. In cases where the default value requires complex allocation or computation (e.g., `{"value_sum": [Fraction(0) for _ in value]}`), this results in significant performance degradation in hot loops. | ||
| **Action:** Replace `dict.setdefault` in hot loops with explicit `if key not in dict:` membership checks and lazy assignment. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ | |
|
|
||
| import json | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| from bytecode import ( | ||
| lower_program, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -697,14 +697,15 @@ def evaluate_free_running_programs( | |
|
|
||
| outcomes.append(outcome) | ||
| bucket = bucket_name(outcome.program_steps) | ||
| bucket_state = per_bucket.setdefault( | ||
| bucket, | ||
| { | ||
| # Performance optimization: Replace `setdefault` with an explicit check | ||
| # to avoid allocating new dictionary objects on every iteration. | ||
| if bucket not in per_bucket: | ||
| per_bucket[bucket] = { | ||
| "program_count": 0, | ||
| "exact_trace_count": 0, | ||
| "exact_final_state_count": 0, | ||
| }, | ||
| ) | ||
| } | ||
| bucket_state = per_bucket[bucket] | ||
|
Comment on lines
+700
to
+708
|
||
| bucket_state["program_count"] += 1 | ||
| bucket_state["exact_trace_count"] += int(outcome.exact_trace_match) | ||
| bucket_state["exact_final_state_count"] += int(outcome.exact_final_state_match) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This membership-check pattern does two dict lookups in the common case (
key not in aggregatesthenaggregates[key]). Since this is in a hot loop, consider switching to a single-lookup lazy init (e.g.,bucket = aggregates.get(key)with a sentinel/None check, ortry: bucket = aggregates[key]/except KeyError:) to keep the optimization from regressing lookup overhead.