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 f4dbaca Fix performance regression related to alignment
f4dbaca is described below
commit f4dbacafd66bc68fb1ec4425ef67a35ac58855b7
Author: Steve Lawrence <[email protected]>
AuthorDate: Mon Mar 23 09:02:59 2020 -0400
Fix performance regression related to alignment
Commit 356291c3f5 changed how priorPotentialTerms worked (for the
better) to remove the tuple. The purpose of the second value in tuple
was to optionally contain the parent model group term if there were no
prior terms or if this was an unordered sequence. This was confusing and
probably not originally a good idea, and it wasn't even documented.
When removing backpointers in the aforementioned commit, the alignment
logic was changed to replace optEnclosingParent (assigned from the
second tuple value) with immediatelyEnclosingModelGroup, a seemingly
logical replacement based on the name. But optEnclosingParent was just
poorly named since it was really just the enclosing parent if this term
had no prior siblings or was in an unordered sequence. By using
imediatelyEnclosingModelGroup, which always exist except for root, we
always included the parent alignment even when it couldn't have an
affect on a terms alignment. This meant we could insert unnecessary
alignment processors and caused a pretty drastic reduction in
performance in some cases.
This changes the logic so we only ever include the parent alignment if
it could have an affect and this terms alignment, and does so without
the need of a confusing tuple.
DAFFODIL-2299
---
.../org/apache/daffodil/grammar/AlignedMixin.scala | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/AlignedMixin.scala
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/AlignedMixin.scala
index 877c6fe..9fa4c53 100644
---
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/AlignedMixin.scala
+++
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/AlignedMixin.scala
@@ -197,10 +197,20 @@ trait AlignedMixin extends GrammarMixin { self: Term =>
}
eaa
}.toSeq
- val parentAlignmentApprox = immediatelyEnclosingModelGroup.map { p =>
- val csa = p.contentStartAlignment
- csa
- }.toSeq
+
+ val parentAlignmentApprox =
+ if (priorSibs.isEmpty || isEverInUnorderedSequence) {
+ // we only care about the parent alignment if we are the first child
+ // in a model group, or if we are in an unordered sequence and we
+ // could be first when parsing
+ immediatelyEnclosingModelGroup.map { p =>
+ val csa = p.contentStartAlignment
+ csa
+ }.toSeq
+ } else {
+ Seq()
+ }
+
val priorAlignmentsApprox = priorSibsAlignmentsApprox ++
parentAlignmentApprox ++ arraySelfAlignment ++ unorderedSequenceSelfAlignment
if (priorAlignmentsApprox.isEmpty)
alignmentApprox // it will be the containing context's responsibility
to insure this IS where we start.