From 9e196bcd8258618972eb53705949a2fcdb963169 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Fri, 22 May 2026 06:46:55 -0400 Subject: [PATCH] perf+cleanup: skip feeds in crawl discovery; lxml link extraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two crawler-discovery changes surfaced while chasing fast-crawl wall time on russell.ballestrini.net: 1. Feed-skip in BFS discovery: the bridge fetched feed/sitemap URLs (a multi-MB atom.xml among them) only for ingest_crawled to discard them. Gate enqueue on the existing _looks_like_feed_url so we never fetch crawl-infrastructure URLs — less wasted work and one fewer slow wave straggler. 2. lxml link extraction, DRY'd: the three duplicated BeautifulSoup html.parser closures (fresh fetch + 2 cache paths) collapse into one module-level extract_page_links() backed by lxml.html (C parser, releases the GIL so to_thread actually parallelises) with a BS4 fallback for markup lxml rejects. Parse on a 24-page wave 3.5s->2.5s. Honest scope: neither moves full-crawl wall time much — measurement showed the dominant cost is the per-page HEAD+GET double round-trip on a per-call ClientSession, not parsing. These are correct-and-cleaner on their own; the wall-time lever (shared session + drop redundant HEAD) is a separate change. lxml extraction is regression-pinned against the BS4 fallback for parity. --- arborist/sources/crawler/async_web_fetcher.py | 144 +++++++++--------- arborist/sources/crawler/bridge.py | 8 + tests/crawler/test_async_web_fetcher.py | 51 +++++++ tests/crawler/test_bridge.py | 20 +++ 4 files changed, 155 insertions(+), 68 deletions(-) diff --git a/arborist/sources/crawler/async_web_fetcher.py b/arborist/sources/crawler/async_web_fetcher.py index 1ca32e4..0c3ac48 100644 --- a/arborist/sources/crawler/async_web_fetcher.py +++ b/arborist/sources/crawler/async_web_fetcher.py @@ -31,6 +31,7 @@ import time # touch the network and must not import this module unless asked. try: import aiohttp + import lxml.html from bs4 import BeautifulSoup from miniuri import Uri except ImportError as e: # pragma: no cover @@ -314,6 +315,69 @@ def normalize_link(link) -> Tuple[str, str]: strip_url_fragment = strip_uri_fragment +def extract_page_links(html, base_url, extract_anchor_text=False): + """Extract followable same-page links from HTML for BFS crawling. + + Parses with ``lxml.html`` (a C parser that releases the GIL during + the parse), so calling this under ``asyncio.to_thread`` across a wide + fetch wave actually parallelises — unlike BeautifulSoup's pure-Python + ``html.parser``, which holds the GIL and serialises every "concurrent" + parse. Falls back to BeautifulSoup if lxml chokes on the markup. + + Honors the same skip rules as before: ``javascript:`` / ``#`` hrefs + and ``rel="nofollow"`` are dropped; only http(s) links survive; + fragments are stripped. Returns a list of URL strings, or — when + ``extract_anchor_text`` — a list of ``{"url", "anchor_text"}`` dicts. + """ + try: + doc = lxml.html.fromstring(html) + except Exception: + return _extract_page_links_bs4(html, base_url, extract_anchor_text) + + result = [] + for a_tag in doc.iter("a"): + href = a_tag.get("href") + if not href or href.startswith(("javascript:", "#")): + continue + rel = (a_tag.get("rel") or "").split() + if "nofollow" in rel: + continue + absolute_url = strip_url_fragment(urljoin(base_url, href)) + parsed = Uri(absolute_url) + if parsed.scheme in ("http", "https"): + if extract_anchor_text: + anchor = (a_tag.text_content() or "").strip() + result.append({"url": absolute_url, "anchor_text": anchor}) + else: + result.append(absolute_url) + return result + + +def _extract_page_links_bs4(html, base_url, extract_anchor_text=False): + """BeautifulSoup fallback for :func:`extract_page_links` — same rules, + pure-Python parser. Used only when lxml fails to parse the markup.""" + result = [] + soup = BeautifulSoup(html, "html.parser") + for a_tag in soup.find_all("a", href=True): + href = a_tag["href"] + if href.startswith(("javascript:", "#")): + continue + rel = a_tag.get("rel", []) + if isinstance(rel, str): + rel = rel.split() + if "nofollow" in rel: + continue + absolute_url = strip_url_fragment(urljoin(base_url, href)) + parsed = Uri(absolute_url) + if parsed.scheme in ("http", "https"): + if extract_anchor_text: + anchor_text = a_tag.get_text(strip=True) + result.append({"url": absolute_url, "anchor_text": anchor_text}) + else: + result.append(absolute_url) + return result + + def extract_text_from_pdf(pdf_bytes: bytes) -> Optional[str]: """ Extract text from PDF binary data. @@ -1641,28 +1705,9 @@ class AsyncWebFetcher: # If we need links but cache has none, re-extract from HTML (in thread to not block) if extract_links and not links and html: logger.info(f"💾 Cache hit but no links cached, re-extracting from HTML: {url}") - def _reextract_links(): - soup = BeautifulSoup(html, "html.parser") - result = [] - for a_tag in soup.find_all("a", href=True): - href = a_tag["href"] - if href.startswith(("javascript:", "#")): - continue - rel = a_tag.get("rel", []) - if isinstance(rel, str): - rel = rel.split() - if "nofollow" in rel: - continue - absolute_url = strip_url_fragment(urljoin(url, href)) - parsed = Uri(absolute_url) - if parsed.scheme in ("http", "https"): - if extract_anchor_text: - anchor_text = a_tag.get_text(strip=True) - result.append({"url": absolute_url, "anchor_text": anchor_text}) - else: - result.append(absolute_url) - return result - links = await asyncio.to_thread(_reextract_links) + links = await asyncio.to_thread( + extract_page_links, html, url, extract_anchor_text + ) logger.info(f"Re-extracted {len(links)} links from cached HTML") else: logger.info(f"💾 Using SQLite3 cached page: {url} (skipping robots.txt + crawl delay)") @@ -1676,28 +1721,9 @@ class AsyncWebFetcher: # If we need links but cache has none, re-extract from HTML (in thread to not block) if extract_links and not cached_links and html: logger.info(f"💾 In-memory cache hit but no links, re-extracting from HTML: {url}") - def _reextract_cached_links(): - soup = BeautifulSoup(html, "html.parser") - result = [] - for a_tag in soup.find_all("a", href=True): - href = a_tag["href"] - if href.startswith(("javascript:", "#")): - continue - rel = a_tag.get("rel", []) - if isinstance(rel, str): - rel = rel.split() - if "nofollow" in rel: - continue - absolute_url = strip_url_fragment(urljoin(url, href)) - parsed_link = Uri(absolute_url) - if parsed_link.scheme in ("http", "https"): - if extract_anchor_text: - anchor_text = a_tag.get_text(strip=True) - result.append({"url": absolute_url, "anchor_text": anchor_text}) - else: - result.append(absolute_url) - return result - cached_links = await asyncio.to_thread(_reextract_cached_links) + cached_links = await asyncio.to_thread( + extract_page_links, html, url, extract_anchor_text + ) logger.info(f"Re-extracted {len(cached_links)} links from in-memory cached HTML") else: logger.info(f"💾 Using in-memory cached page: {url} (cached {time.time() - cached_time:.0f}s ago, skipping crawl delay)") @@ -1789,32 +1815,14 @@ class AsyncWebFetcher: html = await response.text() logger.info(f"Successfully fetched {url} ({len(html)} bytes)") - # Extract links if requested (run in thread to not block event loop) + # Extract links if requested. lxml parsing releases the + # GIL, so running it in a thread genuinely parallelises + # across a wide fetch wave (html.parser would not). links = [] if extract_links: - def _extract_links(): - result = [] - soup = BeautifulSoup(html, "html.parser") - for a_tag in soup.find_all("a", href=True): - href = a_tag["href"] - if href.startswith(("javascript:", "#")): - continue - rel = a_tag.get("rel", []) - if isinstance(rel, str): - rel = rel.split() - if "nofollow" in rel: - continue - absolute_url = strip_url_fragment(urljoin(url, href)) - parsed = Uri(absolute_url) - if parsed.scheme in ("http", "https"): - if extract_anchor_text: - anchor_text = a_tag.get_text(strip=True) - result.append({"url": absolute_url, "anchor_text": anchor_text}) - else: - result.append(absolute_url) - return result - - links = await asyncio.to_thread(_extract_links) + links = await asyncio.to_thread( + extract_page_links, html, url, extract_anchor_text + ) logger.info(f"Extracted {len(links)} links from {url}") # Cache in memory for future fetches in this session diff --git a/arborist/sources/crawler/bridge.py b/arborist/sources/crawler/bridge.py index 74b211f..2fb8101 100644 --- a/arborist/sources/crawler/bridge.py +++ b/arborist/sources/crawler/bridge.py @@ -204,6 +204,14 @@ async def _crawl_seed_async( continue if not normalized.startswith(("http://", "https://")): continue + # Don't crawl feed/sitemap URLs: they're crawl + # infrastructure that ingest_crawled discards anyway, and + # a big feed (e.g. a multi-MB atom.xml) is a slow wave + # straggler that gates everything fetched alongside it. + # Mark seen so a later parent can't re-enqueue it. + if _looks_like_feed_url(normalized): + seen.add(normalized) + continue seen.add(normalized) queue.append((normalized, depth + 1)) diff --git a/tests/crawler/test_async_web_fetcher.py b/tests/crawler/test_async_web_fetcher.py index 9f8200d..6a03e03 100644 --- a/tests/crawler/test_async_web_fetcher.py +++ b/tests/crawler/test_async_web_fetcher.py @@ -28,6 +28,57 @@ from arborist.sources.crawler.async_web_fetcher import ( ) +class TestExtractPageLinks: + """Link extraction for BFS (lxml fast path + BS4 fallback parity).""" + + HTML = ( + '' + 'A' + 'ext' + 'js' + 'frag' + 'nf' + 'C' + '' + ) + + def test_lxml_extraction_rules(self): + from arborist.sources.crawler.async_web_fetcher import extract_page_links + + out = extract_page_links(self.HTML, "https://x.com/") + assert "https://x.com/a" in out # relative resolved + assert "https://other.com/x" in out # off-domain kept (bridge filters) + assert "https://x.com/c" in out # fragment stripped + assert not any("javascript" in u for u in out) + assert "https://x.com/b" not in out # rel=nofollow dropped + assert not any("#" in u for u in out) + + def test_lxml_matches_bs4_fallback(self): + from arborist.sources.crawler.async_web_fetcher import ( + extract_page_links, + _extract_page_links_bs4, + ) + + assert ( + extract_page_links(self.HTML, "https://x.com/") + == _extract_page_links_bs4(self.HTML, "https://x.com/") + ) + + def test_anchor_text_mode(self): + from arborist.sources.crawler.async_web_fetcher import extract_page_links + + out = extract_page_links(self.HTML, "https://x.com/", extract_anchor_text=True) + a = next(d for d in out if d["url"] == "https://x.com/a") + assert a["anchor_text"] == "A" + + def test_malformed_html_falls_back_gracefully(self): + from arborist.sources.crawler.async_web_fetcher import extract_page_links + + # Empty / whitespace markup makes lxml.fromstring raise — the + # helper must fall back, not crash. + assert extract_page_links(" ", "https://x.com/") == [] + + class TestCrawlMode: """Test CrawlMode enum.""" diff --git a/tests/crawler/test_bridge.py b/tests/crawler/test_bridge.py index 78bf621..edf6d6b 100644 --- a/tests/crawler/test_bridge.py +++ b/tests/crawler/test_bridge.py @@ -138,6 +138,26 @@ def test_crawl_seed_drops_off_domain_links(): assert "https://x.com/local" in out +def test_crawl_seed_skips_feed_urls_during_discovery(): + """Feed/sitemap links must not be crawled — they're discarded at + ingest and a big feed is a slow wave straggler. Pin that the BFS + drops them at enqueue (not just at ingest).""" + page_links = { + "https://x.com/": [ + "/a", + "/feeds/all.atom.xml", # big feed — must be skipped + "/sitemap.xml", + "/b.rss", + ], + "https://x.com/a": [], + } + out = _run_crawl(page_links, "https://x.com/", depth=2, max_pages=10) + assert "https://x.com/a" in out + assert "https://x.com/feeds/all.atom.xml" not in out + assert "https://x.com/sitemap.xml" not in out + assert "https://x.com/b.rss" not in out + + def test_crawl_seed_dedups(): page_links = { "https://x.com/": ["/a", "/a", "/b"],