Project

Profile

Help

Bug #3261

closed

Duplicate binding slot assignment (switch optimization)

Added by Radu Coravu almost 7 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
-
Start date:
2017-06-12
Due date:
% Done:

100%

Estimated time:
Legacy ID:
Applies to branch:
9.7, 9.8, 9.9
Fix Committed on Branch:
9.7, 9.8, 9.9
Fixed in Maintenance Release:
Platforms:

Description

This is very similar with:

https://saxonica.plan.io/issues/3195

with probably the same stack trace but caused by another situation, as it still occurs with the patch from https://saxonica.plan.io/issues/3195 present:

java.lang.AssertionError: Duplicate binding slot assignment
	at net.sf.saxon.expr.GlobalVariableReference.setBindingSlot(GlobalVariableReference.java:60)
	at net.sf.saxon.expr.instruct.ComponentCode.processComponentReference(ComponentCode.java:140)
	at net.sf.saxon.expr.instruct.ComponentCode.allocateBindingSlotsRecursive(ComponentCode.java:106)
	at net.sf.saxon.expr.instruct.ComponentCode.allocateBindingSlotsRecursive(ComponentCode.java:109)
	at net.sf.saxon.expr.instruct.ComponentCode.allocateBindingSlotsRecursive(ComponentCode.java:109)
	at net.sf.saxon.expr.instruct.ComponentCode.allocateBindingSlotsRecursive(ComponentCode.java:109)
	at net.sf.saxon.expr.instruct.ComponentCode.allocateBindingSlotsRecursive(ComponentCode.java:109)
	at net.sf.saxon.expr.instruct.ComponentCode.allocateBindingSlotsRecursive(ComponentCode.java:109)
	at net.sf.saxon.expr.instruct.ComponentCode.allocateBindingSlotsRecursive(ComponentCode.java:109)
	at net.sf.saxon.expr.instruct.ComponentCode.allocateAllBindingSlots(ComponentCode.java:100)
	at net.sf.saxon.style.PrincipalStylesheetModule.compile(PrincipalStylesheetModule.java:1334)
	at net.sf.saxon.style.Compilation.compilePackage(Compilation.java:265)
	at net.sf.saxon.style.StylesheetModule.loadStylesheet(StylesheetModule.java:260)
	at net.sf.saxon.style.Compilation.compileSingletonPackage(Compilation.java:101)
	at net.sf.saxon.s9api.XsltCompiler.compile(XsltCompiler.java:859)
	at net.sf.saxon.jaxp.SaxonTransformerFactory.newTemplates(SaxonTransformerFactory.java:177)
	at net.sf.saxon.jaxp.SaxonTransformerFactory.newTransformer(SaxonTransformerFactory.java:131)

I will try to provide some test XML and XSLT.

I did not test this with the latest Saxon 9.8.

Actions #1

Updated by Radu Coravu almost 7 years ago

The test XSLT is this one:

<xsl:stylesheet 
	xmlns:xsl	= "http://www.w3.org/1999/XSL/Transform"
	xmlns:xs	= "http://www.w3.org/2001/XMLSchema"
	xmlns:svg	= "http://www.w3.org/2000/svg"
	exclude-result-prefixes="xs"
	xmlns:w="http://www.werum.com"
	version="2.0">
	
	<xsl:variable name="BOX_WIDTH"	as="xs:double"	select="0"/>
	<xsl:variable name="BOX_HEIGHT"	as="xs:double"	select="0"/>
	
	<xsl:variable name="CIRCLE_R"	as="xs:double"	select="0"/>
	<xsl:variable name="FONT_SIZE"	as="xs:double"	select="0"/>
	

	<xsl:template name="drawItem">
		<xsl:param as="xs:double" name="itemX"/>
		<xsl:param as="xs:double" name="itemY"/>
		<xsl:param as="xs:string" name="itemType" />
		<xsl:param  name="text1"/>
		<xsl:param  name="text2"/>
		
		<xsl:choose>
	
			<xsl:when test="$itemType='StartStep' or $itemType='EndStep'">
				<svg:circle  
					r="{$CIRCLE_R}"
					cx="{$itemX + $CIRCLE_R }"
					cy="{$itemY + $CIRCLE_R }"/>
			</xsl:when>
			<xsl:when test="$itemType='Splitting'">
				<svg:line  
					x1="{$itemX}"
					y1="{$itemY}"
					x2="{$itemX + $BOX_WIDTH }"
					y2="{$itemY}"
					style="stroke:rgb(0,0,0);stroke-width:5"
				/>
			</xsl:when>
			<xsl:otherwise>
				<xsl:choose>
					<xsl:when test="$itemType='Synchronisation'">
						<svg:line  
							x1="{$itemX}"
							y1="{$itemY }"
							x2="{$itemX + $BOX_WIDTH div 2}"
							y2="{$itemY + $BOX_HEIGHT div 2}"
							style="stroke:rgb(0,0,0);stroke-width:5"
						/>
					</xsl:when>
					<xsl:otherwise>
						<svg:rect 
						x		= "{$itemX}" 
						y		= "{$itemY}" 
						width	= "{$BOX_WIDTH}" 
						height	= "{$BOX_HEIGHT}"
						stroke	= "black"
						fill	= "none"/>
					</xsl:otherwise>
				</xsl:choose>
			</xsl:otherwise>
		</xsl:choose>
	</xsl:template>		
</xsl:stylesheet>

The sample XML does not matter as the problem is reported during stylesheet compilation.

Actions #2

Updated by Michael Kay almost 7 years ago

Fails with essentially the same error in 9.8, but the error message is now

Exception in thread "main" java.lang.AssertionError: **** Component reference is already bound

Actions #3

Updated by Michael Kay almost 7 years ago

The xsl:choose has been turned into a SwitchExpression, and there are two branches (corresponding to <xsl:when test="$itemType='StartStep' or $itemType='EndStep'">) that share the same action expression, so it is processed twice when walking the expression tree recursively.

Workaround: split this condition into two separate (but identical) branches.

Actions #4

Updated by Michael Kay almost 7 years ago

The obvious and simple fix here is to copy() the action expression into each relevant entry of the switch. This works, but in some situations, for example when the branch contains a lot of code and the "or" condition has many branches, it's going to be rather inefficient.

A better approach might be to to create an indirection: instead of the hash table mapping a match key directly to an action expression, it should map a match key to an integer, where the integer acts as an index into an array of action expressions. The question then becomes whether we should change the SEF export format to reflect this new structure, or try to keep the SEF format compatible. I really would like to avoid introducing incompatible SEF changes in a maintenance release.

Actions #5

Updated by Michael Kay almost 7 years ago

  • Status changed from New to Resolved
  • Assignee set to Michael Kay
  • Priority changed from Low to Normal
  • Applies to branch 9.8, trunk added
  • Fix Committed on Branch 9.7, 9.8, trunk added

I have implemented the "obvious and simple" (strong and stable??) fix on both the 9.7 and 9.8 branches, with a TODO to consider optimization to common-up the action branches for consideration later.

XSLT 3.0 test case choose-0105 created.

Actions #6

Updated by Radu Coravu almost 7 years ago

Actions #7

Updated by Michael Kay almost 7 years ago

The fix is in commit 1576 in the private repository - module com.saxonica.ee.optim.OptimizerEE.

Actions #8

Updated by Radu Coravu almost 7 years ago

Until now we've restrained from patching the "com.saxonica" packages in our code.

Will there be another Saxon 9.7 minor bug fix release containing this fix?

Actions #9

Updated by Michael Kay almost 7 years ago

There will definitely be further maintenance releases on the 9.7 branch, but I can't give you any specific timing.

Actions #10

Updated by Radu Coravu almost 7 years ago

No problem, thanks for the great support. We'll wait.

Actions #11

Updated by O'Neil Delpratt almost 7 years ago

  • % Done changed from 0 to 100
  • Fixed in Maintenance Release 9.8.0.2 added

Patch applied in the Saxon 9.8.0.2 maintenance release. Leaving bug issue marked as resolved until applied in the next Saxon 9.7 maintenance release.

Actions #12

Updated by O'Neil Delpratt almost 7 years ago

  • Status changed from Resolved to Closed
  • Fixed in Maintenance Release 9.7.0.19 added

Bug fix applied in the 9.7.0.19 maintenance release.

Actions #13

Updated by Michael Kay over 5 years ago

  • Subject changed from Duplicate binding slot assignment to Duplicate binding slot assignment (switch optimization)
  • Description updated (diff)
  • Status changed from Closed to Resolved
  • Applies to branch 9.9 added
  • Applies to branch deleted (trunk)
  • Fix Committed on Branch 9.9 added
  • Fix Committed on Branch deleted (trunk)

On the 9.9 branch, I have made changes to deal with the inefficiencies caused by the previous solution to this bug.

As envisaged, I have introduced an indirection so that several key values can point to the same action expression. This avoids duplication of the expression code, which is particularly relevant if it is compiled.

The exported SEF file is the same (including duplicated expressions) which has the advantage that no changes to Saxon-JS are needed. There is potential for future enhancements to remove the duplication in this case also.

Actions #14

Updated by O'Neil Delpratt over 5 years ago

  • Fixed in Maintenance Release 9.9.1.1 added
  • Fixed in Maintenance Release deleted (9.8.0.2, 9.7.0.19)

Bug fix applied to the Saxon 9.9.1.1 maintenance release.

Actions #15

Updated by O'Neil Delpratt almost 5 years ago

  • Fixed in Maintenance Release 9.9.1.4 added
  • Fixed in Maintenance Release deleted (9.9.1.1)

Bug fix applied in the Saxon 9.9.1.4 maintenance release.

Actions #16

Updated by O'Neil Delpratt over 4 years ago

  • Status changed from Resolved to Closed
  • Fixed in Maintenance Release 9.9.1.5 added
  • Fixed in Maintenance Release deleted (9.9.1.4)

Bug fix applied in the Saxon 9.9.1.5 maintenance release.

Please register to edit this issue

Also available in: Atom PDF