[PATCH] Fun with tag pools

[Casey Lucas <cl...@armassolutions.com>]

Ok, here's the patch.  The attached files apply to tc 3.3.  They 
allow tag handler pooling per the jsp spec.  The files come with
a few questions and comments.

Patch info:
----------
TagPoolManagerInterceptor.java should be placed in
src/facade22/org/apache/tomcat/facade

TagPoolManager.java
TagPoolManagerImpl.java
TagHandlerPool.java
TagHandlerPoolImpl.java
should be placed in src/share/org/apache/jasper/runtime

TagPoolManagerGenerator.java
TagPoolGenerator.java
should be placed in src/share/org/apache/jasper/runtime

The others are diffs.


Questions:
---------
1. Each web application wide TagPoolManager is stored in application scope
as an attribute.  Of course this means that an application could change it.
Is this acceptable?

2. Tag handlers are ALWAYS returned to the tag pool.  Even in the case of
exceptions, tag handlers will be reused.  Should tags be removed from the
pool if an exception is thrown during the normal set of tag calls (doStartTag,
doInitBody, etc.) ?


Caveats:
-------
1. Tag reuse has some important implications for the tag developer.
In general, the handler must be repeatedly usable without calls
to Tag.release.  For example, handlers should be sure to reset
certain variables to a well known state in doEndTag.  Only those
variables that change during the processing of the tag need to be
reset.  Those set via attribute setters (and not changed) are ok.

Note that some of the tags in the watchdog tests do not support
reuse.  So don't enable pooling for watchdog tests.

2. Because handlers are always returned to the pool, exceptions
may cause handlers to miss doEndTag and therefore possibly miss
cleanup / state reset.  The new TryCatchFinally interface fixes
this problem, but it's not available in jsp 1.1.


Comments would be appreciated (and would help my motivation toward
a 4.x patch :)  )

-Casey

Re: [PATCH] Fun with tag pools

["Craig R. McClanahan" <cr...@apache.org>]

On Sun, 18 Mar 2001, Casey Lucas wrote:

> [snip]
> I was thinking of something like:
> 
> If an exception is thrown (from any code, not necessarily the tag) during
> the scope of a tag handler's use (between doStartTag and doEndTag), then
> that tag should be removed from the pool because its state can not be
> guaranteed.
> 

Thinking more about it, this is definitely the safer approach.  The
scenario I described earlier is mostly a development-time issue
anyway; tag instances should throw exceptions rarely in a production
environment so the "cost" of not recycling them is pretty low.

> Of course this is only a Tomcat 3.x issue because JSP 1.2 introduces the
> TryCatchFinally interface.  If a tag must perform state maintenance at
> the end of its use, then it will just use the doFinally method.  By using
> doFinally, the container does not have to make any assumptions about
> a tag handlers possible reuse -- the tag must ensure its ability to
> be reused.
> 

Even in JSP 1.2, it still matters, because we are required to run a JSP
1.1 application (which cannot use TryCatchFinally).  So, the same
reasoning applies.

> -Casey
> 

Craig

Re: [PATCH] Fun with tag pools

[Casey Lucas <cl...@armassolutions.com>]

Thanks for the comments.  See my additional comments within.

"Craig R. McClanahan" wrote:
> 
> Casey, I haven't looked at the details but the concept looks really
> great!  I'm looking forward to a corresponding implementation for 4.0.
> 
> A couple of interspersed comments below:
> 
> On Sun, 18 Mar 2001, Casey Lucas wrote:
> 

snip

> >
> > Questions:
> > ---------
> > 1. Each web application wide TagPoolManager is stored in application scope
> > as an attribute.  Of course this means that an application could change it.
> > Is this acceptable?
> >
> 
> We're doing that kind of thing in 4.0 today with the classpath and class
> loader being passed from Catalina.  The good news:  it works.  The bad
> news:  it exposes information to change that should not really be
> changeable.
> 
> The other alternative would be some sort of global static variable that
> Jasper knows about -- at least in 4.0 each webapp's Jasper is loaded by a
> separate class loader, so statics would be unique.
> 

Yea, I threw around that idea, but had some trouble (confusion) with class
loaders in 3.3.  The interceptor would set the static, but it wasn't visible
to the jsp.  Once I got the application scope attribute working I stayed with
that.  I prefer the JspFactory.get(set)DefaultFactory() style and will
probably revisit it.

> > 2. Tag handlers are ALWAYS returned to the tag pool.  Even in the case of
> > exceptions, tag handlers will be reused.  Should tags be removed from the
> > pool if an exception is thrown during the normal set of tag calls (doStartTag,
> > doInitBody, etc.) ?
> >
> 
> I believe we should plan on reuse, even if exceptions are thrown by this
> tag itself.
> 
> For example, in Struts I have tags that check for valid combinations of
> attributes being set in the doStartTag() method (you need either "x" or
> "y"; if you use "a" you must also use "b"; that sort of thing) and throw
> an exception if the condition is violated.  There's nothing wrong with the
> tag instance itself - it was just used wrong in the page.
> 
> >
> > Caveats:
> > -------
> > 1. Tag reuse has some important implications for the tag developer.
> > In general, the handler must be repeatedly usable without calls
> > to Tag.release.  For example, handlers should be sure to reset
> > certain variables to a well known state in doEndTag.  Only those
> > variables that change during the processing of the tag need to be
> > reset.  Those set via attribute setters (and not changed) are ok.
> >
> 
> Even if you are running under JSP 1.1 (i.e. Tomcat 3.x), tag developers
> will find it useful to review the JSP 1.2 Proposed Final Draft spec.  It
> includes lifecycle diagrams on a tag that identify precisely when the
> container will call which methods.
> 
> > Note that some of the tags in the watchdog tests do not support
> > reuse.  So don't enable pooling for watchdog tests.
> >
> 
> Better would be to fix the tests so they work correctly.  At least on the
> Watchdog 4.0 tests, I can probably acquire some resources to help us get
> these right.
> 
> > 2. Because handlers are always returned to the pool, exceptions
> > may cause handlers to miss doEndTag and therefore possibly miss
> > cleanup / state reset.  The new TryCatchFinally interface fixes
> > this problem, but it's not available in jsp 1.1.
> >
> 
> How about a policy where you don't recycle any tag instances *after* the
> one that threw the exception?  Would that cover all the possible cases?
>

I was thinking of something like:

If an exception is thrown (from any code, not necessarily the tag) during
the scope of a tag handler's use (between doStartTag and doEndTag), then
that tag should be removed from the pool because its state can not be
guaranteed.

Of course this is only a Tomcat 3.x issue because JSP 1.2 introduces the
TryCatchFinally interface.  If a tag must perform state maintenance at
the end of its use, then it will just use the doFinally method.  By using
doFinally, the container does not have to make any assumptions about
a tag handlers possible reuse -- the tag must ensure its ability to
be reused.

-Casey

Re: [PATCH] Fun with tag pools

["Craig R. McClanahan" <cr...@apache.org>]

Casey, I haven't looked at the details but the concept looks really
great!  I'm looking forward to a corresponding implementation for 4.0.

A couple of interspersed comments below:

On Sun, 18 Mar 2001, Casey Lucas wrote:

> 
> Ok, here's the patch.  The attached files apply to tc 3.3.  They 
> allow tag handler pooling per the jsp spec.  The files come with
> a few questions and comments.
> 
> Patch info:
> ----------
> TagPoolManagerInterceptor.java should be placed in
> src/facade22/org/apache/tomcat/facade
> 
> TagPoolManager.java
> TagPoolManagerImpl.java
> TagHandlerPool.java
> TagHandlerPoolImpl.java
> should be placed in src/share/org/apache/jasper/runtime
> 
> TagPoolManagerGenerator.java
> TagPoolGenerator.java
> should be placed in src/share/org/apache/jasper/runtime
> 
> The others are diffs.
> 
> 
> Questions:
> ---------
> 1. Each web application wide TagPoolManager is stored in application scope
> as an attribute.  Of course this means that an application could change it.
> Is this acceptable?
> 

We're doing that kind of thing in 4.0 today with the classpath and class
loader being passed from Catalina.  The good news:  it works.  The bad
news:  it exposes information to change that should not really be
changeable.

The other alternative would be some sort of global static variable that
Jasper knows about -- at least in 4.0 each webapp's Jasper is loaded by a
separate class loader, so statics would be unique.

> 2. Tag handlers are ALWAYS returned to the tag pool.  Even in the case of
> exceptions, tag handlers will be reused.  Should tags be removed from the
> pool if an exception is thrown during the normal set of tag calls (doStartTag,
> doInitBody, etc.) ?
> 

I believe we should plan on reuse, even if exceptions are thrown by this
tag itself.

For example, in Struts I have tags that check for valid combinations of
attributes being set in the doStartTag() method (you need either "x" or
"y"; if you use "a" you must also use "b"; that sort of thing) and throw
an exception if the condition is violated.  There's nothing wrong with the
tag instance itself - it was just used wrong in the page.

> 
> Caveats:
> -------
> 1. Tag reuse has some important implications for the tag developer.
> In general, the handler must be repeatedly usable without calls
> to Tag.release.  For example, handlers should be sure to reset
> certain variables to a well known state in doEndTag.  Only those
> variables that change during the processing of the tag need to be
> reset.  Those set via attribute setters (and not changed) are ok.
> 

Even if you are running under JSP 1.1 (i.e. Tomcat 3.x), tag developers
will find it useful to review the JSP 1.2 Proposed Final Draft spec.  It
includes lifecycle diagrams on a tag that identify precisely when the
container will call which methods.

> Note that some of the tags in the watchdog tests do not support
> reuse.  So don't enable pooling for watchdog tests.
> 

Better would be to fix the tests so they work correctly.  At least on the
Watchdog 4.0 tests, I can probably acquire some resources to help us get
these right.

> 2. Because handlers are always returned to the pool, exceptions
> may cause handlers to miss doEndTag and therefore possibly miss
> cleanup / state reset.  The new TryCatchFinally interface fixes
> this problem, but it's not available in jsp 1.1.
> 

How about a policy where you don't recycle any tag instances *after* the
one that threw the exception?  Would that cover all the possible cases?

> 
> Comments would be appreciated (and would help my motivation toward
> a 4.x patch :)  )
> 

Yes please :-)

> -Casey

Craig McClanahan