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.