Bug #5483
closed
Failure to optimize SEQ[position() = 1 to $n]
Applies to branch:
11, trunk
Fix Committed on Branch:
11, trunk
Fixed in Maintenance Release:
Description
By design, a filter expression of the form SEQ[position() = 1 to $n]
should be optimised to subsequence(SEQ, 1, $n)
.
This isn't happening (revealed by xslt30-extra test case opt-016). The subexpression 1 to $n
is extracted into a variable, which means that the pattern posiiton() = 1 to $n
doesn't get recognized.
It seems to be working on the Saxon 10 branch.
On the 10 branch, by the time we get to FilterExpression.tryToRewritePositionalFilter()
, the predicate is an IntegerRangeTest
. On the 11 branch it is a GeneralComparison
.
The reason the IntegerRangeTest doesn't get created is that the code to create it at GeneralComparison.optimize() (line 474) has been commented out - after first being changed to throw an AssertionError with the message "optimization needs rethinking"!
The commenting out appears to have happened in January 2021 very early in the work on C# transpilation, and it's possible that the code was commented out simply because of early difficulties transpiling it, and never reinstated. (Except that wouldn't explain the comment about rethinking the optimization).
On further examination, I think the optimisation was commented out when RangeExpression
was extended to handle experimental 4.0 syntax "$Start to $End by $Step". Simplest fix is to reinstate the optimisation provided $Step is 1.
I've now reinstated the optimization and opt-016 is looking good -- except there's an odd problem that the SEF file contains 4 global variables, of which 2 are unreferenced. In fact there are two identical expressions so ideally they would be detected as duplicates and we would only have a single global variable.
It seems the SEF file only contains two global variables, but the explain output contains four.
We're running the test with -export:on. The "explain" output is generated by running PreparedStylesheet.explain() on the reconstituted executable, and this appears to have a list of four global variables (plus the GlobalParam). It we run with -export:off, the same list has only two global variables (plus a GlobalParam). That's clearly wrong...
In fact there are only two global variables in the reconstituted executable, but the StylesheetPackageEE has a list of global variables, and this list contains duplicate entries; it is this list that is used to generate the explain output, so the two variables each appear twice.
The first addition is from PackageLoaderHE.readGlobalVariable() at line 561. The second is from PackageLoaderHE.registerGlobalVariable() at line 529. It looks to me as if the second call is redundant.
I've removed this call and confirmed that all tests still run OK with -export:on
- Status changed from New to In Progress
Now fixed - on the Saxon 11 branch. Needs retrofitting to the main branch.
- Subject changed from Failure to optimize SEQ[posiiton() = 1 to $n] to Failure to optimize SEQ[position() = 1 to $n]
- Description updated (diff)
- Status changed from In Progress to Resolved
- Applies to branch 11, trunk added
- Fix Committed on Branch 11, trunk added
- Platforms .NET, Java added
- Status changed from Resolved to Closed
- % Done changed from 0 to 100
- Fixed in Maintenance Release 11.4 added
Bug fix applied in the Saxon 11.4 maintenance release.
Please register to edit this issue
Also available in: Atom
PDF