Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (nike.apache.org: domain of sebbaz@gmail.com designates
 209.85.220.176 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <54D79579.9030805@gmail.com>
References: 
 <CAOGo0VYN_L5qWcXhP2SRgbHvk1kRARAg8GDD+BiAyOfKc=kuKA@mail.gmail.com>
	<54D51CCE.40306@gmail.com>
	<CAOGo0VafjWF5ZSjVMRidZPK2PqtbyoaVAk+ai-PAvDmb2BOQQg@mail.gmail.com>
	<54D541F1.3040601@gmail.com>
	<CAOGo0Va8PLy1vWBe7jdh1xjZ5kD5hvaYu+4VA5mHy2-V7V7Uzw@mail.gmail.com>
	<54D79579.9030805@gmail.com>
Date: Mon, 9 Feb 2015 00:55:47 +0000
Message-ID: 
 <CAOGo0VY3ggSVEN0yezvYinRUtq8J+kpWmyJJJyts=X=evOK39A@mail.gmail.com>
Subject: Re: [Pool2] Alternative identity hashmap pool implementations
From: sebb <sebbaz@gmail.com>
To: Commons Developers List <dev@commons.apache.org>
Content-Type: text/plain; charset=UTF-8

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.

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

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