Repository navigation
#158 jsoup XPathFilter: convert to W3C DOM once per page, not once per expression - #2252
Conversation
…r expression Element.selectXpath() converts the whole document to a W3C DOM and creates a new XPathFactory on every call. XPathFilter and LinkParseFilter now build the W3C DOM once per page and compile their expressions at configuration time. Co-Authored-By: Claude Opus 5.5 <[email protected]>
rzo1
left a comment
There was a problem hiding this comment.
LGTM. Converting once per page and compiling the expressions up front gives the same results as selectXpath and saves a lot. Two small things: LinkParseFilter now returns early when there are no expressions, so it no longer rebuilds the outlinks in that case; that's probably fine, but worth knowing. And LOG.error("... {}: {}", url, e) fills the second placeholder with e, so the stack trace is lost. A small test that an invalid expression now fails in configure() would be nice.
…figure Address review comments: pass the exception to SLF4J as a throwable rather than filling a placeholder, document the early return in LinkParseFilter and check that an invalid XPath fails in configure(). Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thanks @rzo1! Addressed in abb9795: the exception is now passed as a throwable so the stack trace is kept (fixed the two existing "Error evaluating" calls too, and they now include the URL). Added a test that an invalid expression fails in |
dpol1
left a comment
There was a problem hiding this comment.
looks good, will merge when is green, ahm, hopefully
Related to #158.
jsoup's
Element.selectXpath()is not DOM-free: on every call it converts the whole document to a W3C DOM (NodeUtils.selectXpath→W3CDom.fromJsoup), creates a newXPathFactoryand compiles the expression.jsoup.XPathFiltercalls it at least once per key andjsoup.LinkParseFilteronce per pattern, so a page was converted once per expression evaluated.Changes
jsoup.XPathFilterbuilds the W3C DOM once per page (W3CContext, namespace-unaware as in jsoup'sselectXpath) and evaluates all its expressions against it, mapping the results back to the jsoup elements withW3CDom.sourceNodes().jsoup.LinkParseFilteruses the same shared context.The extraction syntax (
/tidyText(),/allText(),/html(),/@attr) and the results are unchanged.Performance
Old vs new
XPathFilter+LinkParseFilterwith the archetype'sjsoupfilters.json(6 XPath expressions over 4 keys + 1 link pattern), on the test pages fromcore/src/test/resources. Average time per page after warm-up, single thread, JDK 17. The outputs (metadata and outlinks) were checked to be identical.On a page with a large DOM, a single W3C conversion costs about as much as the jsoup parsing itself; before this change the two filters converted the page ~5 times, taking more than 4x the time spent parsing it. On small pages the gain comes mostly from no longer creating an
XPathFactoryand compiling the expressions on every call. The gain grows with the number of expressions configured.Behaviour changes
W3CDom.XPathFactoryPropertysystem property (to plug in another XPath engine) is no longer taken into account; expressions are compiled with the default JDKXPathFactory.Not covered
Each filter still does its own conversion, so a configuration with both
XPathFilterandLinkParseFilter(as in the archetypes) converts the page twice. Sharing the W3C DOM across filters would require caching it per page, which could be stale if a previous JSoupFilter modified the document.🤖 Generated with Claude Code