commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [Pool2] Alternative identity hashmap pool implementations
Date Mon, 09 Feb 2015 02:17:48 GMT
On 2/8/15 5:55 PM, sebb wrote:
> On 8 February 2015 at 16:57, Phil Steitz <phil.steitz@gmail.com> wrote:
>> On 2/8/15 8:51 AM, sebb wrote:
>>> On 6 February 2015 at 22:36, Phil Steitz <phil.steitz@gmail.com> wrote:
>>>> On 2/6/15 1:28 PM, sebb wrote:
>>>>> On 6 February 2015 at 19:58, Phil Steitz <phil.steitz@gmail.com>
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:
>>>>>>>
>>>>>>> https://issues.apache.org/jira/browse/POOL-283?focusedCommentId=14307637&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14307637
>>>>>>>
>>>>>>> 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
>> requirement.
> I'm not suggesting changing the default implementation behaviour; that
> might break some existing apps.
>
>>> 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.
> Perhaps, but I imagine a lot of the code would have to be duplicated.
>
> I was thinking instead of changing the code so it uses an abstract Map.
> When the instance is created, the map type would be chosen
> accordingly, with the default as now.

Good idea.  Sorry, I was not clear.  I don't want to duplicate lots
of impl code.  The abstract Map idea might work.  I just don't want
to shove wrapping/unwrapping / extra testing / ideally anything into
the default impl.
>
>>> 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).
> The problem with an alternative impl. is the amount of code that has
> to be duplicated.
>
>> 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
> Agreed.
>
>> 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).
> Not quite sure what you mean by reference-based here.
> Is that different from the IdentityWrapper approach?

I was just trying to state the core requirement of the default
implementation - that from the pool's perspective, identity is
equality of reference.  The IdentityWrapper is one way to satisfy
this requirement. 

Phil

>
> ---------------------------------------------------------------------
> 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