You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by sl...@apache.org on 2020/09/15 12:35:28 UTC
[incubator-daffodil] branch master updated: Reduce performance
degradation caused by commit b0f59ef7c8
This is an automated email from the ASF dual-hosted git repository.
slawrence pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git
The following commit(s) were added to refs/heads/master by this push:
new f5b45a1 Reduce performance degradation caused by commit b0f59ef7c8
f5b45a1 is described below
commit f5b45a1a7bcea3a22f32401b4db21618edad07c8
Author: Steve Lawrence <sl...@apache.org>
AuthorDate: Wed Sep 9 11:43:21 2020 -0400
Reduce performance degradation caused by commit b0f59ef7c8
- Remove the DIFinalizable trait and setFinal function and replace with
direct modification of isFinal. Scala created some pretty inefficient
byte code for this which actually made a noticeable difference in
performance when removed.
- Add function to DINodes to efficiently return a Maybe of the last
child. This replaces the lastOption and avoids an Option allocation
- Remove the containerNode def and just keep track of the container node
in the InfosetWalker with a new stack. Avoids virtual function calls
and instanceOf/asInstanceOf.
- Skip batches of walk() calls. This avoids the overhead related to step
logic when no progress is being made due to PoU's and allows batching
of steps rather than stepping each time a infoset node is added.
- Replace the various infoset walker objects with functions. Rather than
returning objects in a Maybe and calling a function in those optionst,
just call the functions directly and return a boolean that says
whether a step was taken. Essentially the same logic, but avoids
object creation and virtual functions.
- Avoid accessing the top of the infoset walker stacks multiple times.
Instead, access once and pass down as parameters. These stack accesses
actually showed up quite a bit in profiling
- Remove match/case on types, and instead use functions to imply the
type. Avoids instanceOf/asInstanceOf which have a noticeable
overhead in inner loops
- Add a new setTop function to modify the top of an MStack. Avoids logic
to pop and then push a new value.
- Remove extra logic about saved last child node, this didn't make much
of a difference in when we called set final, and just added extra
overhead
- Remove calls to walk() made when PoU's were resolved. Instead, only
walk() when new elements are added. Elements are added all the time so
these extra calls just added overhead. The new step skipping logic
also helps with this.
DAFFODIL-2396
---
.../scala/org/apache/daffodil/util/MStack.scala | 18 ++
.../processors/unparsers/ElementUnparser.scala | 4 +-
.../daffodil/debugger/InteractiveDebugger.scala | 4 +-
.../org/apache/daffodil/dpath/FNFunctions.scala | 4 +-
.../org/apache/daffodil/infoset/InfosetImpl.scala | 139 ++++++---
.../apache/daffodil/infoset/InfosetWalker.scala | 338 ++++++++++++---------
.../apache/daffodil/processors/DataProcessor.scala | 8 +-
.../processors/parsers/ElementKindParsers.scala | 16 +-
.../parsers/ExpressionEvaluatingParsers.scala | 1 -
.../parsers/NilEmptyCombinatorParsers.scala | 1 -
.../daffodil/processors/parsers/Parser.scala | 15 +-
.../processors/parsers/SequenceParserBases.scala | 53 ++--
12 files changed, 343 insertions(+), 258 deletions(-)
diff --git a/daffodil-lib/src/main/scala/org/apache/daffodil/util/MStack.scala b/daffodil-lib/src/main/scala/org/apache/daffodil/util/MStack.scala
index a0899da..bb49e64 100644
--- a/daffodil-lib/src/main/scala/org/apache/daffodil/util/MStack.scala
+++ b/daffodil-lib/src/main/scala/org/apache/daffodil/util/MStack.scala
@@ -95,6 +95,11 @@ final class MStackOfMaybe[T <: AnyRef] {
else One(m)
}
+ @inline final def setTop(m: Maybe[T]) = {
+ if (m.isDefined) delegate.setTop(m.get)
+ else delegate.setTop(nullT)
+ }
+
@inline final def top: Maybe[T] = {
val m = delegate.top
if (m eq null) Nope
@@ -144,6 +149,7 @@ final class MStackOf[T <: AnyRef]
@inline final def push(t: T) = delegate.push(t)
@inline final def pop: T = delegate.pop.asInstanceOf[T]
+ @inline final def setTop(t: T) = delegate.setTop(t)
@inline final def top: T = delegate.top.asInstanceOf[T]
@inline final def bottom: T = delegate.bottom.asInstanceOf[T]
@inline final def isEmpty = delegate.isEmpty
@@ -262,6 +268,18 @@ protected abstract class MStack[@specialized T] private[util] (
}
/**
+ * Change the value on top value of a stack. In some cases, this can be more
+ * performant than popping the top and then pushing a new value
+ *
+ * @param x The element to set to the top of the stack
+ */
+ @inline final def setTop(x: T): Unit = {
+ if (index == 0) Assert.usageError("Stack empty")
+ table(index - 1) = x
+ }
+
+
+ /**
* View the top element of the stack.
*
* Does not remove the element on the top. If the stack is empty,
diff --git a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementUnparser.scala b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementUnparser.scala
index 9c46b82..12f3117 100644
--- a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementUnparser.scala
+++ b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementUnparser.scala
@@ -432,7 +432,7 @@ sealed trait RegularElementUnparserStartEndStrategy
} else {
val doc = state.documentElement
doc.addChild(res, state.tunable) // DIDocument, which is never a current node, must have the child added
- doc.setFinal() // that's the only child.
+ doc.isFinal = true // that's the only child.
}
res
} else {
@@ -477,7 +477,7 @@ sealed trait RegularElementUnparserStartEndStrategy
}
val cur = state.currentInfosetNode
if (cur.isComplex)
- cur.asComplex.setFinal()
+ cur.isFinal = true
state.currentInfosetNodeStack.pop
move(state)
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
index 36789e9..b05b62d 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
@@ -451,12 +451,12 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
val bos = new java.io.ByteArrayOutputStream()
val xml = new XMLTextInfosetOutputter(bos, true)
val iw = InfosetWalker(
- ie.asInstanceOf[DINode],
+ ie.asInstanceOf[DIElement],
xml,
walkHidden = !DebuggerConfig.removeHidden,
ignoreBlocks = true,
removeUnneeded = false)
- iw.walk()
+ iw.walk(lastWalk = true)
bos.toString("UTF-8")
}
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala
index 8eba242..2fc7c46 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala
@@ -38,7 +38,7 @@ import org.apache.daffodil.exceptions.Assert
import org.apache.daffodil.exceptions.SchemaFileLocation
import org.apache.daffodil.exceptions.UnsuppressableException
import org.apache.daffodil.infoset.DIElement
-import org.apache.daffodil.infoset.DIFinalizable
+import org.apache.daffodil.infoset.DINode
import org.apache.daffodil.infoset.DataValue.DataValueBigDecimal
import org.apache.daffodil.infoset.DataValue.DataValueBigInt
import org.apache.daffodil.infoset.DataValue.DataValueBool
@@ -564,7 +564,7 @@ trait ExistsKind {
}
case UnparserBlocking => {
dstate.currentNode match {
- case c: DIFinalizable if (c.isFinal) => false
+ case c: DINode if (c.isFinal) => false
case _ => throw th
}
}
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetImpl.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetImpl.scala
index 648bcc8..70ad1ca 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetImpl.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetImpl.scala
@@ -65,7 +65,7 @@ import org.apache.daffodil.xml.XMLUtils
import passera.unsigned.ULong
import org.apache.daffodil.dsom.DPathCompileInfo
-sealed trait DINode extends DIFinalizable {
+sealed trait DINode {
def asSimple: DISimple = {
this match {
@@ -87,6 +87,16 @@ sealed trait DINode extends DIFinalizable {
}
def isComplex: Boolean
+ def asArray: DIArray = {
+ this match {
+ case diArray: DIArray => diArray
+ case _ => {
+ erd.toss(new InfosetWrongNodeType("arrayType", this)) // see comment with exception class definition for why this can happen
+ }
+ }
+ }
+ def isArray: Boolean
+
def asElement: DIElement =
if (isSimple) asSimple
else asComplex
@@ -109,18 +119,15 @@ sealed trait DINode extends DIFinalizable {
final def numChildren = contents.length
/**
- * Returns the DINode that contains this DINode. Note that this behavior is
- * slightly different from the "parent". The parent method always returns a
- * DIComplex, even if this DINode is in an array. This usually makes sense
- * and is what the caller intended when they call parent.
- *
- * However, we sometimes want the exact DINode that contains this DINode. So
- * if this DINode is a DIElement in an array, then this returns the DIArray.
- * Otherwise (i.e. DIArray or non-array DIElement) then it should return the
- * DIComplex parent. Note that if the node is a DIDocument it should return
- * null, which mimics the behavior of .parent.
+ * If there are any children, returns the last child as a One(lastChild).
+ * Otherwise returns Nope. Note that because Daffodil may set children to
+ * null when it determines they are no longer needed, and One(null) is not
+ * allowed, this must return Nope if the last child has been freed. Thus,
+ * this is not a reliable mechanism to determine if children exist. It should
+ * only be used where we need to modify the state (e.g. isFinal) of the last
+ * child IF it exists.
*/
- def containerNode: DINode
+ def maybeLastChild: Maybe[DINode]
/**
* Free memory associated with the child at a specified index. Note that this
@@ -129,6 +136,22 @@ sealed trait DINode extends DIFinalizable {
* child is used in DPath expressions this should not free the child.
*/
def freeChildIfNoLongerNeeded(index: Int): Unit
+
+ /**
+ * When unparsing, arrays and complex elements have a notion of being finalized.
+ * This is when we know that no more elements will be added to them, so things
+ * like fn:count can return a value knowing it won't change, and fn:exists can
+ * return false, knowing nobody will subsequently append the item that was being
+ * questioned.
+ */
+ var isFinal: Boolean = false
+
+ /**
+ * use to require it be finalized or throw the appropriate
+ * Array or Complex exception.
+ */
+ def requireFinal: Unit
+
}
/**
@@ -829,6 +852,7 @@ sealed trait DIElement
def isSimple: Boolean
def isComplex: Boolean
+ def isArray: Boolean
override final def name: String = erd.name
override final def namespace: NS = erd.targetNamespace
override final def namedQName = erd.namedQName
@@ -911,10 +935,6 @@ sealed trait DIElement
override def parent = _parent
- override def containerNode =
- if (array.isDefined) array.get.asInstanceOf[DINode]
- else parent.asInstanceOf[DINode]
-
def diParent = _parent.asInstanceOf[DIComplex]
override def setParent(p: InfosetComplexElement): Unit = {
@@ -984,8 +1004,6 @@ final class DIArray(
private lazy val nfe = new InfosetArrayNotFinalException(this)
- override def containerNode = parent
-
override def requireFinal: Unit = {
if (!isFinal) {
// If this DIArray isn't final, either we haven't gotten all of its
@@ -999,7 +1017,7 @@ final class DIArray(
// isn't final then we could still be wainting for events to come in, so
// throw an nfe.
if (parent.isFinal) {
- setFinal()
+ isFinal = true
} else {
throw nfe
}
@@ -1008,6 +1026,7 @@ final class DIArray(
override def isSimple = false
override def isComplex = false
+ override def isArray = true
// Parsers don't actually call setHidden on DIArrays, only on DIElements. But
// DIArrays are always created when there is at least one child element, and
@@ -1057,6 +1076,17 @@ final class DIArray(
override def contents: IndexedSeq[DINode] = _contents
+ override def maybeLastChild: Maybe[DINode] = {
+ val len = _contents.length
+ if (len > 0) {
+ // note that if _contents(len - 1) has been removed (i.e. set to null)
+ // then this will by Maybe(null) which is a Nope. This is expected
+ // behavior
+ Maybe(_contents(len - 1))
+ }
+ else Nope
+ }
+
/**
* Note that occursIndex argument starts at position 1.
*/
@@ -1129,6 +1159,7 @@ sealed class DISimple(override val erd: ElementRuntimeData)
final override def isSimple = true
final override def isComplex = false
+ final override def isArray = false
def contents: IndexedSeq[DINode] = IndexedSeq.empty
@@ -1137,6 +1168,8 @@ sealed class DISimple(override val erd: ElementRuntimeData)
override def children: Stream[DINode] = Stream.Empty
+ override def maybeLastChild: Maybe[DINode] = Nope
+
def setDefaultedDataValue(defaultedValue: DataValuePrimitive) = {
setDataValue(defaultedValue)
_isDefaulted = true
@@ -1365,8 +1398,8 @@ sealed class DISimple(override val erd: ElementRuntimeData)
/**
* requireFinal is only ever used on unparse, and we never need to require a
* simple type to be final during unparse. However, we do need to have an
- * implementation of this function, as DISimple needs DIFinalizable and
- * isFinal for parsing and allowing the cleanup of unneeded DINodes.
+ * implementation of this function, as DINode requires isFinal for parsing
+ * and allowing the cleanup of unneeded DINodes.
*/
override def requireFinal: Unit = {
Assert.invariantFailed("Should not requireFinal a simple type")
@@ -1377,25 +1410,6 @@ sealed class DISimple(override val erd: ElementRuntimeData)
}
}
-/**
- * When unparsing, arrays and complex elements have a notion of being finalized.
- * This is when we know that no more elements will be added to them, so things
- * like fn:count can return a value knowing it won't change, and fn:exists can
- * return false, knowing nobody will subsequently append the item that was being
- * questioned.
- */
-sealed trait DIFinalizable {
- private var _isFinal: Boolean = false
-
- def setFinal(): Unit = { _isFinal = true }
- def isFinal = _isFinal
-
- /**
- * use to require it be finalized or throw the appropriate
- * Array or Complex exception.
- */
- def requireFinal: Unit
-}
/**
* Complex elements have an array of DINodes. Nodes may either be DISimple for
@@ -1415,6 +1429,7 @@ sealed class DIComplex(override val erd: ElementRuntimeData)
final override def isSimple = false
final override def isComplex = true
+ final override def isArray = false
final override def isDefaulted: Boolean = {
val res =
@@ -1458,6 +1473,17 @@ sealed class DIComplex(override val erd: ElementRuntimeData)
override def children = childNodes.toStream
+ override def maybeLastChild: Maybe[DINode] = {
+ val len = childNodes.length
+ if (len > 0) {
+ // note that if childNodes(len - 1) has been removed (i.e. set to null)
+ // then this will by Maybe(null) which is a Nope. This is expected
+ // behavior
+ Maybe(childNodes(len - 1))
+ }
+ else Nope
+ }
+
var hasVisibleChildren = false
final def getChild(erd: ElementRuntimeData, tunable: DaffodilTunables): InfosetElement = {
@@ -1564,13 +1590,32 @@ sealed class DIComplex(override val erd: ElementRuntimeData)
if (!e.isHidden && !hasVisibleChildren) hasVisibleChildren = true
if (e.runtimeData.isArray) {
val childERD = e.runtimeData
- if (childNodes.lastOption.map { n => n == null || n.erd != childERD }.getOrElse(true)) {
- // This complex element either has no children, the most recently added
- // child is different than the new child, or the last child is null
- // (which means it was freed because it was finished, and thus must be
- // different from this new element). This element is an array, so in
- // any of these cases, we must create a new DIArray, and then we'll add
- // the new element to it.
+ val needsNewArray =
+ if (childNodes.length == 0) {
+ // This complex element has no children, so we must need to create a
+ // new DIArray to add this array element
+ true
+ } else {
+ val lastChild = childNodes(childNodes.length - 1)
+ if (lastChild == null) {
+ // The last child has been freed by Daffodil. That can only happen
+ // if the array was finished so this new child cannot be part of
+ // that array. We must create a new array
+ true
+ } else if (lastChild.erd ne childERD) {
+ // The last child exists, but it is different than the new child.
+ // We must create a new array.
+ true
+ } else {
+ // The last child exists, and it has the same erd as this one. We
+ // can just append this new child to that last child array. No need
+ // to create a new one
+ false
+ }
+ }
+
+ if (needsNewArray){
+ // create a new array, then we will add the new child to that
val ia = new DIArray(childERD, this, tunable.initialElementOccurrencesHint.toInt)
addChildToFastLookup(ia)
childNodes += ia
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetWalker.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetWalker.scala
index bac3327..7fd6d1d 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetWalker.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetWalker.scala
@@ -19,9 +19,7 @@ package org.apache.daffodil.infoset
import org.apache.daffodil.exceptions.Assert
import org.apache.daffodil.util.MStackOfInt
-import org.apache.daffodil.util.Maybe
-import org.apache.daffodil.util.Maybe.One
-import org.apache.daffodil.util.Maybe.Nope
+import org.apache.daffodil.util.MStackOf
object InfosetWalker {
@@ -60,7 +58,7 @@ object InfosetWalker {
* except while debugging
*/
def apply(
- root: DINode,
+ root: DIElement,
outputter: InfosetOutputter,
walkHidden: Boolean,
ignoreBlocks: Boolean,
@@ -78,7 +76,10 @@ object InfosetWalker {
// walking at a node that isn't the root DIDocument. This gets the
// container of the root node to start at and finds the index in that
// container
- (root.containerNode, root.containerNode.contents.indexOf(root))
+ val container: DINode =
+ if (root.array.isDefined) root.array.get.asInstanceOf[DINode]
+ else root.parent.asInstanceOf[DINode]
+ (container, container.contents.indexOf(root))
}
}
new InfosetWalker(
@@ -178,7 +179,11 @@ class InfosetWalker private (
* we increment this value on the top of the stack so that the starting index
* is correct.
*/
- private var containerNode: DINode = startingContainerNode
+ private var containerNodeStack: MStackOf[DINode] = {
+ val stack = new MStackOf[DINode]()
+ stack.push(startingContainerNode)
+ stack
+ }
private var containerIndexStack: MStackOfInt = {
val stack = MStackOfInt()
stack.push(startingContainerIndex - 1)
@@ -193,6 +198,29 @@ class InfosetWalker private (
def isFinished = finished
/**
+ * The following variables are used to determine when to skip the walk()
+ * logic. The idea here is that calls to walk() result in a decrease in
+ * performance in some situations, especially if we can't take steps due to
+ * points of uncertainty or infoset blocks. In such cases, we perform a lot
+ * of logic without actually making any progress. And the callers don't know
+ * if they can take a step or not since that logic is controlled by this
+ * walker.
+ *
+ * So what we do is we first skip walkSkipMin number of calls to walk(). Once
+ * we have skipped that many walk()'s, only then do we actually try to take a
+ * step. If that step fails, that means there is still some block, so we
+ * double the number skips and return. We continue this doubling up until
+ * walkSkipMax. However, if we are able to successfully make a step, it means
+ * any points of uncertainty have been resolved, and so we drop the number of
+ * steps back down to the min value so that we try taking steps and stream
+ * events more frequently.
+ */
+ private val walkSkipMin = 32
+ private val walkSkipMax = 2048
+ private var walkSkipSize = walkSkipMin
+ private var walkSkipRemaining = walkSkipSize
+
+ /**
* Take zero or more steps in the infoset. This may or may not walk the
* entire infoset. If isFinished returns false, walk can be called again to
* continue attempting to walk the infoset where it left off. Because this is
@@ -201,26 +229,60 @@ class InfosetWalker private (
* possible.
*
* It is an error to call walk() if isFinished returns true
+ *
+ * For performance reasons, sometimes a call to walk will intentionally not
+ * try to take any steps even if it is possible. This lets us essentially
+ * batch infoset events and improve performance. If this is the last time
+ * walk() will be called, the lastWalk parameter should be set to true, which
+ * will cause walk() to not skip any steps.
*/
- def walk(): Unit = {
+ def walk(lastWalk: Boolean = false): Unit = {
Assert.usage(!finished)
- var maybeNextStep: Maybe[InfosetWalkerStep] = Nope
- while ({
- maybeNextStep = getNextStep()
- maybeNextStep.isDefined
- }) {
- maybeNextStep.get.step()
+ if (walkSkipRemaining > 0 && !lastWalk) {
+ // do not attempt to take a step
+ walkSkipRemaining -= 1
+ } else {
+ // no more skips or this is the last walk, so try to take a step
+ var canTakeAnotherStep = maybeDoStep()
+
+ walkSkipSize =
+ if (canTakeAnotherStep) {
+ // we've skipped walkSkipSize calls to walk() and we were able to
+ // make progress taking a step. Let's set the skip size to skip min
+ // to ensure we try to take steps in future calls to walk() more
+ // frequently
+ walkSkipMin
+ } else {
+ // we've skipped skip size calls to walk() and still failed to take
+ // a single step. Most likely we have an early unresolved point of
+ // uncertainty, so double the walkSkipSize to give more time for that
+ // PoU to get resolved before actually trying to take more steps
+ Math.min(walkSkipSize << 2, walkSkipMax)
+ }
+
+ // set the remaining number of skips to the adjusted size so the next
+ // time walk is called we start skipping again
+ walkSkipRemaining = walkSkipSize
+
+ // continue taking steps as far as we can
+ while (canTakeAnotherStep) {
+ canTakeAnotherStep = maybeDoStep()
+ }
+
}
}
/**
- * Determine the next step to take, if any
+ * Take a step if possible. Returns true if it is possible another step could
+ * be take, false if there was a block or something that would prevent
+ * another step from being taken.
*/
- private def getNextStep(): Maybe[InfosetWalkerStep] = {
- if (finished) {
- Nope
- } else if ((containerNode ne startingContainerNode) || containerIndexStack.top == startingContainerIndex) {
+ private def maybeDoStep(): Boolean = {
+ val containerNode = containerNodeStack.top
+ val containerIndex = containerIndexStack.top
+
+ if ((containerNode ne startingContainerNode) || (containerIndex == startingContainerIndex)) {
// The containerNode is either some child of the starting node (container
// node != starting container code) or we are exactly on the starting
// node (container node == starting container node && top of index stack
@@ -231,10 +293,14 @@ class InfosetWalker private (
// other kinds of blocks could exist. So we need to inspect the infoset
// state to determine if we can actually create events for the current
// infoset node and take a step.
- if (ignoreBlocks || canTakeStep()) {
- One(InfosetWalkerStepMove)
+ if (ignoreBlocks || canTakeStep(containerNode, containerIndex)) {
+ infosetWalkerStepMove(containerNode, containerIndex)
+ // took a step, more steps are alloed
+ true
} else {
- Nope
+ // a block prevented us from taking a step, no more steps can be taken
+ // during this walk
+ false
}
} else {
// We only get here if the container node is the starting container node,
@@ -252,16 +318,20 @@ class InfosetWalker private (
// index because we have moved passed the starting element index, and
// thus the next step is an end step. The InfosetWalkerStepEnd is
// responsible for creating the endDocument event and cleaning up state.
- if (containerIndexStack.top < startingContainerIndex) {
- One(InfosetWalkerStepStart)
+ if (containerIndex < startingContainerIndex) {
+ infosetWalkerStepStart()
+ // took the start document step, more steps are allowed
+ true
} else {
- One(InfosetWalkerStepEnd)
+ infosetWalkerStepEnd()
+ // we successfully took a step, but there are no more steps to take
+ // after this
+ false
}
}
}
- private def canTakeStep(): Boolean = {
-
+ private def canTakeStep(containerNode: DINode, containerIndex: Int): Boolean = {
if (containerNode.infosetWalkerBlockCount > 0) {
// This happens in two cases:
//
@@ -284,19 +354,18 @@ class InfosetWalker private (
// element and the child index of this container
val children = containerNode.contents
- val childIndex = containerIndexStack.top
- if (childIndex < children.size) {
+ if (containerIndex < children.length) {
// There is a child element at this index. Taking a step would create
// the events for, and moved passed, this element.
- val elem = children(childIndex)
+ val elem = children(containerIndex)
if (elem.infosetWalkerBlockCount > 0) {
// This element has a block associated with it, likely meaning we are
// speculatively parsing this element and so it may or may not exist.
// We cannot walk into it until we figure it out and the block is
// removed.
false
- } else if (elem.isInstanceOf[DISimple] && !elem.isFinal) {
+ } else if (elem.isSimple && !elem.isFinal) {
// This is a simple element that is not final, i.e. we are still
// doing some parsing for this simple element. Wait until we finish
// that parse before stepping into it.
@@ -321,132 +390,117 @@ class InfosetWalker private (
}
}
- private trait InfosetWalkerStep {
- /**
- * Output events associated with this step kind, and mutate the
- * InfosetWalker state to walk to the next node in the infoset
- */
- def step(): Unit
-
- final def moveToFirstChild(newContainer: DINode): Unit = {
- containerNode = newContainer
- containerIndexStack.push(0)
- }
+ @inline
+ private def moveToFirstChild(newContainer: DINode): Unit = {
+ containerNodeStack.push(newContainer)
+ containerIndexStack.push(0)
+ }
- final def moveToContainer(): Unit = {
- containerNode = containerNode.containerNode
- containerIndexStack.pop
- }
+ @inline
+ private def moveToContainer(): Unit = {
+ containerNodeStack.pop
+ containerIndexStack.pop
+ }
- final def moveToNextSibling(): Unit = {
- containerIndexStack.push(containerIndexStack.pop + 1)
- }
+ @inline
+ private def moveToNextSibling(): Unit = {
+ val top = containerIndexStack.top
+ containerIndexStack.setTop(top + 1)
}
- private object InfosetWalkerStepStart extends InfosetWalkerStep {
- /**
- * Start the document. Note that because the top of container index is
- * initialized to one less that the starting index, we also call
- * moveToNextSibling to increment the starting index to the correct
- * position.
- */
- override def step(): Unit = {
- outputter.startDocument()
- moveToNextSibling()
- }
+ /**
+ * Start the document. Note that because the top of container index is
+ * initialized to one less that the starting index, we also call
+ * moveToNextSibling to increment the starting index to the correct
+ * position.
+ */
+ private def infosetWalkerStepStart(): Unit = {
+ outputter.startDocument()
+ moveToNextSibling()
}
- private object InfosetWalkerStepEnd extends InfosetWalkerStep {
- /**
- * End document and clean up state. Setting finished to true causes
- * the next step to be None, walk() will return, and the caller
- * should not call walk() again because it is finished.
- */
- override def step(): Unit = {
- outputter.endDocument()
- containerNode = null
- containerIndexStack = null
- finished = true
- }
+ /**
+ * End document and clean up state. Setting finished to true causes
+ * the next step to be None, walk() will return, and the caller
+ * should not call walk() again because it is finished.
+ */
+ private def infosetWalkerStepEnd(): Unit = {
+ outputter.endDocument()
+ containerNodeStack = null
+ containerIndexStack = null
+ finished = true
}
- private object InfosetWalkerStepMove extends InfosetWalkerStep {
- /**
- * Output start/end events for DIComplex/DIArray/DISimple, and mutate state
- * so we are looking at the next node in the infoset.
- */
- def step(): Unit = {
- val children = containerNode.contents
- val childIndex = containerIndexStack.top
-
- if (childIndex < children.size) {
- // This block means we need to create a start event for the element in
- // the children array at childIndex. We then need to mutate the walker
- // state so the next call to step() is for either the first child of
- // this element or the next sibling.
-
- children(childIndex) match {
- case s: DISimple => {
- if (!s.isHidden || walkHidden) {
- outputter.startSimple(s)
- outputter.endSimple(s)
- }
- if (removeUnneeded) {
- // now we can remove this simple element to free up memory
- containerNode.freeChildIfNoLongerNeeded(containerIndexStack.top)
- }
- moveToNextSibling()
- }
- case c: DIComplex => {
- if (!c.isHidden || walkHidden) {
- outputter.startComplex(c)
- moveToFirstChild(c)
- } else {
- moveToNextSibling()
- }
- }
- case a: DIArray => {
- if (!a.isHidden || walkHidden) {
- outputter.startArray(a)
- moveToFirstChild(a)
- } else {
- moveToNextSibling()
- }
- }
- }
+ /**
+ * Output start/end events for DIComplex/DIArray/DISimple, and mutate state
+ * so we are looking at the next node in the infoset.
+ */
+ private def infosetWalkerStepMove(containerNode: DINode, containerIndex: Int): Unit = {
+ val children = containerNode.contents
- } else {
- // This else block means that we incremented the index stack past the
- // number of children in this container (must be a DIComplex/DIDocument
- // or DIArray), which means we have created events for all if its
- // children. So we must now create the appropriate end event for the
- // container and then mutate the state so that we are looking at its
- // next sibling. Note that if there is no next sibling, that will be
- // found the next time step() is called (because we incremented past
- // this element) and we will fall into this block again, call the end
- // function again, and repeat the process.
-
- // create appropriate end event
- containerNode match {
- case a: DIArray => {
- outputter.endArray(a)
- }
- case c: DIComplex => {
- outputter.endComplex(c)
- }
- case _ => Assert.impossible()
- }
+ if (containerIndex < children.size) {
+ // This block means we need to create a start event for the element in
+ // the children array at containerIndex. Once we create that event we
+ // need to mutate the state of the InfosetWalker so that the next time we
+ // take a step we are looking at the next element. If this is a complex
+ // type, that next element is the first child. If this is a simple type,
+ // that next element is the next sibling of this element. We will mutate
+ // the state accordingly.
- // we've ended this array/complex associated with the container, so we
- // now want to move to the parent container, potentially free up the
- // memory associated with this container, and then move to the next
- // sibling of this container
- moveToContainer()
+ val child = children(containerIndex)
+
+ if (child.isSimple) {
+ if (!child.isHidden || walkHidden) {
+ val simple = child.asInstanceOf[DISimple]
+ outputter.startSimple(simple)
+ outputter.endSimple(simple)
+ }
if (removeUnneeded) {
- containerNode.freeChildIfNoLongerNeeded(containerIndexStack.top)
+ // now we can remove this simple element to free up memory
+ containerNode.freeChildIfNoLongerNeeded(containerIndex)
}
moveToNextSibling()
+ } else {
+ // must be complex or array, exact same logic for both
+ if (!child.isHidden || walkHidden) {
+ if (child.isComplex) {
+ outputter.startComplex(child.asInstanceOf[DIComplex])
+ } else {
+ outputter.startArray(child.asInstanceOf[DIArray])
+ }
+ moveToFirstChild(child)
+ } else {
+ moveToNextSibling()
+ }
}
+
+ } else {
+ // This else block means that we incremented the index stack past the
+ // number of children in this container (must be a DIComplex/DIDocument
+ // or DIArray), which means we have created events for all if its
+ // children. So we must now create the appropriate end event for the
+ // container and then mutate the state so that we are looking at its
+ // next sibling. Note that if there is no next sibling, that will be
+ // found the next time step() is called (because we incremented past
+ // this element) and we will fall into this block again, call the end
+ // function again, and repeat the process.
+
+ // create appropriate end event
+ if (containerNode.isComplex) {
+ outputter.endComplex(containerNode.asInstanceOf[DIComplex])
+ } else {
+ outputter.endArray(containerNode.asInstanceOf[DIArray])
+ }
+
+ // we've ended this array/complex associated with the container, so we
+ // now want to move to the parent container, potentially free up the
+ // memory associated with this container, and then move to the next
+ // sibling of this container
+ moveToContainer()
+ if (removeUnneeded) {
+ containerNodeStack.top.freeChildIfNoLongerNeeded(containerIndexStack.top)
+ }
+ moveToNextSibling()
}
}
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
index 316aaf8..0522f2d 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
@@ -424,14 +424,14 @@ class DataProcessor private (
val pr = new ParseResult(this, state)
if (!pr.isProcessingError) {
- // By the time we get here, all infoset nodes have been setFinal, all
+ // By the time we get here, all infoset nodes have been set final, all
// walker blocks released, and all elements walked. The one exception
- // is that the root node has not been set final because setFinal is
+ // is that the root node has not been set final because isFinal is
// handled by the sequence parser and there is no sequence around the
// root node. So mark it as final and do one last walk to end the
// document.
- state.infoset.contents(0).setFinal()
- state.walker.walk()
+ state.infoset.contents(0).isFinal = true
+ state.walker.walk(lastWalk = true)
Assert.invariant(state.walker.isFinished)
if (maybeValidationBytes.isDefined) {
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala
index 1e34290..7acc87e 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala
@@ -219,8 +219,6 @@ abstract class ChoiceDispatchCombinatorParserBase(rd: TermRuntimeData,
pstate.resetToPointOfUncertainty(pou)
}
- val savedLastChildNode = pstate.infoset.contents.lastOption
-
// Note that we are intentionally not pushing/popping a new
// discriminator here, as is done in the ChoiceCombinatorParser and
// AltCompParser. This has the effect that if a branch of this direct
@@ -240,16 +238,9 @@ abstract class ChoiceDispatchCombinatorParserBase(rd: TermRuntimeData,
// sequence surrounding them and so they aren't set final. In order
// to set these elements final, we do it here as well. We will
// attempt to walk the infoset after the PoU is discarded.
- //
- // Note that we must do a null check because it's possible there was
- // a sequence, which figured out the element was final, walked it and
- // it was removed.
- val newLastChildNode = pstate.infoset.contents.lastOption
- if (newLastChildNode != savedLastChildNode) {
- val last = newLastChildNode.get
- if (last != null) {
- last.setFinal()
- }
+ val newLastChildNode = pstate.infoset.maybeLastChild
+ if (newLastChildNode.isDefined) {
+ newLastChildNode.get.isFinal = true
}
} else {
@@ -260,7 +251,6 @@ abstract class ChoiceDispatchCombinatorParserBase(rd: TermRuntimeData,
}
}
}
- pstate.walker.walk()
}
}
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
index dde04ce..e7c24a1 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
@@ -120,7 +120,6 @@ trait WithDetachedParser {
res
}
- pstate.walker.walk()
pstate.setParent(priorElement)
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/NilEmptyCombinatorParsers.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/NilEmptyCombinatorParsers.scala
index 9cf8a93..c7f9c64 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/NilEmptyCombinatorParsers.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/NilEmptyCombinatorParsers.scala
@@ -43,7 +43,6 @@ abstract class NilOrValueParser(ctxt: TermRuntimeData, nilParser: Parser, valueP
// no-op. We found nil, withPointOfUncertainty will discard the pou
}
}
- pstate.walker.walk()
}
}
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/Parser.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/Parser.scala
index 087efcd..d34ac1f 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/Parser.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/Parser.scala
@@ -190,8 +190,6 @@ class ChoiceParser(
var successfullyParsedChildBranch = false
- val savedLastChildNode = pstate.infoset.contents.lastOption
-
while (!successfullyParsedChildBranch && i < numAlternatives) {
val parser = childParsers(i)
@@ -211,16 +209,9 @@ class ChoiceParser(
// sequence surrounding them and so they aren't set final. In order
// to set these elements final, we do it here as well. We will
// attempt to walk the infoset after the PoU is discarded.
- //
- // Note that we must do a null check because it's possible there was
- // a sequence, which figured out the element was final, walked it and
- // it was removed.
- val newLastChildNode = pstate.infoset.contents.lastOption
- if (newLastChildNode != savedLastChildNode) {
- val last = newLastChildNode.get
- if (last != null) {
- last.setFinal()
- }
+ val newLastChildNode = pstate.infoset.maybeLastChild
+ if (newLastChildNode.isDefined) {
+ newLastChildNode.get.isFinal = true
}
} else {
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
index 6415cfb..ff84db4 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
@@ -19,7 +19,6 @@ package org.apache.daffodil.processors.parsers
import org.apache.daffodil.dsom.TunableLimitExceededError
import org.apache.daffodil.exceptions.Assert
-import org.apache.daffodil.infoset.DIArray
import org.apache.daffodil.infoset.DIComplex
import org.apache.daffodil.processors.ElementRuntimeData
import org.apache.daffodil.processors.Evaluatable
@@ -113,7 +112,6 @@ abstract class SequenceParserBase(
// keep track of the current last child node. If the last child changes
// while parsing, we know a new child was added in this loop
- val savedLastChildNode = pstate.infoset.contents.lastOption
child = children(scpIndex).asInstanceOf[SequenceChildParser]
@@ -203,31 +201,27 @@ abstract class SequenceParserBase(
pstate.mpstate.moveOverOneGroupIndexOnly()
}
- val newLastChildNode = pstate.infoset.contents.lastOption
- if (newLastChildNode != savedLastChildNode) {
- // We have added at least one child to to this complex during
+ val newLastChildNode = pstate.infoset.maybeLastChild
+ if (newLastChildNode.isDefined) {
+ // We have potentially added a child to to this complex during
// this array loop.
//
// If the new child is a DIArray, we know this DIArray has at
// least one element, but we don't know if we actually added a
// new one in this loop or not. So just get the last array
- // element and set it as final anyways. Note that we need a null
- // check because if we didn't add an array element this go
- // around, the last element may have already been walked and
- // freed and so could now be null.
+ // element and set it as final anyways.
//
// If it's not a DIArray, that means it's just an optional
// simple/complex and that will get set final below where all
// other non-array elements get set as final.
- newLastChildNode.get match {
- case a: DIArray => {
- val lastArrayElem = a.contents.last
- if (lastArrayElem != null) {
- lastArrayElem.setFinal()
- pstate.walker.walk()
- }
+ val lastChild = newLastChildNode.get
+ if (lastChild.isArray) {
+ // not simple or complex, must be an array
+ val lastArrayElem = lastChild.maybeLastChild
+ if (lastArrayElem.isDefined) {
+ lastArrayElem.get.isFinal = true
+ pstate.walker.walk()
}
- case _ => // non-array, that gets handled down below
}
}
@@ -290,12 +284,12 @@ abstract class SequenceParserBase(
// we need to potentially set things as final, get the last child to
// determine if it changed from the saved last child, which lets us know
// if a new child was actually added.
- val newLastChildNode = pstate.infoset.contents.lastOption
+ val newLastChildNode = pstate.infoset.maybeLastChild
if (!isOrdered) {
// In the special case of unordered sequences with arrays, the
// childParser is not a RepeatingChildParser, so array elements aren't
- // setFinal above like normal arrays are a. Instead we parse one
+ // set final above like normal arrays are. Instead we parse one
// instance at a time in this loop.
//
// So if this the new last child node is a DIArray, we must set new
@@ -310,24 +304,20 @@ abstract class SequenceParserBase(
// Also note that the DIArray itself is set as final right below this.
// Again, because unordred sequences get blocked, that array won't be
// walked even though it's final.
- newLastChildNode match {
- case Some(a: DIArray) => a.contents.last.setFinal()
- case _ => // do nothing
+ if (newLastChildNode.isDefined && newLastChildNode.get.isArray) {
+ // we have a new last child, and it's not simple or complex, so must
+ // be an array. Set its last child final
+ newLastChildNode.get.maybeLastChild.get.isFinal = true
}
}
// We finished parsing one part of a sequence, which could either be an
// array, simple or complex. If the last child is different from when we
// started that means we must have added a new element and we can set it
- // final and walk. Note that we must do a null check because it's
- // possible this last child was already determined to be final, walked,
- // and freed in the ChoiceParser.
- if (newLastChildNode != savedLastChildNode) {
- val lastChild = newLastChildNode.get
- if (lastChild != null) {
- lastChild.setFinal()
- pstate.walker.walk()
- }
+ // final and walk.
+ if (newLastChildNode.isDefined) {
+ newLastChildNode.get.isFinal = true
+ pstate.walker.walk()
}
if (isOrdered) {
@@ -379,7 +369,6 @@ abstract class SequenceParserBase(
val ans = pstate.withPointOfUncertainty("SequenceParserBase", parser.context) { pou =>
parseOneInstanceWithMaybePoU(parser, pstate, roStatus, One(pou))
}
- pstate.walker.walk()
ans
} else {
parseOneInstanceWithMaybePoU(parser, pstate, roStatus, Nope)