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

[Thomas Stüfe]

Ping... nobody?

Kind Regards, Thomas

On Tue, Mar 21, 2017 at 9:59 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
wrote:

> Hi,
>
> https://bugs.openjdk.java.net/browse/JDK-8170520
> http://cr.openjdk.java.net/~stuefe/webrevs/METASPACE-2-
> 8170520-Make-Metaspace-ChunkManager-Counters-NonAtomic/jdk10-webrev.00/
> webrev/
>
> may I please get a review of another small cleanup change to the
> metaspace. Compared with the last one (JDK-8170933), this one is smaller.
>
> I posted this originally for jdk 9 last november, and it got reviewed
> already: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2
> 016-November/021946.html. Mikael found a small bug and by then it was too
> late to bring this into jdk9, so I postponed the patch for jdk10.
>
> Now the patch got rebased to Jdk10, and it is also quite a bit simpler
> because it meshes well with the cleanup work done on JDK-8170933.
>
> The change replaces the calls to Atomic::inc/dec for the ChunkManager
> counters with simple +/-, because all code paths which modify the
> ChunkManager counters are under lock protection (SpaceManager::expand_lock()).
> This makes updating these counters cheep and thus removes the need to be
> frugal with the number of updates.
>
> Before the patch - when a list of chunks was returned to a ChunkManager in
> ~SpaceManager() - the increment values were updated once, with just one
> call to ChunkManager::inc_free_chunks_total(). This left a time window in
> which the counters did not reflect reality, so one had to be really careful
> where to place asserts to check the ChunkManager state. That made modifying
> and playing with the code error prone.
>
> Since JDK-8170933, chunks are returned to the ChunkManager via one common
> path, which always ends in ChunkManager::return_single_chunk(). Because
> of that and because updating the counters is now cheap, I moved the several
> invocations of ChunkManager::inc_free_chunks_total() to
> ChunkManager::return_single_chunk().
>
> The rest of the changes is cosmetical:
> - Moved  ChunkManager::inc_free_chunks_total() up to the private section
> of the class, because it is not called from outside anymore
> - renamed arguments for ChunkManager::inc_free_chunks_total()  and
> ChunkManager::dec_free_chunks_total() to be clearer named, and gave both
> of them the same arguments
> - Added an assert to both function asserting that the increment/decrement
> word size value should be a multiple of the smallest chunk size
>
> I ran gtests and jtreg tests on Linux and AIX, no issues popped up.
>
> Thank you for reviewing,
>
> Thomas
>
>
>
>
>
>
>
>
>