perf+cleanup: skip feeds in crawl discovery; lxml link extraction
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.
This commit is contained in:
parent
8298bf8618
commit
9e196bcd82
4 changed files with 155 additions and 68 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
||||
|
|
|
|||
|
|
@ -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 = (
|
||||
'<html><body>'
|
||||
'<a href="/a">A</a>'
|
||||
'<a href="https://other.com/x">ext</a>'
|
||||
'<a href="javascript:void(0)">js</a>'
|
||||
'<a href="#frag">frag</a>'
|
||||
'<a href="/b" rel="nofollow">nf</a>'
|
||||
'<a href="/c#sec">C</a>'
|
||||
'</body></html>'
|
||||
)
|
||||
|
||||
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."""
|
||||
|
||||
|
|
|
|||
|
|
@ -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"],
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue