Skip to content

perf(crawler): reuse JAXP factories in XmlTransformer#176

Merged
marevol merged 4 commits into
masterfrom
perf/crawler-xml-transformer
Jul 9, 2026
Merged

perf(crawler): reuse JAXP factories in XmlTransformer#176
marevol merged 4 commits into
masterfrom
perf/crawler-xml-transformer

Conversation

@marevol

@marevol marevol commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

XmlTransformer rebuilt heavyweight JAXP objects on every crawled document:

  • transform() called DocumentBuilderFactory.newInstance() (JAXP service discovery) plus ~13 setFeature/setAttribute calls per document.
  • getNodeList() built a brand-new XPathFactory + XPath for every field rule of every document — the pre-existing per-thread XPathAPI cache was never actually used for node selection.

This PR reuses those objects while keeping behavior and thread-safety intact.

Changes

  • DocumentBuilderFactory: the configuration block is extracted verbatim into createDocumentBuilderFactory() and held in a per-thread ThreadLocal<DocumentBuilderFactory> initialized in init(). transform() now reuses the thread's configured factory and only calls newDocumentBuilder() per document (a builder is stateful). All security features/attributes (secure processing, external general/parameter entity disabling, external DTD/schema access) and parser options are applied with identical keys, values, and order.
  • XPath: getNodeList() now goes through the existing per-thread XPathAPI cache via a new XPathAPI.selectNodeList(node, expression, namespaceContext), which reuses the thread's XPath instead of rebuilding XPathFactory/XPath per rule.

Design note: why compiled expressions are not cached across documents

A compiled XPathExpression freezes namespace-prefix resolution at compile time. XmlTransformer resolves prefixes with DefaultNamespaceContext, which walks the current document's own xmlns:* declarations — a genuinely per-document context. Caching a compiled expression by rule string alone would silently return wrong/empty results whenever two documents map the same prefix to different URIs. So selectNodeList deliberately rebinds the namespace context and recompiles per call; the win is eliminating the per-rule XPathFactory.newInstance() and the per-document factory reconfiguration, not expression compilation.

Thread-safety

All reuse is thread-confined: the DocumentBuilderFactory is a real per-Thread ThreadLocal, and the reused XPath comes from the XPathAPI cache keyed per crawler thread (the same pattern already used by the sibling HtmlXpathExtractor/HtmlExtractor). No JAXP object is shared across threads.

Testing

  • New regression tests in XmlTransformerTest:
    • test_transform_sequentialDocuments — two different documents through the same transformer instance (then the first again), asserting exact output each time (guards against factory/state leakage).
    • test_transformNs_sequentialDocuments — a namespaced variant where the second document reuses the same prefix bound to a different namespace URI, specifically guarding the compile-time namespace-freeze pitfall.
    • test_transformNs_concurrentDocuments — 20 tasks over a 4-thread pool, each with a uniquely-namespaced document on a shared transformer instance.
  • Full fess-crawler module test suite passes (1981 tests, 0 failures); mvn formatter:format / license:format / javadoc:jar all clean.

Relation to #175

Independent of #175 (the regex/hot-path quick-wins); this PR branches from master and touches a disjoint set of files.

marevol added 4 commits July 9, 2026 01:03
Avoid per-document JAXP service discovery and reconfiguration in
XmlTransformer. transform() previously ran DocumentBuilderFactory.newInstance()
plus ~13 setFeature/setAttribute calls on every document, and getNodeList()
built a fresh XPathFactory + XPath for every field rule of every document
(the pre-existing per-thread XPathAPI cache was never actually used for node
selection).

- Configure the DocumentBuilderFactory once per thread (ThreadLocal) and only
  call newDocumentBuilder() per document; features/attributes are applied with
  identical keys, values, and order.
- Route getNodeList through the existing per-thread XPathAPI cache via a new
  XPathAPI.selectNodeList(...), reusing the thread's XPath instead of rebuilding
  XPathFactory/XPath per rule. The namespace context is rebound and the
  expression recompiled on every call, because DefaultNamespaceContext is
  per-document and a compiled XPathExpression freezes prefix resolution at
  compile time; compiled expressions are therefore intentionally not cached
  across documents.

Behavior is preserved (same parsed DOM, same node selection, same security
features, same per-document namespace resolution) and all reuse is
thread-confined.

Add regression tests: sequential same-instance transforms (including a
namespaced document that reuses a prefix bound to a different URI) and a
4-thread concurrent transform, locking cross-document namespace correctness
and concurrent reuse.
The reused per-thread XPath in XmlTransformer kept the first document's
DefaultNamespaceContext (which strongly references the parsed DOM) resting
on it after the initial selectNodeList call, because the finally block only
restored the previous context when it was non-null. On a fresh XPathAPI the
previous context is null, so the first document's DOM stayed pinned for the
lifetime of the per-thread cached XPathAPI.

Reset the reused XPath to a shared, document-free empty NamespaceContext when
there is no previous context to restore (setNamespaceContext rejects null).
Correctness is unaffected because every call rebinds the context before
evaluation. Add a regression test that exercises the first-call path and
fails on the previous leaking finally block.
XmlTransformer is a DI singleton whose parser-config options are set only at
bean wiring (before init) and never mutated at runtime, so the reused
per-thread DocumentBuilderFactory only ever needs to be built once with the
final configuration. Remove the config-version tracking, the holder wrapper,
and invalidateDocumentBuilderFactory (which guarded a runtime reconfiguration
that never happens), keeping just the per-thread ThreadLocal factory reuse.
Setters now only assign their fields. Drop the two tests that exercised the
removed runtime-reconfiguration path.
@marevol marevol merged commit da175be into master Jul 9, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant