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

[Thomas Stuefe]

On Fri, 20 Nov 2020 06:42:21 GMT, Ioi Lam <iklam 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.
>
> Looks good to me. I checked the current code and this function is used only during printing, etc. I just want to confirm -- there's no plan to use this function in the future for performance-critical stuff, right?

Hi Ioi,

> Looks good to me.

Thanks!

> I checked the current code and this function is used only during printing, etc. I just want to confirm -- there's no plan to use this function in the future for performance-critical stuff, right?

No. Should we need a quick estimate of committed-in-freelist (e.g. at some point for a better JFR statistic) there is a shortcut one could take by just assuming all chunks >= granule size are uncommitted in normal reclaim mode. This is in 98% of cases true. 

Cheers, Thomas

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

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