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

Thomas Stüfe thomas.stuefe at gmail.com
Tue Mar 21 08:59:41 UTC 2017


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/
2016-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


More information about the hotspot-dev mailing list