BiggiePockets approved a maintenance task PR that has a quadratic hot path, which only surfaced later when the task ran slowly in production.
Example: app/tasks/maintenance/dedupe_forum_locations_task.rb in BiggerPockets/biggerpockets (DedupeForumLocationsTask#group_members) re-scans the full in-memory location list and issues a fresh SimplifiedForums::Location.exists? query for every candidate, on every call to #process. That's O(n^2) comparisons plus one DB round-trip per candidate per row processed — fine on a small table, but it silently degrades badly as the table grows.
Ask: have the review workflow specifically check diffs for:
- N+1 queries introduced inside loops (including
exists?/find/where calls per iteration)
- Nested iteration over the same collection (O(n^2) or worse) in batch/task code
- Maintenance Task
#process/#collection implementations in particular, since these run over full tables and errors compound at scale
This doesn't need to block every PR touching a loop, but it should be a class of finding the reviewer explicitly looks for and flags with a suggested fix, similar to how it already checks against JIRA acceptance criteria.
BiggiePockets approved a maintenance task PR that has a quadratic hot path, which only surfaced later when the task ran slowly in production.
Example:
app/tasks/maintenance/dedupe_forum_locations_task.rbinBiggerPockets/biggerpockets(DedupeForumLocationsTask#group_members) re-scans the full in-memory location list and issues a freshSimplifiedForums::Location.exists?query for every candidate, on every call to#process. That's O(n^2) comparisons plus one DB round-trip per candidate per row processed — fine on a small table, but it silently degrades badly as the table grows.Ask: have the review workflow specifically check diffs for:
exists?/find/wherecalls per iteration)#process/#collectionimplementations in particular, since these run over full tables and errors compound at scaleThis doesn't need to block every PR touching a loop, but it should be a class of finding the reviewer explicitly looks for and flags with a suggested fix, similar to how it already checks against JIRA acceptance criteria.