RFR: JDK-8255885: Metaspace: freelist commit counter is not updated when purging

[Thomas Stuefe]

On Wed, 18 Nov 2020 13:35:39 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Metaspace chunks are uncommitted, under certain conditions, when being returned to the Metaspace by a dying class loader.
>> 
>> There are two places where this happens:
>> 1) when individual chunks are returned (see ChunkManager::return_chunk_locked())
>> 2) later, when CLDG::purge()->Metaspace::purge()->ChunkManager::purge() is called - all free chunks are again checked and potentially uncommitted.
>> 
>> Free chunks are kept in FreeChunkList. FreeChunkList has a counter for words-comitted. That counter gets updated when chunks are added and removed. However, path (2) does not change the list, it just uncommits chunks residing in that list. That passes under the FreeChunkList's radar and now the counter is off.
>> 
>> -----
>> 
>> Review note: `ChunkManager` keeps free chunks. It has an instance of `FreeChunkListVector`, which has a series (one per chunk size) of `FreeChunkList` instances. The commit counter value is handed up from `FreeChunkList` to, eventually, `ChunkManager`.
>> 
>> What changed:
>> 
>> - I removed the counter (_committed_word_size) from FreeChunkList. This seemed the simplest and safest measure. Keeping this counter up-to-date is a hassle, since chunks may get committed and uncommitted while being in this list.
>> 
>> - Now we calculate the number of committed words on the fly if asked, by iterating all chunks in the list and accumulating. To reflect this change, I change the name of "FreeChunkList::committed_word_size()" to "FreeChunkList::calc_committed_word_size()". In metaspace, anything named "calc_" is potentially expensive.
>> 
>> - Same renamings in FreeChunkListVector and ChunkManager::total_committed_word_size() - to ChunkManager::calc_committed_word_size()
>> 
>> Now these functions always deliver the correct result. However, they are potentially slower and now require the central metaspace lock. That is not a problem: the only user of this function is when collecting the statistics for the "VM.metaspace" jcmd report, and that happens under lock protection and walks all kind of stuff anyway, so walking the freelists won't hurt.
>> 
>> Just to be sure, I removed the use of these functions from ChunkManager::print_on(), even though that was just a debug function.
>> 
>> -----
>> 
>> Tests:
>> 
>> This patch has been tested in our nightlies for a week now.
>
> Tested in our systems for a week now on all platforms, no problems.

Gentle ping...

this has been quietly brewing in our test systems for two weeks now. I think this is safe. The issue blocks a follow up PR (https://bugs.openjdk.java.net/browse/JDK-8255884).

Cheers, Thomas

-------------

PR: https://git.openjdk.java.net/jdk/pull/1057