RFR(xs): 8170520: Make Metaspace ChunkManager counters non-atomic
mikael.gerdin at oracle.com
Mon Apr 3 08:38:21 UTC 2017
On 2017-04-02 13:26, Thomas Stüfe wrote:
> Ping... nobody?
Sorry for delaying looking at this.
I have two questions:
It looks like all callers of inc and dec both only ever pass "1" as
num_chunks, should we remove the parameter instead?
To document that the inc and dec operations are properly synchronized
maybe we should add
to inc and dec to show that?
> Kind Regards, Thomas
> On Tue, Mar 21, 2017 at 9:59 AM, Thomas Stüfe <thomas.stuefe at gmail.com
> <mailto:thomas.stuefe at gmail.com>> wrote:
> may I please get a review of another small cleanup change to the
> metaspace. Compared with the last one (JDK-8170933), this one is
> I posted this originally for jdk 9 last november, and it got
> reviewed already:
> 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
> 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
> 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,
More information about the hotspot-dev