Optimization: @getodk/xpath is faster without Iterable/Iterator (#377)

Author: eyelidlessness

.changeset/funny-carrots-buy.md

@@ -0,0 +1,6 @@
+---
+'@getodk/xforms-engine': patch
+'@getodk/xpath': patch
+---
+
+Improves XPath processing performance.

packages/xforms-engine/src/integration/xpath/adapter/traversal.ts

@@ -18,7 +18,9 @@ export const getContainingEngineXPathDocument = (node: EngineXPathNode): EngineX
 	return node.rootDocument;
 };
 
-export const getEngineXPathAttributes = (node: EngineXPathNode): Iterable<EngineXPathAttribute> => {
+export const getEngineXPathAttributes = (
+	node: EngineXPathNode
+): readonly EngineXPathAttribute[] => {
 	if (node.nodeType === 'static-element') {
 		return node.attributes;
 	}
@@ -39,7 +41,7 @@ export const getEngineXPathAttributes = (node: EngineXPathNode): Iterable<Engine
  * it throw? It might be nice to be alerted if the assumptions in point 2 above
  * are somehow wrong (or become wrong).
  */
-export const getNamespaceDeclarations = (): Iterable<never> => [];
+export const getNamespaceDeclarations = (): readonly [] => [];
 
 export const getParentNode = (node: EngineXPathNode): EngineXPathParentNode | null => {
 	if (node.nodeType === 'repeat-instance') {

packages/xpath/package.json

@@ -56,7 +56,6 @@
   "dependencies": {
     "@getodk/common": "0.5.2",
     "crypto-js": "^4.2.0",
-    "itertools-ts": "^2.2.0",
     "temporal-polyfill": "^0.3.0"
   },
   "devDependencies": {

packages/xpath/src/adapter/interface/XPathDOMOptimizableOperations.ts

@@ -25,7 +25,7 @@ export interface XPathDOMOptimizableOperations<T extends XPathNode> {
 	readonly getChildrenByLocalName: (
 		node: AdapterParentNode<T>,
 		localName: string
-	) => Iterable<AdapterElement<T>>;
+	) => ReadonlyArray<AdapterElement<T>>;
 
 	readonly getFirstChildNode: (node: T) => AdapterChildNode<T> | null;
 	readonly getFirstChildElement: (node: T) => AdapterElement<T> | null;

packages/xpath/src/adapter/interface/XPathTraversalAdapter.ts

@@ -22,11 +22,11 @@ export interface XPathTraversalAdapter<T extends XPathNode> {
 	 */
 	readonly getContainingDocument: (node: T) => AdapterDocument<T>;
 
-	readonly getNamespaceDeclarations: (node: T) => Iterable<AdapterNamespaceDeclaration<T>>;
-	readonly getAttributes: (node: T) => Iterable<AdapterAttribute<T>>;
+	readonly getNamespaceDeclarations: (node: T) => ReadonlyArray<AdapterNamespaceDeclaration<T>>;
+	readonly getAttributes: (node: T) => ReadonlyArray<AdapterAttribute<T>>;
 	readonly getParentNode: (node: T) => AdapterParentNode<T> | null;
-	readonly getChildNodes: (node: T) => Iterable<AdapterChildNode<T>>;
-	readonly getChildElements: (node: T) => Iterable<AdapterElement<T>>;
+	readonly getChildNodes: (node: T) => ReadonlyArray<AdapterChildNode<T>>;
+	readonly getChildElements: (node: T) => ReadonlyArray<AdapterElement<T>>;
 	readonly getPreviousSiblingNode: (node: T) => AdapterChildNode<T> | null;
 	readonly getPreviousSiblingElement: (node: T) => AdapterElement<T> | null;
 	readonly getNextSiblingNode: (node: T) => AdapterChildNode<T> | null;

packages/xpath/src/adapter/xpathDOMProvider.ts

@@ -1,5 +1,4 @@
 import type { UpsertableMap } from '@getodk/common/lib/collections/UpsertableMap.ts';
-import { filter } from '../lib/iterators/common.ts';
 import type { XPathDOMAdapter } from './interface/XPathDOMAdapter.ts';
 import type { XPathDOMOptimizableOperations } from './interface/XPathDOMOptimizableOperations.ts';
 import type { XPathNode } from './interface/XPathNode.ts';
@@ -118,53 +117,6 @@ const extendNodeKindGuards = <T extends XPathNode>(
 	return Object.assign(base, extensions);
 };
 
-type IterableNodeFilter<T extends XPathNode, U extends T> = (nodes: Iterable<T>) => Iterable<U>;
-
-/**
- * Provides frequently used operations, such as filtering and sorting, on
- * {@link Iterable} sequences of an {@link XPathDOMAdapter}'s node
- * representation.
- */
-interface IterableOperations<T extends XPathNode> {
-	readonly filterAttributes: IterableNodeFilter<T, AdapterAttribute<T>>;
-	readonly filterQualifiedNamedNodes: IterableNodeFilter<T, AdapterQualifiedNamedNode<T>>;
-	readonly filterComments: IterableNodeFilter<T, AdapterComment<T>>;
-	readonly filterNamespaceDeclarations: IterableNodeFilter<T, AdapterNamespaceDeclaration<T>>;
-	readonly filterProcessingInstructions: IterableNodeFilter<T, AdapterProcessingInstruction<T>>;
-	readonly filterTextNodes: IterableNodeFilter<T, AdapterText<T>>;
-
-	// Note: iterable -> array is intentional. Can't sort a lazy, arbitrary-order
-	// sequence without iterating every item!
-	readonly sortInDocumentOrder: (nodes: Iterable<T>) => readonly T[];
-}
-
-interface ExtendedIterableOperations<T extends XPathNode>
-	extends ExtendedNodeKindGuards<T>,
-		IterableOperations<T> {}
-
-/**
- * Derives frequently used {@link IterableOperations | iterable operations} from an
- * {@link XPathDOMAdapter} and its derived
- * {@link NodeKindGuards | node kind predicates}.
- */
-const extendIterableOperations = <T extends XPathNode>(
-	base: ExtendedNodeKindGuards<T>
-): ExtendedIterableOperations<T> => {
-	const extensions: IterableOperations<T> = {
-		filterAttributes: filter(base.isAttribute),
-		filterQualifiedNamedNodes: filter(base.isQualifiedNamedNode),
-		filterComments: filter(base.isComment),
-		filterNamespaceDeclarations: filter(base.isNamespaceDeclaration),
-		filterProcessingInstructions: filter(base.isProcessingInstruction),
-		filterTextNodes: filter(base.isText),
-		sortInDocumentOrder: (nodes: Iterable<T>): readonly T[] => {
-			return Array.from(nodes).sort((a, b) => base.compareDocumentOrder(a, b));
-		},
-	};
-
-	return Object.assign(base, extensions);
-};
-
 type UniqueIDElementLookup<T extends XPathNode> = (
 	node: AdapterDocument<T>,
 	id: string
@@ -180,29 +132,26 @@ const getElementByUniqueIdFactory = <T extends XPathNode>(
 		return adapterImplementation;
 	}
 
-	function* getElementDescendants(node: AdapterParentNode<T>): Iterable<AdapterElement<T>> {
-		if (adapter.isElement(node)) {
-			yield node;
-		}
-
-		for (const element of adapter.getChildElements(node)) {
-			yield element;
-
-			yield* getElementDescendants(element);
+	const getElementByUniqueId = (
+		node: AdapterParentNode<T>,
+		id: string
+	): AdapterElement<T> | null => {
+		if (adapter.isElement(node) && getNamedAttributeValue(node, 'id') === id) {
+			return node;
 		}
-	}
 
-	return (node, id) => {
-		const containingDocument = adapter.getContainingDocument(node);
+		for (const childElement of adapter.getChildElements(node)) {
+			const element = getElementByUniqueId(childElement, id);
 
-		for (const element of getElementDescendants(containingDocument)) {
-			if (getNamedAttributeValue(element, 'id') === id) {
+			if (element != null) {
 				return element;
 			}
 		}
 
 		return null;
 	};
+
+	return getElementByUniqueId;
 };
 
 type QualifiedNamedAttributeValueLookup<T extends XPathNode> = (
@@ -286,7 +235,7 @@ const hasLocalNamedAttributeFactory = <T extends XPathNode>(
 type LocalNamedChildElementsLookup<T extends XPathNode> = (
 	node: AdapterParentNode<T>,
 	localName: string
-) => Iterable<AdapterElement<T>>;
+) => ReadonlyArray<AdapterElement<T>>;
 
 const getChildrenByLocalNameFactory = <T extends XPathNode>(
 	adapter: XPathDOMAdapter<T>
@@ -298,7 +247,7 @@ const getChildrenByLocalNameFactory = <T extends XPathNode>(
 	}
 
 	return (node, localName) => {
-		return Array.from(adapter.getChildElements(node)).filter((element) => {
+		return adapter.getChildElements(node).filter((element) => {
 			return adapter.getLocalName(element) === localName;
 		});
 	};
@@ -364,7 +313,7 @@ const getLastChildElementFactory = <T extends XPathNode>(
 	}
 
 	return (node) => {
-		return Array.from(adapter.getChildElements(node)).at(-1) ?? null;
+		return adapter.getChildElements(node).at(-1) ?? null;
 	};
 };
 
@@ -376,7 +325,7 @@ const getLastChildElementFactory = <T extends XPathNode>(
 type OmitOptionalOptimizableOperations<T> = Omit<T, keyof XPathDOMOptimizableOperations<XPathNode>>;
 
 interface ExtendedOptimizableOperations<T extends XPathNode>
-	extends OmitOptionalOptimizableOperations<ExtendedIterableOperations<T>>,
+	extends OmitOptionalOptimizableOperations<ExtendedNodeKindGuards<T>>,
 		XPathDOMOptimizableOperations<T> {}
 
 /**
@@ -391,7 +340,7 @@ interface ExtendedOptimizableOperations<T extends XPathNode>
  *    will be derived from other aspects of the adapter's required APIs.
  */
 const extendOptimizableOperations = <T extends XPathNode>(
-	base: ExtendedIterableOperations<T>
+	base: ExtendedNodeKindGuards<T>
 ): ExtendedOptimizableOperations<T> => {
 	const getLocalNamedAttributeValue = getLocalNamedAttributeValueFactory(base);
 
@@ -451,7 +400,6 @@ const derivedDOMProvider = <T>(base: T): DerivedDOMProvider & T => {
 export interface XPathDOMProvider<T extends XPathNode>
 	extends OmitOptionalOptimizableOperations<XPathDOMAdapter<T>>,
 		NodeKindGuards<T>,
-		IterableOperations<T>,
 		XPathDOMOptimizableOperations<T>,
 		DerivedDOMProvider {}
 
@@ -477,8 +425,7 @@ export const xpathDOMProvider = <T extends XPathNode>(
 	}
 
 	const extendedGuards = extendNodeKindGuards(adapter);
-	const extendedIterableOperations = extendIterableOperations(extendedGuards);
-	const exended = extendOptimizableOperations(extendedIterableOperations);
+	const exended = extendOptimizableOperations(extendedGuards);
 
 	return derivedDOMProvider(exended);
 };

packages/xpath/src/context/Context.ts

@@ -30,7 +30,7 @@ export interface Context<T extends XPathNode> {
 
 	readonly contextDocument: AdapterDocument<T>;
 	readonly rootNode: AdapterParentNode<T>;
-	readonly contextNodes: Iterable<T>;
+	readonly contextNodes: ReadonlySet<T>;
 
 	contextPosition(): number;
 	contextSize(): number;

packages/xpath/src/context/EvaluationContext.ts

@@ -33,7 +33,7 @@ export class EvaluationContext<T extends XPathNode> implements Context<T> {
 	readonly contextDocument: AdapterDocument<T>;
 	readonly rootNode: AdapterParentNode<T>;
 
-	readonly contextNodes: Iterable<T>;
+	readonly contextNodes: ReadonlySet<T>;
 
 	readonly functions: FunctionLibraryCollection;
 	readonly namespaceResolver: NamespaceResolver<T>;
@@ -56,7 +56,7 @@ export class EvaluationContext<T extends XPathNode> implements Context<T> {
 
 		this.contextDocument = contextDocument;
 		this.evaluationContextNode = contextNode;
-		this.contextNodes = [contextNode];
+		this.contextNodes = new Set([contextNode]);
 		this.rootNode = rootNode;
 		this.functions = options.functions ?? evaluator.functions;
 		this.namespaceResolver = NamespaceResolver.from(

packages/xpath/src/evaluations/Evaluation.ts

@@ -8,7 +8,7 @@ export interface Evaluation<T extends XPathNode, Type extends EvaluationType = E
 	readonly type: Type;
 
 	first(): Evaluation<T, Type> | null;
-	values(): Iterable<Evaluation<T, Type>>;
+	values(): ReadonlyArray<Evaluation<T, Type>>;
 
 	eq(operand: Evaluation<T>): boolean;
 	ne(operand: Evaluation<T>): boolean;
@@ -21,5 +21,5 @@ export interface Evaluation<T extends XPathNode, Type extends EvaluationType = E
 	toNumber(): number;
 	toString(): string;
 
-	readonly nodes: Type extends 'NODE' ? Iterable<T> : null;
+	readonly nodes: Type extends 'NODE' ? ReadonlySet<T> : null;
 }