perf(memory-storage): avoid O(n) deque scan on forefront re-add#2074
Open
anxkhn wants to merge 1 commit into
Open
perf(memory-storage): avoid O(n) deque scan on forefront re-add#2074anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
`MemoryRequestQueueClient.add_batch_of_requests` repositioned an already-pending request to the forefront with `deque.remove`, a linear scan of the whole pending deque. Re-adding a batch of K already-pending requests to a deque of N was O(K*N). Forefront re-adds are a real path: tiered-proxy retries set `request.forefront = True` and re-enqueue while the request is still pending. The forefront reposition now just `appendleft`s the request and leaves the old entry in place. The new object is registered in `_requests_by_unique_key`, superseding the old one, and `fetch_next_request` and `is_empty` skip entries whose object is no longer the one registered for their unique key. Because a forefront reposition always enqueues the live copy ahead of the entry it supersedes, `is_empty` can safely prune stale entries from the front. Only forefront re-adds create such tombstones. A regular re-add of an already-pending request leaves both the deque entry and its registered object untouched, so the identity check never misfires and no request is dropped. Forefront-LIFO / regular-FIFO order and deduplication are unchanged. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.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
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.
MemoryRequestQueueClient.add_batch_of_requestsrepositioned an already-pendingrequest to the forefront by calling
self._pending_requests.remove(existing_request)on the pending
deque.deque.removeis O(n): it linearly scans the deque to findthe entry. Re-adding a batch of K already-pending requests to a pending deque of N
entries is therefore O(K*N).
Forefront re-adds of still-pending requests are a normal runtime path, not an edge
case:
RequestManagerTandem.fetch_next_requestre-enqueues every request it takes fromthe read-only loader with
forefront=True(
src/crawlee/request_loaders/_request_manager_tandem.py).request.forefront = Truebefore re-adding(
src/crawlee/proxy_configuration.py).This change removes the linear scan:
appendlefts the request and leaves the oldentry in the deque. The new object is registered in
_requests_by_unique_key,superseding the old one.
fetch_next_requestandis_emptyskip any popped entry whose object is no longerthe one currently registered for its
unique_key(a stale tombstone). Because aforefront reposition always enqueues the live copy ahead of the entry it supersedes,
is_emptycan safely prune stale entries from the front until it reaches a live one.no-op: neither the deque entry nor its registered object is touched, so the identity
check never misfires and no request is dropped. This matches the sibling file-system
client, which likewise does not re-store an already-pending request on a
non-forefront re-add.
Forefront-LIFO / regular-FIFO ordering, deduplication, and the metadata counts
(
total/pending/handled) are unchanged.This is the memory-client counterpart to #2011, which fixes the same class of
quadratic reposition behavior in the file-system request queue client. The memory
client was not covered there.
Issues
Testing
Added regression tests in
tests/unit/storage_clients/_memory/test_memory_rq_client.py:test_forefront_readd_repositions_without_deque_scan- wraps the pending deque in adequesubclass that countsremovecalls; re-adding 20 still-pending requests withforefront=Truenow performs 0removecalls (was 20 before the change). Guards theperformance fix.
test_forefront_readd_preserves_order_and_dedup- forefront re-add of a subset keepsLIFO order, no duplicates linger, and the queue drains to empty/finished.
test_regular_readd_of_pending_request_is_not_dropped- a non-forefront re-add of astill-pending URL as a distinct
Requestobject stays fetchable exactly once, withconsistent counts (guards against the lazy-tombstone skip dropping a live request).
test_regular_readd_does_not_reorder_pending_queue- a non-forefront re-add leavesFIFO order untouched.
Commands run locally (offline):
Checklist