tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Williams <nicho...@nicholaswilliams.net>
Subject Re: [Bug 55317] Facilitate weaving by allowing ClassFileTransformer to be added to WebppClassLoader
Date Thu, 12 Sep 2013 21:26:29 GMT

On Aug 29, 2013, at 4:28 AM, bugzilla@apache.org wrote:

> https://issues.apache.org/bugzilla/show_bug.cgi?id=55317
> 
> --- Comment #16 from Mark Thomas <markt@apache.org> ---
> (In reply to Nick Williams from comment #15)
>> Created attachment 30749 [details]
>> Proposed implementation of this feature
> 
> 1. Why loop over list rather than using contains() in addTransformer() ?
> 
> 2. Should addTransformer() be looking for multiple instances of the same
> Transformer or multiple instances of the same class of Transformer?
> 
> 3. Why not use List.remove(Object) in removeTransformer() ?
> 
> 4. I'm concerned that synchronizing on the list of transformers while classes
> are transformed will become a bottleneck when lots of classes are being loaded
> and the transformer is relatively slow. A separate ReadWriteLock for the
> transformer list is probably the way to go but really some testing is required
> to determine if there is an issue here or not.
> 
> 5. I'm not a fan of the org.apache.tomcat.unittest package unless the classes
> concerned are going to be used by multiple tests across multiple packages.

FYI, I replied to your comments and submitted a revised patch. It doesn't appear that Bugzilla
sent out an email this time. Not sure what happened, but I wanted to make sure you were notified
somehow so that you could take a look.

Nick
Mime
View raw message