tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: Context switch integration
Date Wed, 22 Jan 2014 20:48:34 GMT
Mark,

On 1/22/14, 5:21 AM, Mark Thomas wrote:
> On 22/01/2014 09:42, Konstantin Kolinko wrote:
>> 2014/1/22 Mark Thomas <markt@apache.org>:
> 
> <snip/>
> 
>>> I've been looking at this some more with an eye to reducing code
>>> duplication. I think the thread binding listener could be merged with
>>> the ContainerListener. I can't see a good reason to keep them apart at
>>> this point.
>>
>> 1. If there are many ContainerListeners installed you will be calling
>> them with an event that they are not interested to handle. It wastes
>> time.
> 
> Fair point. This is something that gets called several times on every
> request. That probably makes the case for a separate listener.
> 
>>
>>> With this in mind, the changes I'm planning are:
>>>
>>> - Add bindThread() and unbindThread() to the Context interface
>>
>> Do you mean as a generic replacement to Thread.setContextClassLoader() calls?
>> I think the current methods of StandardContext are not suitable for
>> such exposure.
> 
> Sort of, with some refactoring because of the JNDI aspects you
> mentioned. My main aim is to reduce code duplication.
> 
>>> - Add optional PA (as in callers can opt to use it if they wish)
>>>   support to bindThread() and unbindThread()
>>
>> What do you mean by "PA"?
> 
> PrivilegedAction
> 
>>> - Switch all the components currently implementing their own
>>>   bind/unbind methods to use these new context methods
>>> - Switch container event type to use an enum
>>
>> Enums are better than Strings for performance,
>> but I am not sure whether those can be extended if needed.
>>
>> I'd prefer to keep Strings here.
> 
> Do we need to extend these? This is all internal to Tomcat. We can add
> values to the enum as easily as we can use new String values.
> 
>>> - Add bind and unbind events to this new enum
>>> - Remove ThreadBindingListener
>>>
>>> The main benefit of this is that all the actions required on bind and
>>> unbind will be in a single place rather than duplicated (sometimes
>>> inconsistently) across the code base.
> 
> This is really what I am aiming at. There are several places across the
> code that do call near enough exactly the code to bind and unbind the
> context class loader. It is this duplication I want to remove.
> 
> I'll start again and address my primary concern - the duplicate code.
> The other aspects I'm happy to think about / discuss further with a view
> to possibly coming back to them later.

Could this be done more simply by providing a pair of utility methods to
do the enter/exit work rather than treating them as events to be
"handled"? That way, they can be invoked directly by components that
need them and ignored by those that don't.

-chris


Mime
View raw message