https://saxonica.plan.io/https://saxonica.plan.io/favicon.ico2020-09-15T14:23:52ZSaxonica Developer CommunitySaxon - Bug #4738: Optimization bug: xsl:choose with branches uniformly comparing untypedAtomic to numerichttps://saxonica.plan.io/issues/4738?journal_id=162992020-09-15T14:23:52ZMichael Kaymike@saxonica.com
<ul></ul><p>Thank you for reporting it. As you have found out, this is caused by a bug in the optimizer.</p>
<p>In fact it's caused by two separate optimizations that interfere with each other. The first of these is to use an optimized comparison of untypedAtomic values to numerics that avoids the full cost of number-to-string conversion; the second is to use a hash table lookup for the choose expression because the branches are homogenous and disjoint. Both of these optimizations are known to give very worthwhile performance improvements for some workloads.</p>
<p>Your easiest workaround is to replace the numeric literals in the <code>@test</code> expressions with string literals, that is, change <code>@SaleLock = 1</code> to <code>@SaleLock = '1'</code>. That's assuming you don't need to match alternative representations like '01' or '+1', in which case you could do the conversion explicitly.</p>
<p>I'm afraid the answers to your questions are somewhat banal:</p>
<p>(1) Bugs happen. Optimization is particularly prone to bugs, because it makes the code less orthogonal and therefore increases the number of test cases needed combinatorially. This bug is very typical of optimization bugs: two conditions which might each be present in 1 out of 50 stylesheets are present in combination only in 1 out of 2500 stylesheets. Although we run hundreds of thousands of test cases, most of them are designed to test features in isolation rather than in combination.</p>
<p>(2) Other bugs. Although 9.9.x is now very stable indeed, and 10.x is also becoming quite stable, in our experience it's not that unusual for a bug to survive in the product for five years before anyone detects it.</p>
<p>(3) Testing. It's a really good idea to have a good test suite for your own stylesheets. A really comprehensive set of XSpec tests would be ideal; but as a minimum, a test suite that simply tests each stylesheet against a small number of input documents and compares the results with expected results to see they haven't changed.</p>
<p>Unless you indicate otherwise, we will take this test case (thanks for packaging it up so cleanly) and add it to the W3C XSLT3 test suite which we run for continuous testing to prevent regressions.</p> Saxon - Bug #4738: Optimization bug: xsl:choose with branches uniformly comparing untypedAtomic to numerichttps://saxonica.plan.io/issues/4738?journal_id=163002020-09-15T14:37:42ZMarek Skorek
<ul></ul><p>Thanks you the answer Micheal,</p>
<blockquote>
<p>Your easiest workaround is to replace the numeric literals in the @test expressions with string literals, that is, change @SaleLock = 1 to @SaleLock = '1'. That's assuming you don't need to match alternative representations like '01' or '+1', in which case you could do the conversion explicitly.</p>
</blockquote>
<p>It is hard to check all the transformations because our integrating team created more than 3,5k different ones. Some of them have side effects or create dynamic data (i.e. timestamps) which makes them hard to compare.</p>
<p>Nevertheless, I am considering to disable optimization by default and enable it only for transfromations with a poor performance. Is it a good way?</p>
<blockquote>
<p>Unless you indicate otherwise, we will take this test case (thanks for packaging it up so cleanly) and add it to the W3C XSLT3 test suite which we run for continuous testing to prevent regressions.</p>
</blockquote>
<p>Feel free to include it as a test case ;-)
I will wait for a bugfix.</p> Saxon - Bug #4738: Optimization bug: xsl:choose with branches uniformly comparing untypedAtomic to numerichttps://saxonica.plan.io/issues/4738?journal_id=163022020-09-15T15:12:48ZMichael Kaymike@saxonica.com
<ul><li><strong>Subject</strong> changed from <i>Upgrading Saxon to 9.9 breaks working transformations</i> to <i>Optimization bug: xsl:choose with branches uniformly comparing untypedAtomic to numeric</i></li><li><strong>Category</strong> set to <i>Internals</i></li><li><strong>Assignee</strong> set to <i>Michael Kay</i></li><li><strong>Applies to branch</strong> <i>10, trunk</i> added</li><li><strong>Fix Committed on Branch</strong> <i>10, 9.9, trunk</i> added</li></ul><p>Fix committed on 9.9, 10, and development branches.</p>
<p>(Although I've committed a fix for 9.9, we're no longer producing routine maintenance releases on that branch: 10.x is now recommended).</p>
<p>The fix ensures that the switch operand is converted from untyped atomic to numeric in this scenario.</p>
<p>W3C test case choose-0202 created.</p> Saxon - Bug #4738: Optimization bug: xsl:choose with branches uniformly comparing untypedAtomic to numerichttps://saxonica.plan.io/issues/4738?journal_id=163032020-09-15T15:20:07ZMichael Kaymike@saxonica.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Resolved</i></li></ul><p><em>Nevertheless, I am considering to disable optimization by default and enable it only for transfromations with a poor performance. Is it a good way?</em></p>
<p>I don't think that would significantly reduce your risk of hitting a previously unknown bug. Given that 9.9 has been out in the field for just short of two years and is used by tens of thousands of users, the chance of hitting any bug is very low. We run far more tests with optimization enabled than with it disabled, and your best chance of avoiding under-exercised paths has to be to default as many options as possible.</p>
<p>Upgrading to a new version of any software library when you have no regression tests for your application is something that would worry me a little, though.</p> Saxon - Bug #4738: Optimization bug: xsl:choose with branches uniformly comparing untypedAtomic to numerichttps://saxonica.plan.io/issues/4738?journal_id=166402020-10-22T16:20:59ZO'Neil Delprattoneil@saxonica.com
<ul><li><strong>Fixed in Maintenance Release</strong> <i>9.9.1.8</i> added</li></ul><p>Bug fix applied on the Saxon 9.9.1.8 maintenance release. Leaving open until applied on the Saxon 10 maintenance release.</p> Saxon - Bug #4738: Optimization bug: xsl:choose with branches uniformly comparing untypedAtomic to numerichttps://saxonica.plan.io/issues/4738?journal_id=167292020-10-28T17:58:00ZO'Neil Delprattoneil@saxonica.com
<ul></ul><p>Bug fix applied in the Saxon 10.3 maintenance release</p> Saxon - Bug #4738: Optimization bug: xsl:choose with branches uniformly comparing untypedAtomic to numerichttps://saxonica.plan.io/issues/4738?journal_id=167512020-10-28T18:12:54ZO'Neil Delprattoneil@saxonica.com
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>Closed</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li><li><strong>Fixed in Maintenance Release</strong> <i>10.3</i> added</li></ul>