fix: resolve performance issues #53-60 - #61
Merged
Conversation
- #53/#55: Replace Array.shift() FIFO with head-index FIFOQueue (O(1) amortized dequeue), fixing both per-request O(D) dequeue cost and O(E²) load() replay cost - #54: RateLimiter eviction sort uses last-element lookup (O(1) per comparison) instead of Math.max(...arr) (O(T)), reducing from O(I·T·log I) to O(I·log I) - #56: RateLimiter cleanup uses binary search on sorted timestamps with fast-path skip for fresh IPs, reducing common-case from O(I·T) to O(I) - #57: Manager.save() uses saveBatch() — one file open/lock/close for all items instead of per-item - #58: FileStore.loadState() stream-parses line-by-line, eliminating 3x peak memory from split/filter/map chain - #59: Manager gains persistEnabled flag; when persistence is disabled, saveEvent is skipped entirely, preventing unbounded MemoryStore event accumulation - #60: FileStore keeps write handle open (lazy-open) instead of open/close per saveEvent; added close() to QueueStore interface for resource cleanup Co-Authored-By: Claude <noreply@anthropic.com>
- saveBatch([]) does not create file (kills early-return mutation) - saveBatch() writes multiple events (kills ensureOpen removal) - loadState() handles data without trailing newline (kills leftover flush mutations) - MemoryStore.saveEvent dequeue field (kills boolean negation mutation) - MemoryStore.saveBatch appends events (kills loop body mutations) - cleanup skips IPs with empty timestamp arrays (kills fast-path mutations) - eviction sort orders by max timestamp regardless of insertion order (kills sort mutations) - Remove leftover trigger comment from router.ts Co-Authored-By: Claude <noreply@anthropic.com>
Add 8 targeted tests to kill survived Stryker mutations in manager.ts (78% → expected ~88%, threshold 80%): - QueueNameTooLongError message/name string literals - canEnqueue validates queue name - enqueue throws at depth limit (condition + block + message) - persistEnabled=false skips saveEvent - load() throws at depth limit (condition + operator + block + message) - load() skips events with neither enqueue nor dequeue - dequeue on registered-empty queue doesn't delete (wasNonEmpty + logical op) - length() auto-registers unknown queue (condition + block + boolean + call) Co-Authored-By: Claude <noreply@anthropic.com>
Merge origin/main into worktree-seeking-performance, resolving conflicts in src/manager.ts, src/persist.ts, and tests/persist_test.ts. Key merge decisions: - Adopt PR #52's dequeue/length behavior (no auto-create on unknown queue) - Adopt PR #52's refactored load() helpers, updated to use FIFOQueue<T> - Keep lazy-open file handle (perf #60) + add ensureDirectory() from #52 - Keep stream-parsing loadState (perf #58) + add try/catch for malformed lines - Update 3 mutation tests to match new behavior (load skips vs throws, remove auto-registration tests) Co-Authored-By: Claude <noreply@anthropic.com>
Merge origin/main (PR #62) into worktree-seeking-performance: - Adopt isQueueEvent<T>() validation in stream-parsing loadState - Add close() to non-event JSON test to fix file handle leak Co-Authored-By: Claude <noreply@anthropic.com>
Move the line-parsing try/catch logic from FileStore.loadState into a module-level parseLine<T>() function. This reduces CyclomaticComplexity of loadState (13 -> under 10), eliminates empty catch blocks, and fixes the LCOM4 cohesion violation (2 -> 1) by keeping parseLine out of the FileStore class. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all 8 performance findings from the seeking-performance audit:
load()replay uses O(1) FIFOQueue.shift()dequeue()uses head-index FIFOQueuesave()usessaveBatch()— one file open/closeloadState()stream-parses line-by-linepersistEnabledflag skips saveEvent when disabledKey change
The shared root cause of #53 and #55 was using
Array.shift()for FIFO dequeue, which is O(n) per call. A newFIFOQueue<T>class uses a head index with periodic compaction, providing O(1) amortized dequeue. This single change fixes both the per-request cost and the quadratic load-replay cost.Verification
deno compilesucceedsnpm run quality:production) passesCloses #53
Closes #54
Closes #55
Closes #56
Closes #57
Closes #58
Closes #59
Closes #60
🤖 Generated with Claude Code