commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Liviu Tudor <liviu.tu...@gmail.com>
Subject Re: [pool] Question / potential improvement on KeyedObjectPool
Date Mon, 30 Jul 2012 08:36:03 GMT
Thanks very much for pointing me in the right direction, Mark!
As with all development tasks, this, however, opened up new questions --
not entirely sure if these are best discussed on this list or whether
occasionally I should open up a JIRA ticket for this so more than
welcoming suggestions here.

* on the [pool2] -- I haven't investigated the eviction mechanism in
detail there, but I understand the need for a List and that's fine. My
original idea was to provide a Set with all the keys for the
KeyedObjectPool interface, as there is no need for ordering of the keys
here (same as in Map.keySet) however, I can just as well have a List and
provide a getKeys() in the KeyedObjectPool interface -- it will be used
the same ultimately for my idea, which in fact needs just a collection
(Set was preferred due to the fact that Map provides access to the keys
via a Set and the fact that keys are unique obviously). So, in the light
of that, which would be better to have in KeyedObjectPool interface (if
any):
  1/ Set<K> keySet();	//returns a Set with all the keys in the pool
  2/ List<K> getKeys(); //this is already implemented part of the Mbean
implementation in GenericKeyedObjectPool
  3/ Collection<K> getKeys();//this allows us to customise it in
subclasses, so we can have List in GenericKeyedObjectPool but perhaps
other implementation can return Set if needed

* Looking at [pool-1.6] -- there are a few problems with the code on that
branch, not sure if this was abandoned or not? The layout of the project
is a bit non-maven, but that is fine. Checkstyle throws a bunch of errors
due to misconfiguration in pom.xml (which I have easily fixed). Also there
are a bunch of FindBungs which I have fixed too. There are a few unit
tests failing which I am looking at while also trying to slot in my actual
commit (as per below) :)
So, what are the best practices here -- separate these into a bunch of
JIRA issues/patches :
  1/ checkstyle fixes
  2/ findbugs fixes
  3/ unit test fixes
  4/ my component
Or just put everything into one patch?

Thanks,


Liv 
 
Liviu Tudor
 
E: liviu.tudor@gmail.com
C: +1 650-2833815 (USA)
M: +44 (0)7591540313 (UK, Europe)
W: http://about.me/liviutudor
Skype: liviutudor
 
I'm nobody, nobody's perfect -- therefore I'm perfect!

 





On 24/07/2012 09:32, "Mark Thomas" <markt@apache.org> wrote:

>On 24/07/2012 08:42, Liviu Tudor wrote:
>> Phil & others who have been in touch with [pool],
>> 
>> A follow up to this: I'm trying to log this in JIRA and submit a patch
>>but
>> I've come across the following problem:
>> 
>> 1/ The mechanism I am referring to had pool-1.6.0 in mind, however, I
>> can't find in SVN the branch for 1.6? There are release branches going
>>up
>> to 1.5 but no 1.6? Can you point me in the right direction so I can
>>create
>> the patch in the right branch?
>
>Pool 2.x: http://svn.apache.org/repos/asf/commons/proper/pool/trunk/
>Pool 1.6:
>http://svn.apache.org/repos/asf/commons/proper/pool/branches/POOL_1_X/
>Pool 1.5:
>http://svn.apache.org/repos/asf/commons/proper/pool/branches/1_5_RELEASE/
>
>In theory pool 1.5.x + conversion to generics = pool 1.6.x
>
>Pool 2.x is a complete re-write of the pooling mechanism.
>
>> 2/ Since we've moved now to pool2, I would like to apply something
>>similar
>> for [pool2], however, looking at the code, I see we now have a
>> 
>>     List<K> getKeys();
>> 
>> Due to the Mbean interface we're implementing. This looks similar to my
>> idea of returning a Set<K> for the keys -- since the order of the keys
>>of
>> the cache is not important I thought. Looking at the code it turns out
>> indeed the order is not important, so List is not justified and Set
>>could
>> have done it just fine. So am I ok to change the Mbean and the class to
>> actually use a Set rather than a List?
>
>No. Order is important for eviction. Internally it has to be a List.
>Externally, the keys could be returned as a Set or a List. I don't have
>any strong feelings. Regardless, we should more clearly (there is a
>comment already) document the requirement for ordering of keys for
>eviction somewhere in the code since it doesn't appear to be clear enough.
>
>Mark
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>For additional commands, e-mail: dev-help@commons.apache.org
>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message