Skip to content

Commit 7568d90

Browse files
adamretteradamretter
committed
[optimize] Dispose eagerly of resources that were accumulated during XQuery exeution but are no longer needed in the Result sequence
1 parent 0fb7d53 commit 7568d90

10 files changed

Lines changed: 531 additions & 85 deletions

File tree

‎exist-core/pom.xml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,6 +1141,7 @@
11411141
<include>src/test/java/org/exist/util/io/ByteArrayContentTest.java</include>
11421142
<include>src/main/java/org/exist/util/io/ContentFile.java</include>
11431143
<include>src/main/java/org/exist/util/io/ContentFilePoolObjectFactory.java</include>
1144+
<include>src/main/java/org/exist/util/io/FilterInputStreamCacheMonitor.java</include>
11441145
<include>src/test/java/org/exist/util/io/FilterInputStreamCacheMonitorTest.java</include>
11451146
<include>src/test/java/org/exist/util/io/OverflowToDiskStreamTest.java</include>
11461147
<include>src/main/java/org/exist/util/io/VirtualTempPath.java</include>
@@ -1968,6 +1969,7 @@
19681969
<exclude>src/test/java/org/exist/util/io/ByteArrayContentTest.java</exclude>
19691970
<exclude>src/main/java/org/exist/util/io/ContentFile.java</exclude>
19701971
<exclude>src/main/java/org/exist/util/io/ContentFilePoolObjectFactory.java</exclude>
1972+
<exclude>src/main/java/org/exist/util/io/FilterInputStreamCacheMonitor.java</exclude>
19711973
<exclude>src/test/java/org/exist/util/io/FilterInputStreamCacheMonitorTest.java</exclude>
19721974
<exclude>src/test/java/org/exist/util/io/OverflowToDiskStreamTest.java</exclude>
19731975
<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: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
import org.exist.util.serializer.SerializerPool;
8787
import org.exist.xquery.XPathException;
8888
import org.exist.xquery.value.AtomicValue;
89+
import org.exist.xquery.value.Item;
8990
import org.exist.xquery.value.NodeValue;
9091
import org.exist.xquery.value.StringValue;
9192
import org.exist.xquery.value.Type;
@@ -219,7 +220,25 @@ else if (root != null && !(root instanceof NodeValue)) {
219220
return content;
220221
}
221222
}
222-
223+
224+
/**
225+
* Allows access to the underlying XDM typed value.
226+
* Should be used sparingly!
227+
*
228+
* @return the Underlying Xdm value.
229+
*/
230+
public @Nullable Item getContentAsXdm() {
231+
if (root instanceof NodeValue) {
232+
return (NodeValue) root;
233+
}
234+
235+
if (value != null) {
236+
return value;
237+
}
238+
239+
return null;
240+
}
241+
223242
@Override
224243
public void getContentAsStream(OutputStream os) throws XMLDBException {
225244
try {

‎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.w3c.dom.Node;
7675
import org.xmldb.api.base.*;

‎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

@@ -431,6 +437,7 @@ public Sequence execute(final DBBroker broker, final CompiledXQuery expression,
431437
context.getProfiler().traceQueryStart();
432438
broker.getBrokerPool().getProcessMonitor().queryStarted(context.getWatchDog());
433439

440+
@Nullable Sequence result = null;
434441
FunctionCall call = null;
435442
try {
436443

@@ -441,7 +448,6 @@ public Sequence execute(final DBBroker broker, final CompiledXQuery expression,
441448
}
442449
}
443450

444-
final Sequence result;
445451
if (expression instanceof LibraryModuleRoot) {
446452
if (functionCall == null) {
447453
if (expression != null) {
@@ -486,11 +492,13 @@ public Sequence execute(final DBBroker broker, final CompiledXQuery expression,
486492
broker.getBrokerPool().getProcessMonitor().queryCompleted(context.getWatchDog());
487493
expression.reset();
488494

489-
if (call != null) {
495+
if(call != null) {
490496
call.reset();
491497
}
492498

493-
if(resetContext) {
499+
runCleanupTasks(context, result);
500+
501+
if (resetContext) {
494502
context.reset();
495503
}
496504
}
@@ -518,12 +526,6 @@ public Sequence execute(final DBBroker broker, final String expression, final Se
518526
final XQueryContext context = new XQueryContext(broker.getBrokerPool());
519527
final CompiledXQuery compiled = compile(context, expression);
520528
return execute(broker, compiled, contextSequence);
521-
// NOTE(AR) we might consider the below cleanup, but what if a binary value is needed from the result sequence?
522-
// try {
523-
// return execute(broker, compiled, contextSequence);
524-
// } finally {
525-
// context.runCleanupTasks();
526-
// }
527529
}
528530

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

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

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

34623462
/**
3463-
* Cleanup Task which is responsible for relasing the streams
3463+
* Cleanup Task which is responsible for releasing the streams
34643464
* of any {@link BinaryValue} which have been used during
34653465
* query execution
34663466
*/
@@ -3588,23 +3588,40 @@ public interface CleanupTask {
35883588

35893589
@Override
35903590
public void runCleanupTasks(final Predicate<Object> predicate) {
3591-
if (importedContextsCleanupTasksFns != null) {
3591+
runCleanupTasks(true, true, (clazz) -> true, predicate);
3592+
}
3593+
3594+
/**
3595+
* Run cleanup tasks.
3596+
*
3597+
* @param runImportedContextsCleanupTasks true if cleanup tasks for imported contexts should be run.
3598+
* @param clearCleanupTasks true if after cleanup tasks have run they should be cleared.
3599+
* @param cleanupTaskFilter a predicate to selectively choose which cleanup tasks are run.
3600+
* @param predicate a predicate to determine what objects should be cleaned up by the cleanup tasks.
3601+
*/
3602+
public void runCleanupTasks(final boolean runImportedContextsCleanupTasks, final boolean clearCleanupTasks, final Predicate<Class<? extends CleanupTask>> cleanupTaskFilter, final Predicate<Object> predicate) {
3603+
if (runImportedContextsCleanupTasks && importedContextsCleanupTasksFns != null) {
35923604
for (final Consumer<Predicate<Object>> importedContextsCleanupTasksFn : importedContextsCleanupTasksFns) {
35933605
importedContextsCleanupTasksFn.accept(predicate);
35943606
}
35953607
importedContextsCleanupTasksFns = null;
35963608
}
35973609

35983610
for (final CleanupTask cleanupTask : cleanupTasks) {
3599-
try {
3600-
cleanupTask.cleanup(this, predicate);
3601-
} catch (final Throwable t) {
3602-
LOG.error("Cleaning up XQueryContext: Ignoring: {}", t.getMessage(), t);
3611+
if (cleanupTaskFilter.test(cleanupTask.getClass())) {
3612+
try {
3613+
cleanupTask.cleanup(this, predicate);
3614+
} catch (final Throwable t) {
3615+
LOG.error("Cleaning up XQueryContext: Ignoring: {}", t.getMessage(), t);
3616+
}
36033617
}
36043618
}
3605-
// now it is safe to clear the cleanup tasks list as we know they have run
3606-
// do not move this anywhere else
3607-
cleanupTasks.clear();
3619+
3620+
if (clearCleanupTasks) {
3621+
// now it is safe to clear the cleanup tasks list as we know they have run
3622+
// do not move this anywhere else
3623+
cleanupTasks.clear();
3624+
}
36083625
}
36093626

36103627
@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 =
@@ -267,6 +267,11 @@ public Sequence[] toArray() {
267267
return (Sequence[]) RT.seqToPassedArray(vector.seq(), array);
268268
}
269269

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

0 commit comments

Comments
 (0)