mbeckerle commented on a change in pull request #437:
URL: https://github.com/apache/incubator-daffodil/pull/437#discussion_r505621141
##########
File path:
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementUnparser.scala
##########
@@ -417,39 +418,56 @@ sealed trait RegularElementUnparserStartEndStrategy
UnparseError(Nope, One(state.currentLocation), "Expected element
start event for %s, but received %s.",
erd.namedQName.toExtendedSyntax, event)
}
- val res = event.info.element
- val mCurNode = state.currentInfosetNodeMaybe
- if (mCurNode.isDefined) {
- val c = mCurNode.get.asComplex
- Assert.invariant(!c.isFinal)
- if (c.maybeIsNilled == MaybeBoolean.True) {
- // cannot add content to a nilled complex element
- UnparseError(One(erd.schemaFileLocation), Nope, "Nilled complex
element %s has content from %s",
- c.erd.namedQName.toExtendedSyntax,
- res.erd.namedQName.toExtendedSyntax)
- }
- c.addChild(res, state.tunable)
- } else {
- val doc = state.documentElement
- doc.addChild(res, state.tunable) // DIDocument, which is never a
current node, must have the child added
- doc.isFinal = true // that's the only child.
- }
- res
+ event.info.element
} else {
Assert.invariant(state.withinHiddenNest)
// Since we never get events for elements in hidden contexts, their
infoset elements
// will have never been created. This means we need to manually
create them
- val e = if (erd.isComplexType) new DIComplex(erd) else new
DISimple(erd)
- e.setHidden()
- state.currentInfosetNode.asComplex.addChild(e, state.tunable)
- e
+ val hiddenElem = if (erd.isComplexType) new DIComplex(erd) else new
DISimple(erd)
+ hiddenElem.setHidden()
+ hiddenElem
+ }
+
+ // now add this new elem to the infoset
+ val parentNodeMaybe = state.currentInfosetNodeMaybe
+ if (parentNodeMaybe.isDefined) {
+ val parentComplex = parentNodeMaybe.get.asComplex
+ Assert.invariant(!parentComplex.isFinal)
+ if (parentComplex.maybeIsNilled == MaybeBoolean.True) {
+ // cannot add content to a nilled complex element
+ UnparseError(One(erd.schemaFileLocation), Nope, "Nilled complex
element %s has content from %s",
+ parentComplex.erd.namedQName.toExtendedSyntax,
+ newElem.erd.namedQName.toExtendedSyntax)
}
+ // We are about to add a child to this complex element. Before we do
+ // that, if the last child added to this complex is a DIArray, and this
+ // new child isn't part of that array, that implies that the DIArray
+ // will have no more children added and should be marked as final, and
Review comment:
So there's some confusion here around the role of isFinal in my mind.
Originally, the point of isFinal is to allow expressions to unblock and
evaluate. isFinal has to do with whether the infoset is fully constructed, and
questions like fn:count of an array or fn:exists(...child ...) can return
false.
Whether the element has been unparsed completely and can be freed if not
needed by an expression seems separate. In my mind these are orthogonal
concepts. isFinal has to do with expressions and construction of the infoset.
Are these now somehow combined? Is this perhaps explained in things not in
this change set?
If we wait for an entire array to be isFinal before we free any of its child
elements, then we cannot truly stream a large array. Freeing element with index
N-1 upon unparse of element N seems essential to obtain streaming behavior.
I would expect we would pass the array index down to the
freeChildIfNoLongerNeeded call, since without that it can't decide to release
things based on index.
##########
File path:
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementUnparser.scala
##########
@@ -515,35 +567,64 @@ trait OVCStartEndStrategy
Assert.invariant(endEv.isEnd && endEv.erd == erd)
val e = new DISimple(erd)
- state.currentInfosetNode.asComplex.addChild(e, state.tunable)
// Remove any state that was set by what created this event. Later
// code asserts that OVC elements do not have a value
e.resetValue
e
} else {
// Event was optional and didn't exist, create a new InfosetElement
and add it
val e = new DISimple(erd)
- state.currentInfosetNode.asComplex.addChild(e, state.tunable)
e
}
} else {
// Event was hidden and will never exist, create a new InfosetElement
and add it
val e = new DISimple(erd)
e.setHidden()
- state.currentInfosetNode.asComplex.addChild(e, state.tunable)
e
}
- val e = One(elem)
- state.currentInfosetNodeStack.push(e)
+ // We are about to add a new OVC child to this complex element. Before we
+ // do that, if the last child added to this complex is a DIArray, that
+ // implies that the DIArray will have no more children added and should be
+ // marked as final, and we can attempt to free that array.
+ val parentNode = state.currentInfosetNode
+ val parentComplex = parentNode.asComplex
+ val lastChildMaybe = parentComplex.maybeLastChild
+ if (lastChildMaybe.isDefined) {
+ val lastChild = lastChildMaybe.get
+ if (lastChild.isArray) {
+ lastChild.isFinal = true
+ if (state.releaseUnneededInfoset) {
+ parentComplex.freeChildIfNoLongerNeeded(parentComplex.numChildren -
1)
+ }
+ }
+ }
+
+ parentComplex.addChild(ovcElem, state.tunable)
+ state.currentInfosetNodeStack.push(One(ovcElem))
}
protected final override def unparseEnd(state: UState): Unit = {
- state.currentInfosetNodeStack.pop
-
// if an OVC element existed, the start AND end events were consumed in
// unparseBegin. No need to advance the cursor here.
+ // ovcElem is finished, free it if possible. OVC elements are not allow in
+ // arrays, so we can directly get the diParent to get the container DINode
+ if (state.releaseUnneededInfoset) {
+ val ovcElem = state.currentInfosetNodeStack.pop
+ val ovcContainer = ovcElem.get.diParent
+ ovcContainer.freeChildIfNoLongerNeeded(ovcContainer.numChildren - 1)
Review comment:
So, we're not doing any incremental removal of array elements, i.e.,
removing element N-1 once we unparse element N even if they are referenced by
expression? I did not find any logic doing that reasoning. Is it here and I
just didn't find it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
