Project

Profile

Help

Bug #4802

closed

Overzealous optimization

Added by David Priest about 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Low
Assignee:
Category:
-
Sprint/Milestone:
-
Start date:
2020-10-24
Due date:
% Done:

100%

Estimated time:
Applies to JS Branch:
2
Fix Committed on JS Branch:
2
Fixed in JS Release:
SEF Generated with:
Platforms:
Company:
-
Contact person:
-
Additional contact persons:
-

Description

I believe I've discovered a subtle bug, in which an array:for-each fails to process correctly. In the code below there are four console messages labeled A, B, C, D. The console messages are intended to provide detailed insight into the content of the arrays/sub-arrays/maps.

The essense of the program is to read a set of HTML 'span' elements into a document-ordered array of maps; then to re-order the maps into an array of columns, each column as an array of maps; then to swap the row/column value for each map. These changes are reflected in the console output.

The sole difference between the two versions is the use of a saxon:timestamp() to force evaluation of the function.

In the "SwapNoTimestamp" function it appears that array:for-each is passing in the entire array of arrays each time it should be passing in a sub-array. In the "SwapWithTimestamp" function, it correctly passes in the sub-arrays.

Isolating and demonstrating this XPath behaviour was like debugging a black box. Perhaps a future blog post could address techniques for debugging XPath?

Saxon 10.0 plugin for Oxygen 22.1, compiling to Saxon-JS 2.0, executing in Firefox on Ubuntu.

<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet
  expand-text="yes"
  extension-element-prefixes="ixsl saxon"
  version="3.0"
  xmlns="http://www.w3.org/1999/xhtml"
  xmlns:array="http://www.w3.org/2005/xpath-functions/array"
  xmlns:f="function"
  xmlns:fn="http://www.w3.org/2005/xpath-functions"
  xmlns:ixsl="http://saxonica.com/ns/interactiveXSLT"
  xmlns:js="http://saxonica.com/ns/globalJS"
  xmlns:map="http://www.w3.org/2005/xpath-functions/map"
  xmlns:saxon="http://saxon.sf.net/"
  xmlns:xhtml="http://www.w3.org/1999/xhtml"
  xmlns:xs="http://www.w3.org/2001/XMLSchema"
  xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
  xpath-default-namespace="http://www.w3.org/1999/xhtml">
  <xsl:template
    name="xsl:initial-template">
    <xsl:message>Entering initial-template</xsl:message>
    <xsl:message>A&#9660;</xsl:message>
    <xsl:message>A&#9650; {f:getCells() => f:msg1()}</xsl:message>
    <xsl:message>B&#9660;</xsl:message>
    <xsl:message>B&#9650; { f:getCells() => f:columnOrder() => f:msg2()}</xsl:message>
    <xsl:message>C&#9660;</xsl:message>
    <xsl:message>C&#9650; { f:getCells() => f:columnOrder() => f:swapWithTimestamp() => f:msg2()
      }</xsl:message>
    <xsl:message>D&#9660;</xsl:message>
    <xsl:message>D&#9650; { f:getCells() => f:columnOrder() => f:swapNoTimestamp() => f:msg2() } </xsl:message>
  </xsl:template>
  <xsl:function
    as="array(map(*))"
    name="f:getCells">
    <xsl:sequence
      select="
        array {
          ixsl:page()/id('grid')//span ! map {
            'val': ./text(),
            'row': xs:integer(./count(preceding::span) idiv 3),
            'col': xs:integer(./count(preceding::span) mod 3)
          }
        }" />
  </xsl:function>
  <xsl:function
    as="array(array(map(*)))"
    name="f:columnOrder">
    <xsl:param
      as="array(map(*))"
      name="cells" />
    <xsl:sequence
      select="
        array {
          for $n in 0 to 2
          return
            array:filter($cells, function ($c) {
              xs:integer(map:find($c, 'col')) = xs:integer($n)
            })
        }" />
  </xsl:function>
  <xsl:function
    name="f:swapWithTimestamp">
    <xsl:param
      as="array(array(map(*))*)"
      name="cells" />
    <xsl:message>SwapWithTimestamp {saxon:timestamp()}: {array:size($cells)}</xsl:message>
    <xsl:sequence
      select="
        array:for-each($cells, function ($c) {
          f:swapRowCol($c, array {})
        })" />
  </xsl:function>
  <xsl:function
    name="f:swapNoTimestamp">
    <xsl:param
      as="array(array(map(*))*)"
      name="cells" />
    <xsl:message>SwapNoTimestamp: {array:size($cells)}</xsl:message>
    <xsl:sequence
      select="
        array:for-each($cells, function ($c) {
          f:swapRowCol($c, array {})
        })" />
  </xsl:function>
  <xsl:function
    as="array(map(*))"
    name="f:swapRowCol">
    <xsl:param
      as="array(map(*))"
      name="cells" />
    <xsl:param
      as="array(map(*))"
      name="acc" />
    <xsl:message>Swapping: {array:size($cells)} cells remaining</xsl:message>
    <xsl:sequence
      select="
        if (array:size($cells) gt 0) then
          let $h := array:head($cells)
          return
            f:swapRowCol(array:tail($cells), array:append($acc, map {
              'val': $h('val'),
              'row': $h('col'),
              'col': $h('row')
            }))
        else
          $acc" />
  </xsl:function>
  <xsl:function
    name="f:msg1">
    <xsl:param
      as="array(map(*))"
      name="cells" />
    <xsl:message xml:space="preserve">{array:size($cells)} cells in the array:{array:for-each($cells,function($c){'&#10;val: '||$c('val')||' row: '||$c('row')||' col: '||$c('col')})}</xsl:message>
  </xsl:function>
  <xsl:function
    name="f:msg2">
    <xsl:param
      as="array(array(map(*)*))"
      name="cols" />
    <xsl:message xml:space="preserve">{array:size($cols)} columns in the array:{array:for-each($cols,function($c){'&#10;'||array:size($c)||' cells in column'||f:msg1($c)})}</xsl:message>
  </xsl:function>
</xsl:stylesheet>
<!doctype html>
<html
	lang="en">
	<head>
		<meta
			charset="utf-8">
		<title>Test</title>
		<script src="js/Saxon-JS-2.0/SaxonJS2.rt.js"></script>
		<script type="text/javascript">
      window.onload = function () {
        SaxonJS.transform({
          stylesheetLocation: "xsl/webapp.sef.json",
          initialTemplate: "Q{http://www.w3.org/1999/XSL/Transform}initial-template",
        });
      };</script>
	</head>
	<body>
		<section>
			<h4>Data Source</h4>
			<div
				id="grid">
				<div><span>1</span><span>2</span><span>3</span></div>
				<div><span>4</span><span>5</span><span>6</span></div>
				<div><span>7</span><span>8</span><span>9</span></div>
			</div>
		</section>
	</body>
</html>

Files

webapp.xsl (3.97 KB) webapp.xsl David Priest, 2020-10-24 02:54
index.html (706 Bytes) index.html David Priest, 2020-10-24 02:54
Actions #1

Updated by Michael Kay about 4 years ago

Thanks for reporting it.

I can well relate to your problems in debugging this. When the going gets tough, I tend to resort to debugging at the level of Saxon internals, but that's obviously not a technique I can recommend to users. We've put effort into improving diagnostics when things fail, and into XSLT-level tracing, but when you've got an XPath expression that delivers the wrong answer (usually an empty sequence) it can be very tough to work out why.

Actions #2

Updated by Michael Kay about 4 years ago

  • Project changed from Saxon to SaxonJS
  • Applies to branch deleted (10)
Actions #3

Updated by Michael Kay about 4 years ago

I tried running this under Node.js (current development branch) with

 -xsl:webApp.xsl -it:main -master:index.html -t -debug

and it failed:

Saxon-JS 2.0dev from Saxonica 
Node.js version v14.9.0
Compiling stylesheet /Users/mike/bugs/2020/4802-Priest/webApp.xsl
Error FODC0002
    Attribute without value at line 1 column 30
Failed to compile stylesheet: Static error in XPath on line 70 in 4802-Priest/webApp.xsl {         array:for-each($cells, function ($c) {           f:swapRowCol($c, array {})         })}: Required type of second argument of f:swapRowCol() is array(map(*)): actual type is array(xs:error0)
(node:47362) UnhandledPromiseRejectionWarning: Error
    at new $XError$$module$temp$js$source$src$xerror$ (/Users/mike/GitHub/saxon-js-enterprise/build/temp/js/build/SaxonJS2N.debug.js:839:36)
    at error (/Users/mike/GitHub/saxon-js-enterprise/build/temp/js/build/SaxonJS2N.debug.js:10666:29)
    at /Users/mike/GitHub/saxon-js-enterprise/build/temp/js/build/SaxonJS2N.debug.js:13325:14
    at $$jscomp$generator$Engine_$.program_ (/Users/mike/GitHub/saxon-js-enterprise/build/temp/js/build/SaxonJS2N.debug.js:12142:23)

I don't know exactly what's going on here, but I guess it's caused by processing HTML where XML is expected. It turned out to be bad copying of the HTML. Turning the HTML into well-formed XML fixed the problem, but we need to investigate why it's crashing rather than producing good diagnostics.

Actions #4

Updated by Michael Kay about 4 years ago

When I fix this, I now get

Compiling stylesheet /Users/mike/bugs/2020/4802-Priest/webApp.xsl
Failed to compile stylesheet: Static error in XPath on line 70 in 4802-Priest/webApp.xsl {         array:for-each($cells, function ($c) {           f:swapRowCol($c, array {})         })}: 
Required type of second argument of f:swapRowCol() is array(map(*)): actual type is array(xs:error0)
Error Q{http://www.w3.org/2005/xqt-errors}XPTY0004 at xpath.xsl#963
    Failed to compile stylesheet

This is a problem with the type inferencing in the XX compiler. Again, it needs to be looked at, but it's not relevant to this issue.

Actions #5

Updated by Michael Kay about 4 years ago

I've now compiled it under XJ.

Running with -explain, noted that function f:swapNoTimestamp is inlined, whereas swapWithTimestamp isn't. This could well turn out to be the critical difference between them.

Now running the SEF file under Saxon-JS we get

Entering initial-template
A▼
0 cells in the array:
A▲ 
B▼
0 cells in the array:
0 cells in the array:
0 cells in the array:
3 columns in the array:
0 cells in column 
0 cells in column 
0 cells in column
B▲ 
C▼
SwapWithTimestamp 2020-10-24T23:10:23.641+01:00: 3
Swapping: 0 cells remaining
Swapping: 0 cells remaining
Swapping: 0 cells remaining
0 cells in the array:
0 cells in the array:
0 cells in the array:
3 columns in the array:
0 cells in column 
0 cells in column 
0 cells in column
C▲ 
D▼
SwapNoTimestamp: 3
Swapping: 0 cells remaining
Swapping: 0 cells remaining
Swapping: 0 cells remaining
0 cells in the array:
0 cells in the array:
0 cells in the array:
3 columns in the array:
0 cells in column 
0 cells in column 
0 cells in column
D▲  

Execution time: 0.22s
Memory used: 23.04Mb
Transformation complete

The output for C and D appears to be identical. I'm not sure what the correct output is supposed to be, but it looks like we haven't reproduced the problem

Actions #6

Updated by Michael Kay about 4 years ago

I've now run the same thing on Saxon-JS 2.0 as issued and the relevant output is:

C▼
SwapWithTimestamp 2020-10-24T23:21:41.705+01:00: 3
Swapping: 0 cells remaining
Swapping: 0 cells remaining
Swapping: 0 cells remaining
0 cells in the array:
0 cells in the array:
0 cells in the array:
3 columns in the array:
0 cells in column 
0 cells in column 
0 cells in column
C▲ 
D▼
SwapNoTimestamp: 3
0 cells in the array:
0 cells in the array:
0 cells in the array:
3 columns in the array:
0 cells in column 
0 cells in column 
0 cells in column
D▲  

This time the two messages are different, so presumably we have reproduced the bug. The supposition is therefore that this is something we have fixed since Saxon-JS 2 was released.

Actions #7

Updated by Michael Kay about 4 years ago

I've been looking in the history of bug fixes since 2.0 and I don't see anything obvious. I've also reviewed the commit log. There's been a lot of development since the 2.0 release that isn't recorded in bug entries, and it's entirely possible that a bug was fixed "in passing" without explicit documentation.

This leaves us in the rather unsatisfactory position that we seem to have fixed it, without knowing what it is; which in turn makes it difficult to provide a workaround.

Actions #8

Updated by Michael Kay about 4 years ago

I tried generating the SEF file with -opt:-f which switches off function inlining, and the execution on Node.js now produces correct output using the released Saxon-JS 2 product. So as a workaround, please switch off this optimisation.

This still doesn't explain the cause, unfortunately.

Actions #9

Updated by Michael Kay about 4 years ago

  • Status changed from New to Resolved
  • Assignee set to Michael Kay
  • Applies to JS Branch 2.0 added
  • Fix Committed on JS Branch Trunk added

I'm going to assume that we have fixed this, since the problem is no longer reproducible on the development branch.

Actions #10

Updated by David Priest about 4 years ago

(Assuming the "Edit" button at the bottom of the page appends a new message)

Sorry for not responding to your findings; I only received email when the issue was closed and was unaware that you'd been working on it. I, too, will assume that the issue is fixed. :-)

Actions #11

Updated by Community Admin almost 4 years ago

  • Applies to JS Branch 2 added
  • Applies to JS Branch deleted (2.0)
Actions #12

Updated by Debbie Lockett almost 4 years ago

  • Fix Committed on JS Branch 2 added
  • Fix Committed on JS Branch deleted (Trunk)
Actions #13

Updated by Debbie Lockett almost 4 years ago

  • Status changed from Resolved to Closed
  • % Done changed from 0 to 100
  • Fixed in JS Release set to Saxon-JS 2.1

Bug fix applied in the Saxon-JS 2.1 maintenance release.

Please register to edit this issue

Also available in: Atom PDF Tracking page