metaspace.cpp: why are counters in ChunkManager updated atomically?

[Erik Helin]

On 10/27/2016 02:56 PM, Thomas Stüfe wrote:
> Hi Mikael,
>
> On Thu, Oct 27, 2016 at 2:41 PM, Mikael Gerdin <mikael.gerdin at oracle.com>
> wrote:
>
>> Hi Thomas,
>>
>> On 2016-10-27 14:08, Thomas Stüfe wrote:
>>
>>> Hi all,
>>>
>>> I am currently working on a prototype for
>>> https://bugs.openjdk.java.net/browse/JDK-8166690 and have a question
>>> about
>>> the ChunkManager class in metaspace.cpp.
>>>
>>> ChunkManager has the _free_chunks_total, _free_chunks_count counters. It
>>> seems the coding goes some lengths to avoid updating them often, so
>>> instead
>>> of updating them when a chunk is freed it attemps to delay and accumulate
>>> updates. This makes changing the MetaChunk allocation quite complicated,
>>> because there are large windows during which the counters are invalid and
>>> do not reflect reality.
>>>
>>> I see that the counters are updated atomically, so I assume the reason for
>>> delaying the updates is that atomics are expensive. But I could not find a
>>> good reason why the counters are updated atomically. To me, all
>>> modifications seem happen under lock protection
>>> (SpaceManager::expand_lock()). What am I overlooking?
>>>
>>
>> I don't think you are overlooking anything. The fact that these are
>> updated with atomics is something that I've noticed as well at some point
>> but I don't think I ever got around to fixing that.
>>
>>
>> I'm not sure I understand where in the code the delayed and accumulated
>> updates take place but if you think that's the case then it's probably
>> true. I suspect that at this point you are one of the handful of people who
>> are familiar with the chunk allocation code :)
>>
>>
> :) That is good news, because I then can straighten the updates out, this
> makes making the changes much simpler. Will probably do this in a separate
> fix. Thank you!

I came to the same conclusion as Mikael the last time I checked, but due 
to lack of time I didn't got around to fix it. Please send this out as a 
separate patch, it will make reviewing much easier.

Thanks,
Erik

> Thomas
>
>
>> Regards
>> /Mikael
>>
>>
>>
>>> Thanks a lot,
>>>
>>> Kind Regards, Thomas
>>>
>>>