On 12 December 2011 21:07, Phil Steitz <phil.ste...@gmail.com> wrote: > On 12/12/11 1:15 PM, sebb wrote: >> On 12 December 2011 19:42, Phil Steitz <phil.ste...@gmail.com> wrote: >>> On 12/12/11 12:29 PM, Simone Tripodi wrote: >>>> Hi Phil, >>>> thanks for the feedbacks! I would put some energies on [pool2] since I >>>> am currently in the situation I need it - of course in the meantime I >>>> can work with snapshots, but having a release would be definitively >>>> better :) >>>> >>>> So I start filling an issue and update synchronized pools in >>>> PoolUtils, then IMHO we should back speaking about implementing >>>> Read/Write policy in Pools implementation. Thoughts? >>> Personally, I think the 2.0 implementations in GKOP and GOP are good >>> and would prefer to focus on making the final preparations for >>> release rather than reworking the - very tricky - locking and >>> synchronization semantics in those implementations. >> +1 >> >>> That said, if >>> you or anyone else has better ideas, we can certainly look at them. >>> Performance and intensive concurrency testing of the code in trunk >>> would also be very helpful. >> Probably also useful to work through documenting which classes are >> intended to be thread-safe, immutable, etc, and then checking that >> they are ... >> >> For example, it looks as though GenericKeyedObjectPool is intended to >> be thread-safe (but this is not yet documented). >> However, the boolean field _lifo is not volatile, and the >> setter/getter are not synchronized. >> This means changes to the field are not guaranteed to be safely published. >> Of course that's not a problem if GKOP is not intended to be thread-safe. > > GKOP should be threadsafe.
OK. > Probably _lifo should be made final. OK, the setter could be removed then, as lifo is part of the ctor init. However that was just an example picked at random - there are lots of other similar setters, including a setConfig() that sets them all ... I think GKOP should be largely immutable - does it make sense to change the behaviour once it has been instantiated? IIRC, the idea of the Config class was to hold all the various settings, and then the ctor would save them in final variables. However at present, the settings are saved in mutable variables, and are also settable individually. This does not make sense to me. Given that the setters are currently part of the public API, I think this needs to be addressed before 2.0 can be released. Exactly the same issues apply to GenericObjectPool. Also, what about GenericKeyedObjectPoolFactory ? That also has setters for the fields that are already set via the ctor. Very flexible, but harder to test thoroughly and not thread-safe as it stands. Given that Pool2 is a new API and does not need to maintain compatibilty, I think it would make sense to start with the smallest possible public API and only expand as necessary. So I would favour making almost all fields final. Any fields that need to be mutable must be private. Final fields should also be private unless they are constants. > Phil >> >>> Phil >>>> -Simo >>>> >>>> http://people.apache.org/~simonetripodi/ >>>> http://simonetripodi.livejournal.com/ >>>> http://twitter.com/simonetripodi >>>> http://www.99soft.org/ >>>> >>>> >>>> >>>> On Mon, Dec 12, 2011 at 7:21 PM, Phil Steitz <phil.ste...@gmail.com> wrote: >>>>> On 12/12/11 10:56 AM, Phil Steitz wrote: >>>>>> On 12/12/11 10:26 AM, Simone Tripodi wrote: >>>>>>> Hi all guys, >>>>>>> time ago we spoke about replacing the synchronized blocks inside >>>>>>> pools, maybe using different strategies like Java5 Read/Write lock (I >>>>>>> remind you Pool2 requires Java5) and I just started playing with >>>>>>> PoolUtils with the SynchronizedObjectPool[1] inner class... >>>>>>> Can we discuss about introducing such modifications inside pool >>>>>>> implementations? >>>>>> I would say go for it in PoolUtils. Assuming we keep these pools, >>>>>> we should update them as we have GOP, GKOP. Regarding the pool you >>>>>> mention specifically, it is a little funny in that it is designed to >>>>>> be fully synchronized. Make sure that whatever implementation >>>>>> changes you propose maintain the contract. >>>>> Looking at the github code, I think there may be a problem. Borrow >>>>> is as much a "write" operation as return - both modify the state of >>>>> the pool. >>>>> >>>>> Phil >>>>>> Phil >>>>>>> Many thanks in advance, all the best! >>>>>>> -Simo >>>>>>> >>>>>>> [1] https://gist.github.com/1468252 >>>>>>> >>>>>>> http://people.apache.org/~simonetripodi/ >>>>>>> http://simonetripodi.livejournal.com/ >>>>>>> http://twitter.com/simonetripodi >>>>>>> http://www.99soft.org/ >>>>>>> >>>>>>> --------------------------------------------------------------------- >>>>>>> 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 >>>>> >>>> --------------------------------------------------------------------- >>>> 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 >>> >> --------------------------------------------------------------------- >> 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
