You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Jeremy Boynes <jb...@apache.org> on 2010/07/09 21:51:20 UTC
[taglibs] XPath support
In light of the performance issues logged against the XML taglib and functional issues like #49578, I was looking at refactor the XML tags to use the JAXP XPath API to pre-compile expressions and dynamically resolve variables. I think this can be done fairly easily and will eliminate a lot of the integration code in XPathUtil. However, I could not see how to expose the full iteration context described in the spec for <x:forEach>:
* the context position is the iteration 'count' (with the same meaning as in <c:forEach>)
* the context size is equal to the number of nodes in the node-set over which <x:forEach> is iterating
It looks like the current implementation does not support this:
https://issues.apache.org/bugzilla/show_bug.cgi?id=22765
and in testing these functions always return -1 and 0 respectively.
Presumably these should be returned by the XPath core functions position() and last() respectively. However, the JAXP API only allows a single Node to be passed in for evaluation and I could not see a way to provide the context needed by these functions. I think this might be a limitation of JAXP.
I plan to go ahead with the refactor as I think by simplifying our implementation we will address the current performance issues and fix some of the functional edge cases. It will also remove the hard dependency on the Xalan implementation.The iteration context functions will remain broken consistent with the current implementation.
It might be possible to make this work using the low-level internal Xalan APIs as this functionality is supported in Xalan's XSLT processing. To support this in the future, I'll try to make the XPath usage pluggable so we can drop in a Xalan-specific version in the future.
Thoughts?
Jeremy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [taglibs] XPath support
Posted by Rex Wang <rw...@gmail.com>.
2010/10/3 Jeremy Boynes <jb...@apache.org>
> On Jul 15, 2010, at 12:19 AM, Henri Yandell wrote:
>
> > On Wed, Jul 14, 2010 at 8:45 PM, Jeremy Boynes <jb...@apache.org>
> wrote:
> >> On Jul 12, 2010, at 7:04 PM, Jeremy Boynes wrote:
> >>
> >>> I'm going to ping Xalan about the increase in time taken as expressions
> are evaluated as I would assume I'm doing something silly.
> >>
> >> I looked into the Xalan implementation and the problem appears to be in
> creation of the DTM used by the underlying implementation. To evaluate the
> expression it walks up the DOM tree to the parent and then iterates forward
> over the tree until it reaches the initial context node. This leads to a
> linear increase in execution time as the context node progresses through the
> NodeList from the <forEach>.
> >>
> >> I changed <x:forEach> and <x:out> to use Jaxen and did not see this
> issue. The execution time for xpath evalutation over 1000 iterations was
> constant and substantially faster than with Xalan: total of 62ms reparsing
> each time or 21ms if precompiled, down from 1800ms.
> >>
> >> In light of this I'd like to propose we switch to Jaxen and add it as a
> dependency.
> >
> > Absolutely: +1
> >
> > Great work :)
>
> I've not committed these changes as the latest version of Jaxen is not
> available in the Maven repo and there did not appear to be much interest in
> doing so. We might be able to get similar improvements using JXPath.
>
>
Could we start the release process without resolving this performance issue?
If not, the servicemix bundled jaxen
http://repo1.maven.org/maven2/org/apache/servicemix/bundles/org.apache.servicemix.bundles.jaxen/1.1.1_1/
might be a good choice as the dependency.
-Rex
> --
> Jeremy
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
--
Lei Wang (Rex)
rwonly AT apache.org
Re: [taglibs] XPath support
Posted by Jeremy Boynes <jb...@apache.org>.
On Jul 15, 2010, at 12:19 AM, Henri Yandell wrote:
> On Wed, Jul 14, 2010 at 8:45 PM, Jeremy Boynes <jb...@apache.org> wrote:
>> On Jul 12, 2010, at 7:04 PM, Jeremy Boynes wrote:
>>
>>> I'm going to ping Xalan about the increase in time taken as expressions are evaluated as I would assume I'm doing something silly.
>>
>> I looked into the Xalan implementation and the problem appears to be in creation of the DTM used by the underlying implementation. To evaluate the expression it walks up the DOM tree to the parent and then iterates forward over the tree until it reaches the initial context node. This leads to a linear increase in execution time as the context node progresses through the NodeList from the <forEach>.
>>
>> I changed <x:forEach> and <x:out> to use Jaxen and did not see this issue. The execution time for xpath evalutation over 1000 iterations was constant and substantially faster than with Xalan: total of 62ms reparsing each time or 21ms if precompiled, down from 1800ms.
>>
>> In light of this I'd like to propose we switch to Jaxen and add it as a dependency.
>
> Absolutely: +1
>
> Great work :)
I've not committed these changes as the latest version of Jaxen is not available in the Maven repo and there did not appear to be much interest in doing so. We might be able to get similar improvements using JXPath.
--
Jeremy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [taglibs] XPath support
Posted by Henri Yandell <fl...@gmail.com>.
On Wed, Jul 14, 2010 at 8:45 PM, Jeremy Boynes <jb...@apache.org> wrote:
> On Jul 12, 2010, at 7:04 PM, Jeremy Boynes wrote:
>
>> I'm going to ping Xalan about the increase in time taken as expressions are evaluated as I would assume I'm doing something silly.
>
> I looked into the Xalan implementation and the problem appears to be in creation of the DTM used by the underlying implementation. To evaluate the expression it walks up the DOM tree to the parent and then iterates forward over the tree until it reaches the initial context node. This leads to a linear increase in execution time as the context node progresses through the NodeList from the <forEach>.
>
> I changed <x:forEach> and <x:out> to use Jaxen and did not see this issue. The execution time for xpath evalutation over 1000 iterations was constant and substantially faster than with Xalan: total of 62ms reparsing each time or 21ms if precompiled, down from 1800ms.
>
> In light of this I'd like to propose we switch to Jaxen and add it as a dependency.
Absolutely: +1
Great work :)
Hen
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [taglibs] XPath support
Posted by Jeremy Boynes <jb...@apache.org>.
On Jul 12, 2010, at 7:04 PM, Jeremy Boynes wrote:
> I'm going to ping Xalan about the increase in time taken as expressions are evaluated as I would assume I'm doing something silly.
I looked into the Xalan implementation and the problem appears to be in creation of the DTM used by the underlying implementation. To evaluate the expression it walks up the DOM tree to the parent and then iterates forward over the tree until it reaches the initial context node. This leads to a linear increase in execution time as the context node progresses through the NodeList from the <forEach>.
I changed <x:forEach> and <x:out> to use Jaxen and did not see this issue. The execution time for xpath evalutation over 1000 iterations was constant and substantially faster than with Xalan: total of 62ms reparsing each time or 21ms if precompiled, down from 1800ms.
In light of this I'd like to propose we switch to Jaxen and add it as a dependency.
Thanks
Jeremy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [taglibs] XPath support
Posted by Jeremy Boynes <jb...@apache.org>.
On Jul 12, 2010, at 1:08 AM, Mark Thomas wrote:
> On 12/07/2010 06:40, Jeremy Boynes wrote:
>> Implementation seems to work but does not provide as much benefit as expected. Jasper tag pooling does not pool tags with the same attribute values so the select attribute is set every time causing recompilation. The time taken to iterate 1000 <x:out> tags drops from around 2800ms to 1800ms.
>
> Providing the same attributes with the same values are present, Jasper
> should be pooling the tags. There was a bug in this area [1] but it was
> fixed some time ago. If this isn't the case then please open a Tomcat
> bug and provide a test case.
I opened 49589 [2] to illustrate this behaviour. Most of the issues related to 38197 discuss tags being invoked multiple times before release is invoked, which is actually what is desired here with the additional optimization that constant attributes like "select" on <x:out> don't get reset. That would increase the complexity of the tag pooling though as it would need to be able to differentiate initialized instances.
>
>> To work around this I added a thread-local cache of compiled XPath expressions. This does reduce the initial time taken for the first few iterations but the time taken to evaluate the compiled expression grows from 500us to 2100us toward the end of the loop (measured with nanoTime() around the call to evaluate). There may be some issue with Xalan; the same behaviour is seen with Sun's JAXP implementation included in JDK1.6 (which is based on Xalan).
>
> Use of ThreadLocals in this way is almost certainly going to trigger
> memory leaks on web application reload.
I think it could be made to work but agree that it's a *bad idea* for a tag to be doing this and there has to be a better way.
I'm going to ping Xalan about the increase in time taken as expressions are evaluated as I would assume I'm doing something silly.
Thanks
Jeremy
[2] https://issues.apache.org/bugzilla/show_bug.cgi?id=49589
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [taglibs] XPath support
Posted by Mark Thomas <ma...@apache.org>.
On 12/07/2010 06:40, Jeremy Boynes wrote:
> Implementation seems to work but does not provide as much benefit as expected. Jasper tag pooling does not pool tags with the same attribute values so the select attribute is set every time causing recompilation. The time taken to iterate 1000 <x:out> tags drops from around 2800ms to 1800ms.
Providing the same attributes with the same values are present, Jasper
should be pooling the tags. There was a bug in this area [1] but it was
fixed some time ago. If this isn't the case then please open a Tomcat
bug and provide a test case.
> To work around this I added a thread-local cache of compiled XPath expressions. This does reduce the initial time taken for the first few iterations but the time taken to evaluate the compiled expression grows from 500us to 2100us toward the end of the loop (measured with nanoTime() around the call to evaluate). There may be some issue with Xalan; the same behaviour is seen with Sun's JAXP implementation included in JDK1.6 (which is based on Xalan).
Use of ThreadLocals in this way is almost certainly going to trigger
memory leaks on web application reload.
> The same slowdown is seen if the expression is evaluated each time, or if the xpath is run in a standalone testcase outside taglibs entirely. There may be an issue here with Xalan and/or the JDK.
>
> We might be advised to consider a different XPath implementation. Does anyone have any thoughts on Jaxen?
Sorry, no idea on this.
Mark
[1] https://issues.apache.org/bugzilla/show_bug.cgi?id=38197
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [taglibs] XPath support
Posted by Jeremy Boynes <jb...@apache.org>.
Implementation seems to work but does not provide as much benefit as expected. Jasper tag pooling does not pool tags with the same attribute values so the select attribute is set every time causing recompilation. The time taken to iterate 1000 <x:out> tags drops from around 2800ms to 1800ms.
To work around this I added a thread-local cache of compiled XPath expressions. This does reduce the initial time taken for the first few iterations but the time taken to evaluate the compiled expression grows from 500us to 2100us toward the end of the loop (measured with nanoTime() around the call to evaluate). There may be some issue with Xalan; the same behaviour is seen with Sun's JAXP implementation included in JDK1.6 (which is based on Xalan).
The same slowdown is seen if the expression is evaluated each time, or if the xpath is run in a standalone testcase outside taglibs entirely. There may be an issue here with Xalan and/or the JDK.
We might be advised to consider a different XPath implementation. Does anyone have any thoughts on Jaxen?
Thanks
Jeremy
On Jul 10, 2010, at 10:38 PM, Jeremy Boynes wrote:
> I've added two attachments to bug 27717
> https://issues.apache.org/bugzilla/show_bug.cgi?id=27717
> that show ExprSupport updated to use the JAXP XPath API to precompile expressions and an implementation of an XPathVariableResolver that handles the JSTL variable resolution scheme.
>
> There are very few tests for the XML tags so I want to add more to check for regressions and that may take a while. Comments on the approach would be appreciated before I start on the other tags. I'll post some performance numbers once I've updated <x:forEach>
>
> Cheers
> Jeremy
>
> On Jul 9, 2010, at 9:56 PM, Henri Yandell wrote:
>
>> On Fri, Jul 9, 2010 at 12:51 PM, Jeremy Boynes <jb...@apache.org> wrote:
>>> In light of the performance issues logged against the XML taglib and functional issues like #49578, I was looking at refactor the XML tags to use the JAXP XPath API to pre-compile expressions and dynamically resolve variables. I think this can be done fairly easily and will eliminate a lot of the integration code in XPathUtil. However, I could not see how to expose the full iteration context described in the spec for <x:forEach>:
>>>
>>> * the context position is the iteration 'count' (with the same meaning as in <c:forEach>)
>>> * the context size is equal to the number of nodes in the node-set over which <x:forEach> is iterating
>>>
>>> It looks like the current implementation does not support this:
>>> https://issues.apache.org/bugzilla/show_bug.cgi?id=22765
>>> and in testing these functions always return -1 and 0 respectively.
>>>
>>> Presumably these should be returned by the XPath core functions position() and last() respectively. However, the JAXP API only allows a single Node to be passed in for evaluation and I could not see a way to provide the context needed by these functions. I think this might be a limitation of JAXP.
>>>
>>> I plan to go ahead with the refactor as I think by simplifying our implementation we will address the current performance issues and fix some of the functional edge cases. It will also remove the hard dependency on the Xalan implementation.The iteration context functions will remain broken consistent with the current implementation.
>>>
>>> It might be possible to make this work using the low-level internal Xalan APIs as this functionality is supported in Xalan's XSLT processing. To support this in the future, I'll try to make the XPath usage pluggable so we can drop in a Xalan-specific version in the future.
>>>
>>> Thoughts?
>>
>> +1. Resolving the speed and some of the issues is worth making it
>> harder to resolve the other issues imo.
>>
>> Hen
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [taglibs] XPath support
Posted by Jeremy Boynes <jb...@apache.org>.
I've added two attachments to bug 27717
https://issues.apache.org/bugzilla/show_bug.cgi?id=27717
that show ExprSupport updated to use the JAXP XPath API to precompile expressions and an implementation of an XPathVariableResolver that handles the JSTL variable resolution scheme.
There are very few tests for the XML tags so I want to add more to check for regressions and that may take a while. Comments on the approach would be appreciated before I start on the other tags. I'll post some performance numbers once I've updated <x:forEach>
Cheers
Jeremy
On Jul 9, 2010, at 9:56 PM, Henri Yandell wrote:
> On Fri, Jul 9, 2010 at 12:51 PM, Jeremy Boynes <jb...@apache.org> wrote:
>> In light of the performance issues logged against the XML taglib and functional issues like #49578, I was looking at refactor the XML tags to use the JAXP XPath API to pre-compile expressions and dynamically resolve variables. I think this can be done fairly easily and will eliminate a lot of the integration code in XPathUtil. However, I could not see how to expose the full iteration context described in the spec for <x:forEach>:
>>
>> * the context position is the iteration 'count' (with the same meaning as in <c:forEach>)
>> * the context size is equal to the number of nodes in the node-set over which <x:forEach> is iterating
>>
>> It looks like the current implementation does not support this:
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=22765
>> and in testing these functions always return -1 and 0 respectively.
>>
>> Presumably these should be returned by the XPath core functions position() and last() respectively. However, the JAXP API only allows a single Node to be passed in for evaluation and I could not see a way to provide the context needed by these functions. I think this might be a limitation of JAXP.
>>
>> I plan to go ahead with the refactor as I think by simplifying our implementation we will address the current performance issues and fix some of the functional edge cases. It will also remove the hard dependency on the Xalan implementation.The iteration context functions will remain broken consistent with the current implementation.
>>
>> It might be possible to make this work using the low-level internal Xalan APIs as this functionality is supported in Xalan's XSLT processing. To support this in the future, I'll try to make the XPath usage pluggable so we can drop in a Xalan-specific version in the future.
>>
>> Thoughts?
>
> +1. Resolving the speed and some of the issues is worth making it
> harder to resolve the other issues imo.
>
> Hen
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [taglibs] XPath support
Posted by Henri Yandell <fl...@gmail.com>.
On Fri, Jul 9, 2010 at 12:51 PM, Jeremy Boynes <jb...@apache.org> wrote:
> In light of the performance issues logged against the XML taglib and functional issues like #49578, I was looking at refactor the XML tags to use the JAXP XPath API to pre-compile expressions and dynamically resolve variables. I think this can be done fairly easily and will eliminate a lot of the integration code in XPathUtil. However, I could not see how to expose the full iteration context described in the spec for <x:forEach>:
>
> * the context position is the iteration 'count' (with the same meaning as in <c:forEach>)
> * the context size is equal to the number of nodes in the node-set over which <x:forEach> is iterating
>
> It looks like the current implementation does not support this:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=22765
> and in testing these functions always return -1 and 0 respectively.
>
> Presumably these should be returned by the XPath core functions position() and last() respectively. However, the JAXP API only allows a single Node to be passed in for evaluation and I could not see a way to provide the context needed by these functions. I think this might be a limitation of JAXP.
>
> I plan to go ahead with the refactor as I think by simplifying our implementation we will address the current performance issues and fix some of the functional edge cases. It will also remove the hard dependency on the Xalan implementation.The iteration context functions will remain broken consistent with the current implementation.
>
> It might be possible to make this work using the low-level internal Xalan APIs as this functionality is supported in Xalan's XSLT processing. To support this in the future, I'll try to make the XPath usage pluggable so we can drop in a Xalan-specific version in the future.
>
> Thoughts?
+1. Resolving the speed and some of the issues is worth making it
harder to resolve the other issues imo.
Hen
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org