[#157] Find XML entries by __UID__ and __NAME__ through an index

[maximthomas]

Problem

After #176 and #177 the save is linear and read-only calls no longer write the file, but every lookup of an entry is still an XQuery over the live Xerces DOM. Saxon walks the container through NodeList.item(i), and on Xerces 2.6.2 that makes finding the entry at position k cost O(k^2), and a lookup that finds nothing walks the whole document. create (duplicate check), update and delete (two lookups each), authenticate and every search by __UID__ or __NAME__ pay this, so a facade call on a store of 40,000 entries still takes seconds although nothing else in it is slow.

Change

• New EntryIndex (package-private). It holds the entries by object class, __UID__ and __NAME__, and answers with the entries that the XQuery /icf:OpenICFContainer//ri:CLASS[$x/icf:FIELD = 'VALUE'] gives, in the same order:
  • entries at any depth below the container, not only its children;
  • only identifier child elements in the ICF namespace count (a __NAME__ from another namespace does not);
  • the string value is compared as it is, CDATA included, with no trimming;
  • an entry is found under every value of its identifier elements;
  • duplicates come in document order, so the first of several is the one that create, update, delete and authenticate use, as before;
  • a document whose root is not the container indexes nothing.
• How the index stays in step with the document:
  • it is rebuilt every time a document is loaded;
  • create, update and delete report their changes to it under the handler's write lock. update reports in a finally, because a rejected update may already have changed the entry (a failed rename leaves no stale name);
  • an element inside an entry that is itself named like an object class (a schema attribute with the name of an object class) is an entry for the XQuery's //ri:CLASS, and so for the index: create also indexes the entries inside the new entry, and an update that writes such an element rebuilds the index, because an entry in the middle of the document has no place in its document order yet;
  • one hand-edited shape is not followed: an entry inside another entry's identifier element is part of that identifier's value, and changing or deleting it does not re-key the outer entry until the index is rebuilt. The connector never writes that shape;
  • dispose() rebuilds it only if XmlDocumentWriter.normalizeText reports that it changed the tree (it now returns boolean). A loaded document is normalized at load, and create and update write one text node per value, so this happens only when a whitespace-only value was written in memory (an optional attribute set to spaces; for the index it matters only when that attribute is an identifier).
  • Reads do not change what the index holds, so concurrent readers are safe as long as nobody changes the index.
• What is answered from the index:
  • the duplicate check of create, and the lookups of update, delete and authenticate; update and delete do one lookup instead of two;
  • getObject, and searches whose filter is a single = on __UID__ or __NAME__ (identifierField recognizes the translated comparison; for an object class without __UID__ the framework's __UID__ filter is already translated to __NAME__);
  • searches without a filter.
  • Identifier values are compared as they are, so a value that contains & no longer fails with an XQuery syntax error, and identifier lookups use the namespaces of their own configuration, not the JVM-wide ones that the last configured handler set. Searches on other attributes still build the XQuery as before.
• What is not: every other filter still runs the XQuery of QueryBuilder over the DOM and stays quadratic (the issue's step 5, a Java evaluator for the other filters, is not part of this PR). Those searches also still call NodeList.item(i) inside Saxon, which updates the document's node list cache under the read lock; this PR does not remove those writes, it only takes the identifier lookups and the unfiltered search out of them. A search on a connector without a loaded document fails with the same message as before.
• Plumbing: XMLHandler.search(Query, ObjectClass) (also on ConcurrentXMLHandler, under the read lock), which XMLConnector.executeQuery now calls instead of building the XQuery string itself; ComparisonQuery keeps the raw value and its getters; XMLFilterTranslator hands the raw value over and its XQuery text is unchanged; its rule that __UID__ is the __NAME__ element for an object class without __UID__ is now the static XMLFilterTranslator.elementName, which the handler's own lookups use too.

Measurements

Time through ConnectorFacade, one call per measurement (init(), the operation, dispose()), with the issue's XmlPerf harness (-Dreps=3, ri:__ACCOUNT__ entries with __UID__, __NAME__ and a few more fields; the lookups are for the middle entry). "Before" is master with #176 and #177 (ef2f7ee4). "After" is a848dc38 of this branch, which differs from the final branch in that every dispose() of a changed document rebuilt the index (about 20 to 85 ms at 10,000 to 40,000 entries); update and create end in such a dispose, so their "after" cells include the rebuild. The final branch also walks the elements of each created or updated entry once more, to find entries inside it; that was not measured. JDK 26 (OpenJDK Zulu26.28+59-CA) on macOS. The machine was not idle (load average 4 to 6), so the absolute values are noisy; the gap between the sides is not. Each cell is the median of three runs, alternating before and after.

Entries	search __NAME__ before	after	authenticate before	after	update before	after
10,000	162 ms	3 ms	40 ms	2 ms	127 ms	70 ms
20,000	586 ms	3 ms	151 ms	2 ms	402 ms	131 ms
40,000	2,144 ms	5 ms	547 ms	4 ms	1,288 ms	248 ms

Entries	create before	after	search all before	after	getObject before	after
10,000	225 ms	80 ms	269 ms	176 ms	169 ms	2 ms
20,000	651 ms	140 ms	683 ms	179 ms	540 ms	4 ms
40,000	2,313 ms	250 ms	2,425 ms	293 ms	2,159 ms	5 ms

(getObject was timed by a separate small probe, three calls per size and run, nine samples per cell.)

Growth per doubling of the number of entries (median at 2N over median at N, 10,000 to 20,000, then 20,000 to 40,000):

	before	after
search __NAME__	3.6, 3.7	1.0, 1.7 (3, 3, 5 ms)
authenticate	3.8, 3.6	1.0, 2.0 (2, 2, 4 ms)
update	3.2, 3.2	1.9, 1.9
create	2.9, 3.6	1.8, 1.8

Through the facade, identifier lookups take 2 to 5 ms at every size. With the document held by another instance, which leaves out the facade's per-call work, a search takes 1 ms at every size, an update 0 to 1 ms and a create 1 to 2 ms, so the lookups themselves no longer grow with the file. update and create still grow by about 1.8 per doubling: what is left in them is the save, which writes the whole file. A search without a filter returns every entry, so it stays proportional to N; it no longer walks the document through Saxon.

dispose() after a change, at 39504d62, whose dispose() is the same as on the final branch (handler API, update of one entry and then dispose() on a document of N entries; four disposes per run, three runs per side, median of the nine from the second dispose on; a clean document returns from dispose() at once on both sides, 0 ms), and the parse, at a848dc38 with the issue's XmlBreakdown (three runs of three handler instances, the first of each run cold; the final branch builds the index during the parse with fewer objects per entry, see the memory figures below, and that was not timed):

Entries	dirty dispose() before	after	parse before	after
10,000	57 ms	72 ms	75 ms	96 ms
20,000	103 ms	122 ms	142 ms	134 ms
40,000	224 ms	302 ms	228 ms	264 ms

The dirty dispose() is not faster, and in this measurement it is 15 to 78 ms slower than before, with ranges that overlap (40,000 entries: 188 to 296 ms before, 193 to 389 ms after). The index is not rebuilt in these disposes, because the loaded document is already normalized, so the difference is not the rebuild; I did not isolate its cause. The parse now also builds the index and is about 15% slower at 40,000 entries (228 to 264 ms). The facade tables above include the parse and a dispose. The index also takes memory: about 350 bytes per entry, 13.4 MiB at 40,000 entries with a __UID__ and a __NAME__ each, next to about 27 MiB for the DOM (heap after a full GC on JDK 26, a second index built over the loaded document, two runs of three measurements: 13,708 to 13,755 KiB; with a LinkedHashSet of keys per entry, as the branch had before, it took 23,040 to 24,048 KiB).

The numbers in #157 (a search by __NAME__ at 40,000 entries took 13.5 s, an authenticate 9.7 s, an update 10.5 s, a create 12.0 s; with the document loaded, 4.4 s, 2.1 s and 4.2 s for search, update and create) were taken before #176, on another host and JDK, so they are not comparable cell by cell. The baseline here already contains part 1, which is why "before" is at 2 s and not at 13 s. The change that the issue predicts for its steps 1 to 4 shows up as expected: identifier lookups are O(1), and a change still pays for one write of the file (about a quarter of a second at 40,000 entries here).

Tests

mvn -o -pl OpenICF-xml-connector verify over the whole module on JDK 26 and on JDK 11: surefire 227 run, 0 failed, 0 skipped; failsafe 1 run, 0 failed (XMLConnectorBundleIT), on both.

• EntryIndexTests (15): entries by name and uid in document order; nested entries are found and what is not an entry is ignored; object classes are separate; every identifier value finds the entry once; values are compared as they are; the root must be the container, and an entry added to a document with another root is not indexed; added puts an entry last and indexes the entries inside it; removed exposes the next duplicate and takes the entries inside a removed wrapper along; rekeyed keeps the entry's place in document order, an entry whose name was removed is found by uid only, an entry written into an earlier entry takes its place in document order, and without such a new entry the rest of the index is left as it is.
• XMLHandlerIndexTests (14, on a fixture with nested entries, duplicates and a foreign-namespace __NAME__):
  • delete removes the first duplicate and exposes the next; a renamed entry is found only under its new name; a failed rename leaves no stale name, and the index finds the entry under its new name; authenticate finds the entry by name; a name that contains & is found by a search and by authenticate (create failed on it before this change); identifier lookups use the handler's own namespaces after another handler with another ri namespace was configured; a replaced value takes the entries inside it along; a class without __UID__ is found by name;
  • indexAgreesWithXQueryAfterChanges compares the index with the XQuery for 24 identifier values (every name in the fixture, a sample of uids, the empty and a missing value), for the search without a filter and for the empty query, at each state of the document (loaded; after a create, a rename back to a duplicate, a delete, the replacement of a value that holds an entry, a rejected rename, and a save);
  • the save rebuilds the index when normalization empties an identifier; only a single = on an identifier goes to the index (identifierField: a negated =, startsWith and >= do not); a query on another attribute falls back to XQuery; lookupsAndSavesNeverUseNodeLists asserts that identifier lookups and the save never call NodeList.item; a search without a document fails as before.
• XMLHandlerReloadTests.externalCopyWithAnOlderMtimeIsLoaded also authenticates after the reload, through the index; the other reload tests read through the XQuery string search, which does not use it.
• XMLFilterTranslatorTests.comparisonExpressionsAreUnchanged: the XQuery text of the translated comparisons is the same as before the change (it was green on the unchanged translator first).
• XMLConnectorScaleTests: 20,000 entries, then for 20 names a search by __NAME__, a getObject and an authenticate, plus one search for a missing name (61 lookups), all through the facade after an untimed first call. The limit is 5 s. It takes 343 ms. With the identifier routing in XMLHandlerImpl switched off the same run takes 24.8 s, and with XMLConnector.executeQuery sending the XQuery string again it takes 30.0 s, so both fail the limit. The test removes its handler from the static handler cache at the end.
• XMLConnectorBundleIT: a facade search on lastname (a non-identifier attribute) on the packaged bundle, with the framework's result filter switched off so that a connector that ignored the query would show. This is the one call that still runs Saxon XQJ under the bundle's class loader. It is the only test that fails if executeQuery hands the handler a null query; the unit and scale tests do not see that, because the framework filters the results again.
• XmlDocumentWriterTests: normalizeText returns false for a normalized document and true when it removed blank text or merged CDATA and adjacent text.

Fixes #157

[vharseko]

Reviewed at 6beb334. Nothing blocking from me; a few small points.

Not from this PR, filed separately as #182. update, delete and authenticate break on an entry below another element: removeChildrenFromElement removes same-named descendants as children (DOMException NOT_FOUND_ERR midway, and the outer entry is saved without its __NAME__), delete calls removeChild on the container only, and authenticate reads __PASSWORD__ from any descendant (an inner entry's password is accepted, a missing one gives an NPE). I reproduced all of it on master as well as on this branch, so the PR neither causes nor needs to fix it; it just keeps finding these entries, as the // XQuery did.

Small points:

1. XMLHandlerImpl.identifierElement (L716-L720) repeats XMLFilterTranslator.getAttributeName's __UID__ → __NAME__ rewrite, as its comment says. Before, getEntry built its lookup through the translator, so the rule lived in one place. A static helper used by both would keep create/update/delete/authenticate and the search keyed on the same identifier if the rule ever changes.
2. EntryIndex has the same pre-order walk with following() three times (addAll, removed, holdsUnindexedEntry). One walk helper taking a per-node callback would leave one copy to keep right.
3. Optional, not measured: of the ~600 B per entry you measured, part is the LinkedHashSet<Key> per entry for usually one or two keys; a small array or ArrayList would do, since keysOf only needs to drop a repeated key.

[maximthomas]

Answers to review 5487963111 (of 6beb334a). The branch is now at ccd2125f, three commits on top of 6beb334a.

#182. Agreed, it stays out of this PR. The index finds those entries where the // XQuery found them; indexAgreesWithXQueryAfterChanges compares the two on a fixture with nested entries.

1. __UID__ → __NAME__ in two places. Done in fba445b. The rule is now XMLFilterTranslator.elementName(attributeName, preferUidOverName), and getAttributeName and XMLHandlerImpl.identifierElement both call it. On the lookup side the rewrite does not fire today: AUTO picks __UID__ only when the object class has it, which is what isSupportUid checks, BY_NAME is __NAME__ already, and nothing passes BY_UID. So the handler's call matters only if one of those changes. A mutant that drops the call from identifierElement leaves all 227 tests green; one that ignores the flag in elementName fails XMLHandlerIndexTests.onlySingleEqualityOnAnIdentifierGoesToTheIndex.

2. Three copies of the walk. Done in 3e9db1e. entriesIn(subtree) is the one walk: it returns the entries of the subtree in document order, and addAll, removed and holdsUnindexedEntry loop over it. I used a returned list rather than a callback, so the three stay plain loops; the list holds the entries of one subtree, which is one or two except in rebuild, where it holds every entry for the length of the call. The round-1 mutants of these paths (added indexing only the new entry, no rebuild for an unindexed inner entry, a rebuild on every update) and two new ones (no descent below an entry, removed taking only the root) each still fail their test.

3. LinkedHashSet<Key> per entry. Done in ccd2125, and measured. keysOf returns a Key[] and drops a repeated key with a contains check; EntryIndexTests.everyIdentifierValueFindsTheEntryOnce fails without that check. Heap after a full GC of a second EntryIndex built over a loaded document of 40,000 accounts with a __UID__ and a __NAME__ each, JDK 26, two runs of three measurements, alternating before and after: 23,040 to 24,048 KiB before, 13,708 to 13,755 KiB after, so about 600 bytes per entry became about 350. The DOM plus the handler's own index went from about 51,000 KiB to about 41,750 KiB.

The PR body has the new memory figures, the shared rule under Plumbing, and a note that the parse, timed at a848dc38, now builds the index with fewer objects per entry and was not timed again. mvn -o -pl OpenICF-xml-connector verify at ccd2125f, on JDK 26 and on JDK 11: surefire 227 run, 0 failed, 0 skipped; failsafe 1 run, 0 failed.

[vharseko]

Re-reviewed the fixes at ccd2125f (6beb334a..ccd2125f). All three points are resolved:

1. XMLFilterTranslator.elementName is now the single place for the __UID__ → __NAME__ rule, and identifierElement calls it with isSupportUid(objClass), which is the same flag XMLConnector.createFilterTranslator builds the translator with. Same result as before.
2. entriesIn(subtree) is the one walk. removed() no longer skips the isEntry check, but only entries are ever put into entries, so it behaves the same.
3. Key[] per entry: link/unlink work unchanged, and the drop in heap from ~600 B to ~350 B per entry is a good result.

Checked locally on JDK 21: the five index-related test classes are green at ccd2125f. With the contains check in keysOf removed, EntryIndexTests.everyIdentifierValueFindsTheEntryOnce fails as you described.

Nothing further from me. LGTM.