RFR(xs): 8170520: Make Metaspace ChunkManager counters non-atomic (original) (raw)
Thomas Stüfe thomas.stuefe at gmail.com
Tue Mar 21 08:59:41 UTC 2017
- Previous message: RFR (XS): JDK-8177070 - Quarantine tests due to JDK-8177069
- Next message: C++ inlining of references
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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
- Previous message: RFR (XS): JDK-8177070 - Quarantine tests due to JDK-8177069
- Next message: C++ inlining of references
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]