RFR(s): 8170520: Make Metaspace ChunkManager counters non-atomic

[Mikael Gerdin]

Hi Thomas,

On 2016-11-30 08:43, Thomas Stüfe wrote:
> Hi all,
>
> please take a look at this small fix. (This is one of the smaller fixes in
> preparation for a prototype for JDK-8166690, I try to move the smaller
> stuff out of the way first).
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8170520
>
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8170520-Make-
> Metaspace-ChunkManager-Counters-NonAtomic/webrev.00/webrev/

I threw some testing at your change and it seems like you've forgotten 
about calling inc_free_chunks_total for the humongous chunks in 
~SpaceManager. I'm going to run more tests on a patched build where I 
just hack in a call to inc_free_chunks_total for reach humongous chunk.

It would be great if you could have a look at unifying the 
ChunkManager::return_chunks API for standard and humongous chunks so 
that we wouldn't need the special casing for this in ~SpaceManager
That way we could also make humongous_dictionary() private to 
ChunkManager which would improve encapsulation.

Thanks
/Mikael

>
> Small discussion here:
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-October/021709.html
>
> Background: ChunkManager keeps counters (_free_chunks_count and
> free_chunks_total). These counters are updated using Atomics. Because this
> is expensive, code goes thru some lengths to accumulate updates, which in
> turn makes the code more complicated and error prone as necessary. But
> updating the counters atomically is not needed, because updates happen
> under lock protection anyway.
>
> The proposed fix makes updates non-atomic and moved the call to
> inc_free_chunks_total() into ChunkManager::return_chunks() - close to where
> the chunk is actually returned to the freelist, so the time window where
> the counters are invalid is very small now.
>
> It also fixes an assert and makes inc_free_chunks_total() private because
> now it can be private.
>
> Note that I do not intend to push this into 9, so this is for the upcoming
> 10 repository. I would like to get some reviews nevertheless, so this small
> change can be pushed quickly once the 10 repo exists.
>
> Thanks!
>
> Best Regards, Thomas
>