Hiya, just getting to read this. To shed a little extra light here: You
aren't quite correct, caching tags is actually always the default. It's
needed for several tag libraries to work.

Now, the caching is supposed to be per-thread. Tags are cached in the
TagScript that instantiates the Tag object. The parser is supposed to create
one TagScript each time it finds an XML tag that is resolved by a
TagLibrary. There is supposed to be one TagScript instance per Tag, and per
XML tag.

The old code used a ThreadLocal to cache the tags. The memory leak was
caused by this ThreadLocal, all I did to fix it was to change from a
ThreadLocal to a WeakHashMap from a Thread to a Tag object. The map is local
to the TagScript. So, there should be a unique XML tag, a unique TagScript,
and so, a unique Map that caches the Tag objects .

What I cannot figure out is: why are multiple XML tags somehow sharing the
same map that caches Tag objects?

-----Original Message-----
From: peter royal [mailto:[EMAIL PROTECTED] 
Sent: Thursday, July 21, 2005 5:13 AM
To: Jakarta Commons Developers List
Subject: Re: [jelly] 1.0-final problem wrt tag caching

On Jul 21, 2005, at 9:10 AM, Paul Libbrecht wrote:
> I am sorry I wasn't able to take this further yet.
> The idea was to introduce something like a reset() method on tags  
> to ensure that nullity... but if you think we need to disable  
> caching then we'll have to need a form of method like  
> "doesNeedCaching" which would then cache in any cases. I know for  
> example such things for Swing or Define tags (but not per-class).
>
> I still think Cache.reset() is better and Kristofer accept it as an  
> alternative. Would you ?

Since not caching tags was the default previously, I think we should  
revert to that (The difference between revisions 136360 and 136390 in  
svn).

It would be re-introducing JellyContext.isCacheTags(), but keeping  
the memory leak fix (iirc, that was only removed because we thought  
we could fix the memory leak by moving tag caching in the JellyContext?)

I just committed a unit test that illustrates the problem. Any  
solution that doesn't require modification of the unit test is fine  
with me (as I believe it represents the assumptions that Tag authors  
have made, and it would be very onerous to make all users validate  
their applications to determine where caching is *not* needed).
-pete

-- 
peter royal


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to