Hi again guys, just realized that pasted code is not so readable, I just copied it on pastie[1]. If you don't have any objections, I can start committing that stuff so you can review it. Many thanks in advance, Simo
[1] http://pastie.org/1411379 http://people.apache.org/~simonetripodi/ http://www.99soft.org/ On Sun, Dec 26, 2010 at 1:41 PM, Simone Tripodi <simone.trip...@gmail.com> wrote: > Hi Phil, > that's the way how I propose to synchronize pools using proxies > without implementing synchronized wrapper classes, if you don't see > issues about it I can start committing it let you all reviewing. > Please let me know, have a nice day!!! > Simo > > {{{ > > public static <T> ObjectPool<T> synchronizedPool(final ObjectPool<T> pool) > { > return synchronizedObject(pool, ObjectPool.class); > } > > public static <K,V> KeyedObjectPool<K,V> synchronizedPool(final > KeyedObjectPool<K,V> keyedPool) { > return synchronizedObject(keyedPool, KeyedObjectPool.class); > } > > public static <T> PoolableObjectFactory<T> > synchronizedPoolableFactory(final PoolableObjectFactory<T> factory) { > return synchronizedObject(factory, PoolableObjectFactory.class); > } > > public static <K,V> KeyedPoolableObjectFactory<K,V> > synchronizedPoolableFactory(final KeyedPoolableObjectFactory<K,V> > keyedFactory) { > return synchronizedObject(keyedFactory, > KeyedPoolableObjectFactory.class); > } > > private static <T> T synchronizedObject(final T toBeSynchronized, > final Class<T> type) { > if (toBeSynchronized == null) { > throw new IllegalArgumentException("Object has to be > synchronized must not be null."); > } > > /** > * Used to synchronize method declared on the pool/factory > interface only. > */ > final Set<Method> synchronizedMethods = new HashSet<Method>(); > for (Method method : type.getDeclaredMethods()) { > synchronizedMethods.add(method); > } > > return type.cast(Proxy.newProxyInstance(type.getClassLoader(), > new Class<?>[] { type }, > new InvocationHandler() { > > private final Object lock = new Object(); > > public Object invoke(Object proxy, Method method, > Object[] args) throws Throwable { > if (synchronizedMethods.contains(method)) { > synchronized (this.lock) { > return method.invoke(toBeSynchronized, args); > } > } > return method.invoke(toBeSynchronized, args); > } > > } > )); > } > > }}} > > http://people.apache.org/~simonetripodi/ > http://www.99soft.org/ > > > > On Fri, Dec 24, 2010 at 3:32 PM, Simone Tripodi > <simone.trip...@gmail.com> wrote: >> Hi Phil, >> thanks a lot for feedbacks and for mentoring, much more than appreciated. >> I take advantage to wish you all a Merry Christmas, all the best, >> Simo >> >> http://people.apache.org/~simonetripodi/ >> http://www.99soft.org/ >> >> >> >> On Fri, Dec 24, 2010 at 4:32 AM, Phil Steitz <phil.ste...@gmail.com> wrote: >>> On Thu, Dec 23, 2010 at 2:28 AM, Simone Tripodi >>> <simone.trip...@gmail.com>wrote: >>> >>>> Hi Phil, >>>> >>>> >> >>>> >> org.apache.commons.pool2.impl >>>> >> |---- generic >>>> >> |---- reference >>>> >> |---- stack >>>> >> >>>> >> common stuff could be included directly under impl. >>>> >> >>>> > >>>> > What exactly would that be? >>>> > >>>> >>>> just realized that there are no common stuff shared by different kind of >>>> pool :P >>>> >>>> > >>>> > I was going to propose dropping the stack pools altogether. The >>>> LIFO/FIFO >>>> > config option in the generic pools makes them mostly irrelevant (i.e., >>>> you >>>> > can get the same behavior with a suitably configured GOP / GKOP with much >>>> > more configurability) >>>> > >>>> >>>> I had the same feelings here, but felt a little shy on saying that. >>>> >>> >>> No reason for that :) >>> >>> >>>> You've my full support on this, I agree on dropping stack based pool >>>> implementations. >>>> >>>> Good. Lets do it. >>> >>> >>>> > I don't want to sound too conservative and I will certainly not stand in >>>> the >>>> > way of new / different pool implementations, but I would personally >>>> prefer >>>> > to keep the number of included pool impls as small as possible. >>>> > >>>> >>>> I propose a more democratic way, I mean, like you made us notice, >>>> keeping/adding the pool impl only if it makes sense to. >>>> I wouldn't think about the included pools in therms of "size" but >>>> rather in therms of "meaning" >>>> >>> >>> Agree this is completely up to the community and them who bring the >>> patches. The reason that I tend to be conservative is that more impls mean >>> more bugs, which makes fixing them all and getting releases out quickly >>> harder. Pooling code is tricky to maintain, so fewer pools with >>> less-exotic features might be better all around - not just in terms of >>> maintenance, but also approachability from the standpoint of users and >>> contributors. >>> >>>> >>>> > I think the first thing we need to do is to decide what implementations >>>> we >>>> > are going to a) keep or b) add for 2.0. I have been convinced that we >>>> need >>>> > to keep GKOP as well as GOP. As I said above, I would like to consider >>>> > dropping the stack-based pools. I think we should keep the >>>> reference-based >>>> > pool and I am open to the new ones you suggest, just don't have use cases >>>> > myself for them. >>>> >>>> I'm not ready to show use cases too, sorry :( But they can be added >>>> with a trivial refactory, moving the current SoftReferenceObjectPool >>>> implementation to an >>>> >>>> AbstractReferenceObjectPool<T, R extends Reference<T>> extends >>>> BaseObjectPool<T> implements ObjectPool<T> >>>> >>>> then in subclasses do the minimum. I can quickly provide a patch for it. >>>> >>> >>> I am OK with this as long as it does not complicate the code too much. >>> >>>> >>>> > There are quite a few impls buried in PoolUtils that might >>>> > make sense to pull out (or eliminate). >>>> > >>>> >>>> I just made a census (with proposals): >>>> >>>> * PoolableObjectFactoryAdaptor >>>> * KeyedPoolableObjectFactoryAdaptor >>>> * ObjectPoolAdaptor >>>> * KeyedObjectPoolAdaptor >>>> >>>> I propose to eliminate these adaptors, >>> >>> >>> That's the spirit - he he >>> >>> >>>> they implement a behavior that >>>> users can replicate with a trivial code and without build a >>>> pool/factory on top of an existing ones >>>> >>>> * CheckedObjectPool >>>> * CheckedKeyedObjectPool >>>> >>>> These can be eliminated too, having introduced the generics >>>> >>> >>> +1 - I think Gary may have mentioned this already. Lets get rid of them. >>> >>>> >>>> * ObjectPoolMinIdleTimerTask >>>> * KeyedObjectPoolMinIdleTimerTask >>>> >>>> I propose these pools can be pulled out and moved to a proper package >>>> >>>> I wonder if anyone uses these? I wonder also if it might be better to just >>> expose ensureMinIdle. >>> >>> >>>> * SynchronizedObjectPool >>>> * SynchronizedKeyedObjectPool >>>> * SynchronizedPoolableObjectFactory >>>> * SynchronizedKeyedPoolableObjectFactory >>>> >>>> These could be pulled out too, even if something suggests me that >>>> pools synchronization can be realized with just a Proxy, I'll do a >>>> little experiment to submit so you can evaluate. >>>> >>> >>> Interested in ideas on this. Here again, I am not sure how much use these >>> actually get. >>> >>>> >>>> > What might make sense is to replace "impl" with "instance" (or "object") >>>> and >>>> > "reference" (or "ref"). So you have o.a.c.p, o.a.c.p.instance, >>>> > o.a.c.p.reference. >>>> > >>>> >>>> sounds much better than keeping the intermediate "impl", I agree :) >>>> >>>> Should I have to write all these notes on the wiki and open issues >>>> before proceeding? >>>> >>> >>> I would say wait a bit to see if anyone has problems with above and if not, >>> go ahead and make the changes. >>> >>> >>>> Many thanks in advance, have a nice day! >>>> >>> >>> Thank you! >>> >>> Phil >>> >>>> Simo >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>> >>>> >>> >> > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org