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 <slawre...@apache.org>
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.

Reply via email to