XalanJ2
  1. XalanJ2
  2. XALANJ-2206

[PATCH] Propagate template inlining trax attribute

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.7
    • Fix Version/s: 2.7.1
    • Component/s: XSLTC
    • Labels:
      None
    • Environment:
      trax, command line is not affected
    • Xalan info:
      PatchAvailable

      Description

      Template inlining attribute set on xsltc TransformerFactoryImpl has no effect, default value from Stylesheet is always used. Attached patch resolves this issue.

      This is required in order to use large stylesheets (such as where applyTemplates method with template inlining is over 64Kb).

      1. inlining-patch.diff
        6 kB
        Vadim Gritsenko
      2. xalanj-2206.inlining-patch4.txt
        4 kB
        Brian Minchau

        Activity

        Hide
        Vadim Gritsenko added a comment -

        Patch:

        • Change default value of Stylesheet._templateInlining to false, to match XSLTC._templateInlining
        • Add XSLTC.isTemplateInlining()
        • Initialize template inlining in TemplatesHandlerImpl.<init>
        • Propagate template inlining value to created Stylesheet object
        • Support ENABLE_INLINING attribute in TransformerFactoryImpl.getAttribute()
        Show
        Vadim Gritsenko added a comment - Patch: Change default value of Stylesheet._templateInlining to false, to match XSLTC._templateInlining Add XSLTC.isTemplateInlining() Initialize template inlining in TemplatesHandlerImpl.<init> Propagate template inlining value to created Stylesheet object Support ENABLE_INLINING attribute in TransformerFactoryImpl.getAttribute()
        Hide
        Brian Minchau added a comment -

        Assiging to Yash T., per the JIRA meeting on Nov. 1, 2005

        Show
        Brian Minchau added a comment - Assiging to Yash T., per the JIRA meeting on Nov. 1, 2005
        Hide
        Brian Minchau added a comment -

        Per the JIRA meeting on Oct 16, 2005, Yash has agreed to look at this one again.

        Show
        Brian Minchau added a comment - Per the JIRA meeting on Oct 16, 2005, Yash has agreed to look at this one again.
        Hide
        Vadim Gritsenko added a comment -

        So Brian, was Yash able to do it?

        Show
        Vadim Gritsenko added a comment - So Brian, was Yash able to do it?
        Hide
        Brian Minchau added a comment -

        Yash is going to look at this one. If OK it should make the 2.7.1 release, but this will probably be the last fix in the door for that release. - Brian

        Show
        Brian Minchau added a comment - Yash is going to look at this one. If OK it should make the 2.7.1 release, but this will probably be the last fix in the door for that release. - Brian
        Hide
        Brian Minchau added a comment -

        Actually Yash punted the problem to another committer, but the ball bounced off them and I'm catching it before it falls on the ground.

        I have some review comments on the patch so far:

        Basically good so far.

        Curious that the patch gets rid of a few new Boolean(true) sort of things and replaces them
        with Boolean.TRUE, which is a good thing, ... but then the patch goes on to add this in TransformerFactoryImpl:
        return new Boolean(_enableInling)
        rather than this:
        if (_enableInlining)
        return Boolean.TRUE;
        else
        return Boolean.FALSE;

        Minor stuff over all. A bit more review of mine needed before I approve/commit a slightly modified version of the patch.

        Show
        Brian Minchau added a comment - Actually Yash punted the problem to another committer, but the ball bounced off them and I'm catching it before it falls on the ground. I have some review comments on the patch so far: Basically good so far. Curious that the patch gets rid of a few new Boolean(true) sort of things and replaces them with Boolean.TRUE, which is a good thing, ... but then the patch goes on to add this in TransformerFactoryImpl: return new Boolean(_enableInling) rather than this: if (_enableInlining) return Boolean.TRUE; else return Boolean.FALSE; Minor stuff over all. A bit more review of mine needed before I approve/commit a slightly modified version of the patch.
        Hide
        Brian Minchau added a comment -

        Attaching a slightly modified patch. The changes are mostly superficial.

        A little more setting of the inlining value from one object to another,
        not just when the value is true, but regardless of the value. The old code is a little too smug about knowing the default value (false) and so not bothering to set it when it is false.

        Other than that I'm convinced that default value of true for the inlining flag inside org/apache/xalan/xsltc/compiler/Stylesheet was wrong. The default of similar flags in these classes:
        org/apache/xalan/xsltc/compiler/XSLTC
        org/apache/xalan/xsltc/trax/TransformerFactoryImpl
        was already false, and the value in StyleSheet was over-ridden by the TransformerFactory anyway, so that is a moot point. No performance hits by setting the default value to false like the other two.

        I approve the change and am going to commit this modified patch.

        Show
        Brian Minchau added a comment - Attaching a slightly modified patch. The changes are mostly superficial. A little more setting of the inlining value from one object to another, not just when the value is true, but regardless of the value. The old code is a little too smug about knowing the default value (false) and so not bothering to set it when it is false. Other than that I'm convinced that default value of true for the inlining flag inside org/apache/xalan/xsltc/compiler/Stylesheet was wrong. The default of similar flags in these classes: org/apache/xalan/xsltc/compiler/XSLTC org/apache/xalan/xsltc/trax/TransformerFactoryImpl was already false, and the value in StyleSheet was over-ridden by the TransformerFactory anyway, so that is a moot point. No performance hits by setting the default value to false like the other two. I approve the change and am going to commit this modified patch.
        Hide
        Brian Minchau added a comment -

        Problem fixed. I committed the fix to the main development code branch, and it will shortly make it under 2.7.1 as well.

        Show
        Brian Minchau added a comment - Problem fixed. I committed the fix to the main development code branch, and it will shortly make it under 2.7.1 as well.
        Hide
        Brian Minchau added a comment -

        Changing affected version to 2.7, and fixed version to 2.7.1

        Show
        Brian Minchau added a comment - Changing affected version to 2.7, and fixed version to 2.7.1
        Hide
        Brian Minchau added a comment -

        Would the originator of this issue please verify that this issue is fixed in the 2.7.1 release, by adding a comment to this issue, so that we can close this issue.

        A lack of response by February 1, 2008 will be taken as consent that we can close this resolved issue.

        Regards,
        Brian Minchau

        Show
        Brian Minchau added a comment - Would the originator of this issue please verify that this issue is fixed in the 2.7.1 release, by adding a comment to this issue, so that we can close this issue. A lack of response by February 1, 2008 will be taken as consent that we can close this resolved issue. Regards, Brian Minchau

          People

          • Assignee:
            Brian Minchau
            Reporter:
            Vadim Gritsenko
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development