fix(deep_crawling): BFS O(n²) parent lookup and BestFirst duplicate enqueue - #2243
Open
chelsealong wants to merge 1 commit into
Open
fix(deep_crawling): BFS O(n²) parent lookup and BestFirst duplicate enqueue#2243chelsealong wants to merge 1 commit into
chelsealong wants to merge 1 commit into
Conversation
… BestFirst BFSDeepCrawlStrategy re-scanned the entire current_level list once per fetched result to find that result's parent URL, making per-level bookkeeping O(n^2) instead of O(n). Build a url->parent dict once per level instead. BestFirstCrawlingStrategy.link_discovery only checked `visited` without updating it, so a URL discovered by two sibling pages processed in the same batch was scored and pushed onto the priority queue twice before either copy was dequeued. Track newly discovered URLs in a separate `_enqueued` set (kept distinct from `visited`, which the dequeue loop relies on to mean "already processed") so a repeat discovery within the same crawl is skipped. Fixes unclecode#2242
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.
Summary
Fixes #2242, two related performance bugs in
deep_crawling:BFSDeepCrawlStrategy— O(n²) parent lookup._arun_batchand_arun_streamlooked up each result's parent URL withnext((parent for (u, parent) in current_level if u == url), None),a linear scan of the whole current level, executed once per result in
that level. For a level of N URLs this is O(N²) pure-Python work. Fixed
by building a
url -> parentdict once per level (dict(current_level))and doing an O(1)
.get(url)per result.BestFirstCrawlingStrategy— duplicate enqueue.link_discoveryonly read
visited(if base_url in visited: continue) but neverupdated it, so when two sibling pages processed in the same batch both
link to the same third URL, that URL was scored and pushed onto the
priority queue twice before either copy was ever dequeued (the dupe was
later silently dropped at dequeue time via the same
visitedcheck, sofinal output was correct — the waste was in scoring/enqueueing).
The obvious fix — marking
visitedas soon as a URL is discovered,inside
link_discovery— turned out to be wrong:visitedis alsoread by the dequeue loop in
_arun_best_firstto mean "alreadyprocessed/dequeued", and marking a URL visited at discovery time
made the dequeue loop skip it before it was ever crawled. Instead this
adds a separate
self._enqueuedset, tracked distinctly fromvisitedand checked/updated only inlink_discovery, mirroring thepattern
DFSDeepCrawlStrategyalready uses for its own_dfs_seensetfor exactly this reason (see the docstring on
DFSDeepCrawlStrategy).Test plan
Added
tests/deep_crawling/test_deep_crawl_efficiency.pywith tworegression tests, both built on the mocked-crawler pattern already used in
tests/deep_crawling/test_deep_crawl_cancellation.py:test_parent_lookup_scales_linearly_with_level_size— runsBFSDeepCrawlStrategy._arun_batchagainst a mocked crawler with a levelof 2,500 URLs and then one of 20,000 URLs, and asserts the wall-clock
growth ratio stays under 12x for an 8x increase in level size (linear
bookkeeping keeps this near 8x; O(n²) bookkeeping pushes it well past
it). This is a self-relative check (small vs. large run of the same
code) so it isn't tied to absolute machine speed.
test_shared_link_is_scored_and_enqueued_once— start URL links topages A and B, both of which link to a shared third URL. Asserts the
shared URL's scorer is called exactly once and it appears exactly once
in the yielded results.
Both tests fail on the pre-fix code and pass after the fix:
Full existing deep-crawling unit suite still green (excluding two
pre-existing failures caused by Playwright's browser binary not being
installed in this sandbox — confirmed identical on unmodified
main):Notes
CONTRIBUTING.md, PRs should target thedevelopbranch rather thanmain— please retarget if this was opened againstmain.reviewed and verified locally before submission.
🤖 Generated with Claude Code