commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <>
Subject Re: [Pool2] Alternative identity hashmap pool implementations
Date Sun, 08 Feb 2015 16:57:29 GMT
On 2/8/15 8:51 AM, sebb wrote:
> On 6 February 2015 at 22:36, Phil Steitz <> wrote:
>> On 2/6/15 1:28 PM, sebb wrote:
>>> On 6 February 2015 at 19:58, Phil Steitz <> wrote:
>>>> On 2/6/15 11:56 AM, sebb wrote:
>>>>> There seem to be a few use-cases for pools that always treat different
>>>>> instances as different entries, rather than using the current equals()
>>>>> check.
>>>> Yes
>>>>> Would it make sense to implement alternative versions that accept an
>>>>> object and wrap it in a class that implements equals() using == and
>>>>> System.identityHashcode?
>>>> Yes, we should definitely support this use case.
>>>>> The IdentityWrapper class was suggested here:
>>>>> I think it could be done by subclassing the existing implementations
>>>>> to provide the wrapping/unwrapping support.
>>>>> There would be an overhead because the wrappers would need to be
>>>>> created every time an object was passed in. The wrappers are very
>>>>> small.
>>>> Once I get the DBCP release I am working on out, I plan to explore
>>>> all three alternatives in my comment on POOL-284.  The performance
>>>> regression testing tool that Thomas found and referenced on that
>>>> ticket looks very interesting and could help assessing the cost of
>>>> wrapping / unwrapping or changing the current impl to use
>>>> sync-protected IdentityHashMap.
>>> Changing to IdentityHashMap implies dropping support for
>>> equals()-equivalent pool entries.
>>> Are there no use-cases for such behaviour?
>> Good point.  We would probably want to make this configurable.
> We seem to be moving towards changing the implementations to support
> both the current equals() comparison and a new identity comparison
> (whether by wrapper or alternate hashmap implementation).
I agree we need to provide something that works for non-HashMap
compatible objects.

I am not ready to conclude though that we should change the main /
default impl until we have done some performance testing, which is
very tricky to get right.  I haven't played with the tool that
Thomas referenced; but it looks promising for this kind of thing. 
In any case,  I don't want to do anything that impacts performance
of DBCP and other clients that work fine under the Hashmap compat
> That would allow Pool2 to be used in the same way as Pool1 (with
> suitable config).
> I think that would be better than providing the identity comparing
> pools as independent PoolUtils methods.

The advantage of the separate impl approach is we can keep the
current impl simple and performant.
> Also it should make it easier to change the identity implementation at
> a later date if necessary.
> I.e. the initial impl. could use the IdentityWrapper, as that is known
> to work and is available now.
> If a bettter impl. were developed later it could replace the IdentityWrapper.

As an alternative impl, I have no problem with this; but I don't
want to add this overhead to the default unless it really can be
shown to be costless (which I doubt). 

It could be we can just do the "alternative impl" stuff behind the
scenes in config.  The main points IMO are

0) we don't want to do anything to make current performance worse
1) we need to make the contract clear
2) we should provide support for reference-based instance management
(both the use case where equals does not discern distinct objects
and where mutability causes equals to "incorrectly" discern
returning instances from references to them in allObjects).

>> Phil
>>>> If we can get a solution that "just
>>>> works" at demonstrably very low cost, I will advocate for that.  If
>>>> not, then probably what you are describing above, made available
>>>> like other custom pools via PoolUtils, makes sense.
>>>> Phil
>>>>> S.
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail:
>>>>> For additional commands, e-mail:
>>>>> .
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail:
>>>> For additional commands, e-mail:
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail:
>>> For additional commands, e-mail:
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail:
>> For additional commands, e-mail:
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message