Skip to content

Commit 5de1e3a

Browse files
committed
[optimize] Dispose eagerly of resources that were accumulated during XQuery exeution but are no longer needed in the Result sequence
1 parent 015c5d2 commit 5de1e3a

10 files changed

Lines changed: 530 additions & 84 deletions

File tree

exist-core/pom.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,6 +1105,7 @@
11051105
<include>src/test/java/org/exist/util/io/ByteArrayContentTest.java</include>
11061106
<include>src/main/java/org/exist/util/io/ContentFile.java</include>
11071107
<include>src/main/java/org/exist/util/io/ContentFilePoolObjectFactory.java</include>
1108+
<include>src/main/java/org/exist/util/io/FilterInputStreamCacheMonitor.java</include>
11081109
<include>src/test/java/org/exist/util/io/FilterInputStreamCacheMonitorTest.java</include>
11091110
<include>src/test/java/org/exist/util/io/OverflowToDiskStreamTest.java</include>
11101111
<include>src/main/java/org/exist/util/io/VirtualTempPath.java</include>
@@ -1910,6 +1911,7 @@
19101911
<exclude>src/test/java/org/exist/util/io/ByteArrayContentTest.java</exclude>
19111912
<exclude>src/main/java/org/exist/util/io/ContentFile.java</exclude>
19121913
<exclude>src/main/java/org/exist/util/io/ContentFilePoolObjectFactory.java</exclude>
1914+
<exclude>src/main/java/org/exist/util/io/FilterInputStreamCacheMonitor.java</exclude>
19131915
<exclude>src/test/java/org/exist/util/io/FilterInputStreamCacheMonitorTest.java</exclude>
19141916
<exclude>src/test/java/org/exist/util/io/OverflowToDiskStreamTest.java</exclude>
19151917
<exclude>src/main/java/org/exist/util/io/VirtualTempPath.java</exclude>

exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747

4848
import org.exist.EXistException;
4949
import org.exist.TestUtils;
50+
import org.exist.source.StringSource;
5051
import org.exist.storage.serializers.EXistOutputKeys;
5152
import org.exist.util.DatabaseConfigurationException;
5253
import org.exist.xmldb.EXistCollection;
@@ -199,8 +200,7 @@ private void stopXmlDb() throws XMLDBException {
199200

200201

201202
public ResourceSet executeQuery(final String query) throws XMLDBException {
202-
final CompiledExpression compiledQuery = xpathQueryService.compile(query);
203-
return xpathQueryService.execute(compiledQuery);
203+
return xpathQueryService.execute(new StringSource(query));
204204
}
205205

206206
public ResourceSet executeQuery(final String query, final Map<String, Object> externalVariables)

exist-core/src/main/java/org/exist/util/io/FilterInputStreamCacheMonitor.java

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,28 @@
11
/*
2+
* Elemental
3+
* Copyright (C) 2024, Evolved Binary Ltd
4+
*
5+
* admin@evolvedbinary.com
6+
* https://www.evolvedbinary.com | https://www.elemental.xyz
7+
*
8+
* This library is free software; you can redistribute it and/or
9+
* modify it under the terms of the GNU Lesser General Public
10+
* License as published by the Free Software Foundation; version 2.1.
11+
*
12+
* This library is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15+
* Lesser General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public
18+
* License along with this library; if not, write to the Free Software
19+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
20+
*
21+
* NOTE: Parts of this file contain code from 'The eXist-db Authors'.
22+
* The original license header is included below.
23+
*
24+
* =====================================================================
25+
*
226
* eXist-db Open Source Native XML Database
327
* Copyright (C) 2001 The eXist-db Authors
428
*
@@ -19,7 +43,6 @@
1943
* License along with this library; if not, write to the Free Software
2044
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
2145
*/
22-
2346
package org.exist.util.io;
2447

2548
import java.util.*;
@@ -44,15 +67,6 @@ public static FilterInputStreamCacheMonitor getInstance() {
4467
return INSTANCE;
4568
}
4669

47-
/**
48-
* Intentionally package private!
49-
*
50-
* Only for use by org.exist.util.io.FilterInputStreamCacheMonitorTest
51-
*/
52-
void clear() {
53-
activeCaches.clear();
54-
}
55-
5670
public void register(final FilterInputStreamCache cache) {
5771
final long now = System.currentTimeMillis();
5872
final FilterInputStreamCacheInfo info = new FilterInputStreamCacheInfo(now, cache);

exist-core/src/main/java/org/exist/xmldb/LocalXMLResource.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
import org.exist.util.serializer.SerializerPool;
8585
import org.exist.xquery.XPathException;
8686
import org.exist.xquery.value.AtomicValue;
87+
import org.exist.xquery.value.Item;
8788
import org.exist.xquery.value.NodeValue;
8889
import org.exist.xquery.value.StringValue;
8990
import org.exist.xquery.value.Type;
@@ -224,6 +225,24 @@ else if (root != null && !(root instanceof NodeValue)) {
224225
}
225226
}
226227

228+
/**
229+
* Allows access to the underlying XDM typed value.
230+
* Should be used sparingly!
231+
*
232+
* @return the Underlying Xdm value.
233+
*/
234+
public @Nullable Item getContentAsXdm() {
235+
if (root instanceof NodeValue) {
236+
return (NodeValue) root;
237+
}
238+
239+
if (value != null) {
240+
return value;
241+
}
242+
243+
return null;
244+
}
245+
227246
private String serialize(final DBBroker broker, final ConsumerE<Serializer, SAXException> toSaxFunction) throws SAXException, IOException {
228247
final Serializer serializer = broker.borrowSerializer();
229248
SAXSerializer saxSerializer = null;

exist-core/src/main/java/org/exist/xmldb/LocalXPathQueryService.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
import org.exist.xquery.XQueryContext;
7171
import org.exist.xquery.XQueryUtil;
7272
import org.exist.xquery.value.AnyURIValue;
73-
import org.exist.xquery.value.BinaryValue;
7473
import org.exist.xquery.value.Sequence;
7574
import org.exist.xquery.value.SequenceIterator;
7675
import org.w3c.dom.Node;

exist-core/src/main/java/org/exist/xquery/XQuery.java

Lines changed: 118 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151

5252
import java.io.*;
5353
import java.text.NumberFormat;
54+
import java.util.Iterator;
5455
import java.util.List;
5556
import java.util.Optional;
5657
import java.util.Properties;
@@ -71,11 +72,16 @@
7172
import org.exist.source.Source;
7273
import org.exist.source.StringSource;
7374
import org.exist.storage.DBBroker;
75+
import org.exist.xquery.functions.array.ArrayType;
76+
import org.exist.xquery.functions.map.MapType;
7477
import org.exist.xquery.parser.XQueryLexer;
7578
import org.exist.xquery.parser.XQueryParser;
7679
import org.exist.xquery.parser.XQueryTreeParser;
7780
import org.exist.xquery.util.ExpressionDumper;
81+
import org.exist.xquery.value.BinaryValue;
82+
import org.exist.xquery.value.Item;
7883
import org.exist.xquery.value.Sequence;
84+
import org.exist.xquery.value.SequenceIterator;
7985

8086
import javax.annotation.Nullable;
8187

@@ -425,6 +431,7 @@ public Sequence execute(final DBBroker broker, final CompiledXQuery expression,
425431
context.getProfiler().traceQueryStart();
426432
broker.getBrokerPool().getProcessMonitor().queryStarted(context.getWatchDog());
427433

434+
@Nullable Sequence result = null;
428435
FunctionCall call = null;
429436
try {
430437

@@ -435,7 +442,6 @@ public Sequence execute(final DBBroker broker, final CompiledXQuery expression,
435442
}
436443
}
437444

438-
final Sequence result;
439445
if (expression instanceof LibraryModuleRoot) {
440446
if (functionCall == null) {
441447
if (expression != null) {
@@ -480,11 +486,13 @@ public Sequence execute(final DBBroker broker, final CompiledXQuery expression,
480486
broker.getBrokerPool().getProcessMonitor().queryCompleted(context.getWatchDog());
481487
expression.reset();
482488

483-
if (call != null) {
489+
if(call != null) {
484490
call.reset();
485491
}
486492

487-
if(resetContext) {
493+
runCleanupTasks(context, result);
494+
495+
if (resetContext) {
488496
context.reset();
489497
}
490498
}
@@ -512,12 +520,6 @@ public Sequence execute(final DBBroker broker, final String expression, final Se
512520
final XQueryContext context = new XQueryContext(broker.getBrokerPool());
513521
final CompiledXQuery compiled = compile(context, expression);
514522
return execute(broker, compiled, contextSequence);
515-
// NOTE(AR) we might consider the below cleanup, but what if a binary value is needed from the result sequence?
516-
// try {
517-
// return execute(broker, compiled, contextSequence);
518-
// } finally {
519-
// context.runCleanupTasks();
520-
// }
521523
}
522524

523525
/**
@@ -536,11 +538,112 @@ public Sequence execute(final DBBroker broker, final File file, final Sequence c
536538
final XQueryContext context = new XQueryContext(broker.getBrokerPool());
537539
final CompiledXQuery compiled = compile(context, new FileSource(file.toPath(), true));
538540
return execute(broker, compiled, contextSequence);
539-
// NOTE(AR) we might consider the below cleanup, but what if a binary value is needed from the result sequence?
540-
// try {
541-
// return execute(broker, compiled, contextSequence);
542-
// } finally {
543-
// context.runCleanupTasks();
544-
// }
541+
}
542+
543+
/**
544+
* Runs cleanup tasks after query execution to free any resources that are no longer needed.
545+
*
546+
* @param xqueryContext the XQuery Context.
547+
* @param queryResult the result Sequence of the Query.
548+
*/
549+
private static void runCleanupTasks(final XQueryContext xqueryContext, @Nullable final Sequence queryResult) {
550+
if (queryResult == null) {
551+
// There are no results produced by the query, so we can cleanup everything
552+
xqueryContext.runCleanupTasks();
553+
554+
} else {
555+
// There are results produced, so we can only cleanup resources that don't appear in the result sequence
556+
cleanupOrphanedValues(xqueryContext, queryResult);
557+
}
558+
}
559+
560+
/**
561+
* Runs cleanup tasks that cleanup items that do not appear in the Query Result.
562+
* At present the only items that may need to be cleaned up are BinaryValue types.
563+
*
564+
* @param xqueryContext the XQuery Context.
565+
* @param queryResult the result Sequence of the Query.
566+
*/
567+
private static void cleanupOrphanedValues(final XQueryContext xqueryContext, final Sequence queryResult) {
568+
xqueryContext.runCleanupTasks(false, false, clazz -> clazz.equals(XQueryContext.BinaryValueCleanupTask.class), obj -> {
569+
if (obj instanceof BinaryValue) {
570+
final boolean hasReference = sequenceHasItemReference(queryResult, obj);
571+
return !hasReference;
572+
}
573+
574+
return true;
575+
});
576+
}
577+
578+
/**
579+
* Checks if a Sequence contains an Item.
580+
* The check is performed by reference equality.
581+
*
582+
* @param sequence the Sequence to inspect.
583+
* @param itemReference The item to look for by reference.
584+
*
585+
* @return true if the sequence contains the item, false otherwise.
586+
*/
587+
private static boolean sequenceHasItemReference(final Sequence sequence, final Object itemReference) {
588+
if (sequence instanceof MapType) {
589+
return itemHasItemReference((MapType) sequence, itemReference);
590+
}
591+
592+
if (sequence instanceof ArrayType) {
593+
return itemHasItemReference((ArrayType) sequence, itemReference);
594+
}
595+
596+
try {
597+
final SequenceIterator itItem = sequence.iterate();
598+
while (itItem.hasNext()) {
599+
final Item item = itItem.nextItem();
600+
if (itemHasItemReference(item, itemReference)) {
601+
return true;
602+
}
603+
}
604+
} catch (final XPathException e) {
605+
// no-op
606+
}
607+
608+
return false;
609+
}
610+
611+
/**
612+
* Checks if an Item matches an Item (by reference equality).
613+
* If the Item is a Map or Array, we also checks its members (recursively).
614+
*
615+
* @param item the Item to inspect.
616+
* @param itemReference The item to look for by reference.
617+
*
618+
* @return true if the Item matches or contains the Item reference, false otherwise.
619+
*/
620+
private static boolean itemHasItemReference(final Item item, final Object itemReference) {
621+
if (item == itemReference) {
622+
return true;
623+
}
624+
625+
if (item instanceof MapType) {
626+
final MapType mapType = (MapType) item;
627+
final Iterator<Sequence> itMapValues = mapType.valueIterator();
628+
while (itMapValues.hasNext()) {
629+
final Sequence mapValue = itMapValues.next();
630+
if (sequenceHasItemReference(mapValue, itemReference)) {
631+
return true;
632+
}
633+
}
634+
}
635+
636+
if (item instanceof ArrayType) {
637+
final ArrayType arrayType = (ArrayType) item;
638+
final Iterator<Sequence> itArrayValues = arrayType.iterator();
639+
while (itArrayValues.hasNext()) {
640+
final Sequence arrayValue = itArrayValues.next();
641+
if (sequenceHasItemReference(arrayValue, itemReference)) {
642+
return true;
643+
}
644+
}
645+
}
646+
647+
return false;
545648
}
546649
}

exist-core/src/main/java/org/exist/xquery/XQueryContext.java

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3471,7 +3471,7 @@ public void registerBinaryValueInstance(final BinaryValue binaryValue) {
34713471
}
34723472

34733473
/**
3474-
* Cleanup Task which is responsible for relasing the streams
3474+
* Cleanup Task which is responsible for releasing the streams
34753475
* of any {@link BinaryValue} which have been used during
34763476
* query execution
34773477
*/
@@ -3603,23 +3603,40 @@ public interface CleanupTask {
36033603

36043604
@Override
36053605
public void runCleanupTasks(final Predicate<Object> predicate) {
3606-
if (importedContextsCleanupTasksFns != null) {
3606+
runCleanupTasks(true, true, (clazz) -> true, predicate);
3607+
}
3608+
3609+
/**
3610+
* Run cleanup tasks.
3611+
*
3612+
* @param runImportedContextsCleanupTasks true if cleanup tasks for imported contexts should be run.
3613+
* @param clearCleanupTasks true if after cleanup tasks have run they should be cleared.
3614+
* @param cleanupTaskFilter a predicate to selectively choose which cleanup tasks are run.
3615+
* @param predicate a predicate to determine what objects should be cleaned up by the cleanup tasks.
3616+
*/
3617+
public void runCleanupTasks(final boolean runImportedContextsCleanupTasks, final boolean clearCleanupTasks, final Predicate<Class<? extends CleanupTask>> cleanupTaskFilter, final Predicate<Object> predicate) {
3618+
if (runImportedContextsCleanupTasks && importedContextsCleanupTasksFns != null) {
36073619
for (final Consumer<Predicate<Object>> importedContextsCleanupTasksFn : importedContextsCleanupTasksFns) {
36083620
importedContextsCleanupTasksFn.accept(predicate);
36093621
}
36103622
importedContextsCleanupTasksFns = null;
36113623
}
36123624

36133625
for (final CleanupTask cleanupTask : cleanupTasks) {
3614-
try {
3615-
cleanupTask.cleanup(this, predicate);
3616-
} catch (final Throwable t) {
3617-
LOG.error("Cleaning up XQueryContext: Ignoring: {}", t.getMessage(), t);
3626+
if (cleanupTaskFilter.test(cleanupTask.getClass())) {
3627+
try {
3628+
cleanupTask.cleanup(this, predicate);
3629+
} catch (final Throwable t) {
3630+
LOG.error("Cleaning up XQueryContext: Ignoring: {}", t.getMessage(), t);
3631+
}
36183632
}
36193633
}
3620-
// now it is safe to clear the cleanup tasks list as we know they have run
3621-
// do not move this anywhere else
3622-
cleanupTasks.clear();
3634+
3635+
if (clearCleanupTasks) {
3636+
// now it is safe to clear the cleanup tasks list as we know they have run
3637+
// do not move this anywhere else
3638+
cleanupTasks.clear();
3639+
}
36233640
}
36243641

36253642
@Immutable

exist-core/src/main/java/org/exist/xquery/functions/array/ArrayType.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
*
6565
* @author Wolf
6666
*/
67-
public class ArrayType extends FunctionReference implements Lookup.LookupSupport {
67+
public class ArrayType extends FunctionReference implements Iterable<Sequence>, Lookup.LookupSupport {
6868

6969
// the signature of the function which is evaluated if the map is called as a function item
7070
private static final FunctionSignature ACCESSOR =
@@ -266,6 +266,11 @@ public Sequence[] toArray() {
266266
return (Sequence[]) RT.seqToPassedArray(vector.seq(), array);
267267
}
268268

269+
@Override
270+
public Iterator<Sequence> iterator() {
271+
return new SeqIterator<>(vector.seq());
272+
}
273+
269274
public int getSize() {
270275
return vector.length();
271276
}

0 commit comments

Comments
 (0)