commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [configuration] Checkstyle warning about double-checked locking in DynamicCombinedConfiguration
Date Mon, 30 Jul 2012 22:21:27 GMT
On 30 July 2012 22:49, Ralph Goers <ralph.goers@dslextreme.com> wrote:
>
> On Jul 30, 2012, at 2:17 PM, sebb wrote:
>
>> On 30 July 2012 22:13, Ralph Goers <ralph.goers@dslextreme.com> wrote:
>>>
>>> On Jul 30, 2012, at 2:10 PM, sebb wrote:
>>>
>>>> On 30 July 2012 20:34, Oliver Heger <oliver.heger@oliver-heger.de>
wrote:
>>>>> Am 29.07.2012 22:41, schrieb Ralph Goers:
>>>>>
>>>>>> I used that specifically to avoid creating multiple combined
>>>>>> configurations since it can be fairly expensive to create them. 
I looked
>>>>>> into the guarantees that ConcurrentMap provides before I implemented
that
>>>>>> and included the comment since I knew it would catch someone's eye.
>>>>>>
>>>>>> Ralph
>>>>>
>>>>>
>>>>> Thanks for clarifying. I added a suppression in the Checkstyle configuration
>>>>> so that this warning will not pop up again.
>>>>
>>>> An alternative solution might be to drop the synchronised block and
>>>> use the atomic putIfAbsent() method.
>>>>
>>>> That would reduce the synch. overhead.
>>>> The downside is that the code might occasionally create and configure
>>>> an instance of CombinedConfiguration that is then not used (if another
>>>> thread happens to do the same).
>>>
>>> Did you read Oliver's message which is directly below what you wrote?
>>
>> Yes, but that only talks about suppressing the warning, not reducing
>> the amount of time spent in the synchronised block.
>
> We must be reading something different. In the second paragraph below Oliver also suggested
using putIfAbsent and removing the synchronization.  My response at the top of this is exactly
why I chose not to do that.

Sorry, I see now.  I read the code and the last part of the thread (only).

It would be helpful to add a comment to the code as to why putIfAbsent
was not used so this is not raised again.

> Ralph
>
>>
>>> Ralph
>>>
>>>>
>>>>> Oliver
>>>>>
>>>>>
>>>>>>
>>>>>> On Jul 29, 2012, at 12:40 PM, Oliver Heger wrote:
>>>>>>
>>>>>>> There is a checkstyle warning about double-checked locking in
method
>>>>>>> DynamicCombinedConfiguration.getCurrentConfig(). Indeed, the
double-check
>>>>>>> locking idiom is used, however, there is a comment saying that
this safe due
>>>>>>> to the usage of a ConcurrentMap.
>>>>>>>
>>>>>>> This may be true, but I wonder whether it would be better to
use the
>>>>>>> map's putIfAbsent() method and avoid synchronization. The worst
thing that
>>>>>>> can happen is that on parallel access multiple CombinedConfiguration
>>>>>>> instances are created which can be passed immediately to the
garbage
>>>>>>> collector.
>>>>>>>
>>>>>>> Thoughts?
>>>>>>>
>>>>>>> Oliver
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> 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
>>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> 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
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
>>
>
>
> ---------------------------------------------------------------------
> 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