Skip to content

Commit bfa2ba3

Browse files
alanpaxtonalanpaxton
committed
[optimize] reader uses less granular locking
Rework EmbeddedXMLStreamReader to hoist locks to the top of its public methods. In particular a big call to `filter()` a large set of noses is much more efficient this way. One sample workload went from 61s to 47s.
1 parent a1d63ba commit bfa2ba3

File tree

3 files changed

+160
-90
lines changed

3 files changed

+160
-90
lines changed

‎exist-core/src/main/java/org/exist/stax/EmbeddedXMLStreamReader.java‎

Lines changed: 141 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
import org.exist.storage.DBBroker;
3030
import org.exist.storage.Signatures;
3131
import org.exist.storage.btree.Value;
32-
import org.exist.storage.dom.AbstractRawNodeIterator;
32+
import org.exist.storage.dom.ManualLockRawNodeIterator;
33+
import org.exist.storage.lock.ManagedLock;
3334
import org.exist.util.ByteConversion;
3435
import org.exist.util.XMLString;
3536
import org.exist.util.serializer.AttrList;
@@ -47,6 +48,7 @@
4748
import java.util.ArrayList;
4849
import java.util.Deque;
4950
import java.util.List;
51+
import java.util.concurrent.locks.ReentrantLock;
5052

5153
public class EmbeddedXMLStreamReader implements IEmbeddedXMLStreamReader, ExtendedXMLStreamReader {
5254

@@ -59,7 +61,7 @@ public class EmbeddedXMLStreamReader implements IEmbeddedXMLStreamReader, Extend
5961
// members which don't (generally) change!
6062
private DBBroker broker;
6163
private DocumentImpl document;
62-
private final AbstractRawNodeIterator iterator;
64+
private final ManualLockRawNodeIterator iterator;
6365
private boolean reportAttributes;
6466

6567
// mutable members which hold the current state of the stream
@@ -77,7 +79,7 @@ public class EmbeddedXMLStreamReader implements IEmbeddedXMLStreamReader, Extend
7779
private final XMLString text = new XMLString(256);
7880

7981

80-
public EmbeddedXMLStreamReader(final DBBroker broker, final DocumentImpl document, final AbstractRawNodeIterator iterator, @Nullable final NodeHandle origin, final boolean reportAttributes)
82+
public EmbeddedXMLStreamReader(final DBBroker broker, final DocumentImpl document, final ManualLockRawNodeIterator iterator, @Nullable final NodeHandle origin, final boolean reportAttributes)
8183
throws XMLStreamException {
8284
this.broker = broker;
8385
this.document = document;
@@ -88,23 +90,35 @@ public EmbeddedXMLStreamReader(final DBBroker broker, final DocumentImpl documen
8890

8991
@Override
9092
public void reposition(final DBBroker broker, final NodeHandle node, final boolean reportAttributes) throws IOException {
91-
this.broker = broker;
92-
// Seeking to a node with unknown address will reuse this reader, so do it before setting all
93-
// the fields otherwise they could get overwritten.
94-
iterator.seek(node);
95-
reset();
96-
this.current = null;
97-
this.previous = null;
98-
this.elementStack.clear();
99-
this.state = BEFORE;
100-
this.consumedState = false;
101-
this.reportAttributes = reportAttributes;
102-
this.document = node.getOwnerDocument();
103-
this.origin = node;
93+
try (final ManagedLock<ReentrantLock> dbLock = iterator.acquireReadLock()) {
94+
this.broker = broker;
95+
// Seeking to a node with unknown address will reuse this reader, so do it before setting all
96+
// the fields otherwise they could get overwritten.
97+
iterator.seek(node);
98+
reset();
99+
this.current = null;
100+
this.previous = null;
101+
this.elementStack.clear();
102+
this.state = BEFORE;
103+
this.consumedState = false;
104+
this.reportAttributes = reportAttributes;
105+
this.document = node.getOwnerDocument();
106+
this.origin = node;
107+
}
104108
}
105109

106110
@Override
107111
public boolean hasNext() throws XMLStreamException {
112+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
113+
return hasNextLocked();
114+
}
115+
}
116+
117+
/**
118+
* LP == iterator.ReadLock
119+
* @throws XMLStreamException
120+
*/
121+
private boolean hasNextLocked() throws XMLStreamException {
108122
if (consumedState || state == BEFORE) {
109123
getNext();
110124

@@ -121,13 +135,28 @@ public boolean hasNext() throws XMLStreamException {
121135

122136
@Override
123137
public int next() throws XMLStreamException {
124-
if (!hasNext()) {
138+
try (final ManagedLock<ReentrantLock> dbLock = iterator.acquireReadLock()) {
139+
return nextLocked();
140+
}
141+
}
142+
143+
/**
144+
* LP == iterator.ReadLock
145+
* @throws XMLStreamException
146+
*/
147+
private int nextLocked() throws XMLStreamException {
148+
if (!hasNextLocked()) {
125149
throw new IllegalStateException("hasNext()==false");
126150
}
127151
consumedState = true; // mark that we have consumed the current state
128152
return state;
129153
}
130154

155+
156+
/**
157+
* LP == iterator.ReadLock
158+
* @throws XMLStreamException
159+
*/
131160
private void getNext() throws XMLStreamException {
132161
if(state != END_ELEMENT) {
133162
previous = current;
@@ -165,10 +194,12 @@ private void getNext() throws XMLStreamException {
165194

166195
@Override
167196
public void filter(final StreamFilter filter) throws XMLStreamException {
168-
while (hasNext()) {
169-
next();
170-
if (!filter.accept(this)) {
171-
break;
197+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
198+
while (hasNextLocked()) {
199+
nextLocked();
200+
if (!filter.accept(this)) {
201+
break;
202+
}
172203
}
173204
}
174205
}
@@ -200,6 +231,10 @@ private void initNode() {
200231
readNodeId();
201232
}
202233

234+
/**
235+
* LP == iterator.ReadLock
236+
* @throws XMLStreamException
237+
*/
203238
private void skipAttributes() throws XMLStreamException {
204239
if(attributes == null) {
205240
// attributes were not yet read. skip them...
@@ -212,6 +247,10 @@ private void skipAttributes() throws XMLStreamException {
212247
}
213248
}
214249

250+
/**
251+
* LP == iterator.ReadLock
252+
* @throws XMLStreamException
253+
*/
215254
private void readAttributes() {
216255
if(attributes == null) {
217256
final ElementEvent parent = elementStack.peek();
@@ -352,14 +391,16 @@ public boolean isWhiteSpace() {
352391

353392
@Override
354393
public String getAttributeValue(final String namespaceURI, final String localName) {
355-
readAttributes();
356-
for(int i = 0; i < attributes.getLength(); i++) {
357-
final org.exist.dom.QName qn = attributes.getQName(i);
358-
if(qn.getNamespaceURI().equals(namespaceURI) && qn.getLocalPart().equals(localName)) {
359-
return attributes.getValue(i);
394+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
395+
readAttributes();
396+
for (int i = 0; i < attributes.getLength(); i++) {
397+
final org.exist.dom.QName qn = attributes.getQName(i);
398+
if (qn.getNamespaceURI().equals(namespaceURI) && qn.getLocalPart().equals(localName)) {
399+
return attributes.getValue(i);
400+
}
360401
}
402+
return null;
361403
}
362-
return null;
363404
}
364405

365406
@Override
@@ -370,99 +411,115 @@ public int getAttributeCount() {
370411

371412
@Override
372413
public javax.xml.namespace.QName getAttributeName(final int index) {
373-
if (state != START_ELEMENT) {
374-
throw new IllegalStateException("Cursor is not at an element");
375-
}
376-
readAttributes();
377-
if(index > attributes.getLength()) {
378-
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
414+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
415+
if (state != START_ELEMENT) {
416+
throw new IllegalStateException("Cursor is not at an element");
417+
}
418+
readAttributes();
419+
if (index > attributes.getLength()) {
420+
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
421+
}
422+
return attributes.getQName(index).toJavaQName();
379423
}
380-
return attributes.getQName(index).toJavaQName();
381424
}
382425

383426
@Override
384427
public org.exist.dom.QName getAttributeQName(final int index) {
385-
if (state != START_ELEMENT) {
386-
throw new IllegalStateException("Cursor is not at an element");
387-
}
388-
readAttributes();
389-
if(index > attributes.getLength()) {
390-
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
428+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
429+
if (state != START_ELEMENT) {
430+
throw new IllegalStateException("Cursor is not at an element");
431+
}
432+
readAttributes();
433+
if(index > attributes.getLength()) {
434+
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
435+
}
436+
return attributes.getQName(index);
391437
}
392-
return attributes.getQName(index);
393438
}
394439

395440
@Override
396441
public String getAttributeNamespace(final int index) {
397-
if (state != START_ELEMENT) {
398-
throw new IllegalStateException("Cursor is not at an element");
399-
}
400-
readAttributes();
401-
if(index > attributes.getLength()) {
402-
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
442+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
443+
if (state != START_ELEMENT) {
444+
throw new IllegalStateException("Cursor is not at an element");
445+
}
446+
readAttributes();
447+
if (index > attributes.getLength()) {
448+
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
449+
}
450+
return attributes.getQName(index).getNamespaceURI();
403451
}
404-
return attributes.getQName(index).getNamespaceURI();
405452
}
406453

407454
@Override
408455
public String getAttributeLocalName(final int index) {
409-
if (state != START_ELEMENT) {
410-
throw new IllegalStateException("Cursor is not at an element");
411-
}
412-
readAttributes();
413-
if(index > attributes.getLength()) {
414-
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
456+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
457+
if (state != START_ELEMENT) {
458+
throw new IllegalStateException("Cursor is not at an element");
459+
}
460+
readAttributes();
461+
if (index > attributes.getLength()) {
462+
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
463+
}
464+
return attributes.getQName(index).getLocalPart();
415465
}
416-
return attributes.getQName(index).getLocalPart();
417466
}
418467

419468
@Override
420469
public String getAttributePrefix(final int index) {
421-
if (state != START_ELEMENT) {
422-
throw new IllegalStateException("Cursor is not at an element");
423-
}
424-
readAttributes();
425-
if(index > attributes.getLength()) {
426-
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
470+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
471+
if (state != START_ELEMENT) {
472+
throw new IllegalStateException("Cursor is not at an element");
473+
}
474+
readAttributes();
475+
if (index > attributes.getLength()) {
476+
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
477+
}
478+
return attributes.getQName(index).getPrefix();
427479
}
428-
return attributes.getQName(index).getPrefix();
429480
}
430481

431482
@Override
432483
public String getAttributeType(final int index) {
433-
if (state != START_ELEMENT) {
434-
throw new IllegalStateException("Cursor is not at an element");
435-
}
436-
readAttributes();
437-
if(index > attributes.getLength()) {
438-
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
484+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
485+
if (state != START_ELEMENT) {
486+
throw new IllegalStateException("Cursor is not at an element");
487+
}
488+
readAttributes();
489+
if (index > attributes.getLength()) {
490+
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
491+
}
492+
final int type = attributes.getType(index);
493+
return AttrImpl.getAttributeType(type);
439494
}
440-
final int type = attributes.getType(index);
441-
return AttrImpl.getAttributeType(type);
442495
}
443496

444497
@Override
445498
public String getAttributeValue(final int index) {
446-
if (state != START_ELEMENT) {
447-
throw new IllegalStateException("Cursor is not at an element");
448-
}
449-
readAttributes();
450-
if(index > attributes.getLength()) {
451-
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
499+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
500+
if (state != START_ELEMENT) {
501+
throw new IllegalStateException("Cursor is not at an element");
502+
}
503+
readAttributes();
504+
if (index > attributes.getLength()) {
505+
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
506+
}
507+
return attributes.getValue(index);
452508
}
453-
return attributes.getValue(index);
454509
}
455510

456511
@Override
457512
public NodeId getAttributeId(final int index) {
458-
if (state != START_ELEMENT) {
459-
throw new IllegalStateException("Cursor is not at an element");
460-
}
461-
readAttributes();
462-
if(index > attributes.getLength()) {
463-
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
513+
try (final ManagedLock<ReentrantLock> domFileLock = iterator.acquireReadLock()) {
514+
if (state != START_ELEMENT) {
515+
throw new IllegalStateException("Cursor is not at an element");
516+
}
517+
readAttributes();
518+
if (index > attributes.getLength()) {
519+
throw new ArrayIndexOutOfBoundsException("index should be < " + attributes.getLength());
520+
}
521+
return attributes.getNodeId(index);
464522
}
465-
return attributes.getNodeId(index);
466523
}
467524

468525
@Override

‎exist-core/src/main/java/org/exist/storage/NativeBroker.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ public NativeValueIndex getValueIndex() {
620620
public IEmbeddedXMLStreamReader getXMLStreamReader(final NodeHandle node, final boolean reportAttributes)
621621
throws IOException, XMLStreamException {
622622
if(streamReader == null) {
623-
final AbstractRawNodeIterator iterator = new GranularRawNodeIterator(this, domDb, node);
623+
final ManualLockRawNodeIterator iterator = new ManualLockRawNodeIterator(this, domDb, node);
624624
streamReader = new EmbeddedXMLStreamReader(this, node.getOwnerDocument(), iterator, node, reportAttributes);
625625
} else {
626626
streamReader.reposition(this, node, reportAttributes);
@@ -631,7 +631,7 @@ public IEmbeddedXMLStreamReader getXMLStreamReader(final NodeHandle node, final
631631
@Override
632632
public IEmbeddedXMLStreamReader newXMLStreamReader(final NodeHandle node, final boolean reportAttributes)
633633
throws IOException, XMLStreamException {
634-
final GranularRawNodeIterator iterator = new GranularRawNodeIterator(this, domDb, node);
634+
final ManualLockRawNodeIterator iterator = new ManualLockRawNodeIterator(this, domDb, node);
635635
return new EmbeddedXMLStreamReader(this, node.getOwnerDocument(), iterator, null, reportAttributes);
636636
}
637637

0 commit comments

Comments
 (0)