JDK-8302130 : Make MetaspaceReclaimPolicy a diagnostic switch
  • Type: CSR
  • Component: hotspot
  • Sub-Component: runtime
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 21
  • Submitted: 2023-02-09
  • Updated: 2023-02-14
  • Resolved: 2023-02-13
Related Reports
CSR :  
JDK-8302129 - Make MetaspaceReclaimPolicy a diagnostic switch
Relates :  
JDK-8302385 - Remove MetaspaceReclaimPolicy=none
Description
##Summary

Make MetaspaceReclaimPolicy a diagnostic switch

##Problem

JEP-387 revamped Metaspace and, among other things, introduced the product switch `MetaspaceReclaimPolicy`. The switch switches between three different behavioral modes:

1) **MetaspaceReclaimPolicy=balanced** (default) attempts to reclaim Metaspace upon class loader death

2) **MetaspaceReclaimPolicy=aggressive** does the same but more enthusiastically; 

The difference between them is minor; Metaspace has an inbuilt granularity with which memory is committed and uncommitted - 64K by default, 16K in aggressive mode. In practice, it did not matter much: aggressive mode would lead to slightly more reclaimed memory at the cost of slightly more VMA fragmentation.

3) **MetaspaceReclaimPolicy=none** forfeits memory reclamation altogether. This is a larger break from the other two modes and leads to a bunch of special-path coding in metaspace. 

These three modes lead to different code paths that all need to get tested separately to prevent regressions. MetaspaceReclaimPolicy=none, in particular, also makes the code more complex.

The original purpose of this switch, especially the "none" mode, was to have a fallback in case of errors. But post-JEP-387-Metaspace has been very stable and robust; there was never a need for this fallback.

##Solution

I would like to make `MetaspaceReclaimPolicy` a diagnostic switch, with the expressed freedom to decrease later the number of permutations and/or to completely remove the switch as a user-facing control, without further CSRs.

##Specification

    --- a/src/hotspot/share/runtime/globals.hpp
    +++ b/src/hotspot/share/runtime/globals.hpp
    @@ -1423,7 +1423,7 @@ const int ObjectAlignmentInBytes = 8;
               "Force the class space to be allocated at this address or "       \
               "fails VM initialization (requires -Xshare=off.")                 \
                                                                                 \
    -  product(ccstr, MetaspaceReclaimPolicy, "balanced",                        \
    +  product(ccstr, MetaspaceReclaimPolicy, "balanced", DIAGNOSTIC,            \
               "options: balanced, aggressive, none")                            \
                                                                                 \
       product(bool, PrintMetaspaceStatisticsAtExit, false, DIAGNOSTIC,          \
Comments
Moving to Approved.
13-02-2023
> (I always wondered why I have to give a concrete patch as specification For flags like this the only "specification" is what we have in the globals.hpp file(s), so it has become standard practice to just put the code diff as the specification in these cases. The textual description is already given in the solution section.
12-02-2023
Looks reasonable to me. I doubt anyone is actually using this flag in product environments.
11-02-2023
> Thomas Stuefe diagnostic switches are still "product" switches, they just have to be unlocked before use. From a CSR perspective once a switch is "diagnostic" then it doesn't need a CSR request for future changes. Thanks for the information. What I would like, as I wrote, is to be able to change it or even completely remove it. That should be possible with a diagnostic switch without CSR, right? Sorry about the bad specification. I was short in time and just did it blindly (thought that would be fine since its obvious). (I always wondered why I have to give a concrete patch as specification - a textual form should suffice, because why would I do the code changes if I don't even know the CSR will be approved in its current form, or at all. The patch should be the end of a process that starts with the CSR.)
10-02-2023
[~stuefe] diagnostic switches are still "product" switches, they just have to be unlocked before use. From a CSR perspective once a switch is "diagnostic" then it doesn't need a CSR request for future changes. \+ diagnostic(ccstr, MetaspaceReclaimPolicy, "balanced", That is not how diagnostic flags are defined these days: \+ product(ccstr, MetaspaceReclaimPolicy, "balanced", DIAGNOSTIC
09-02-2023
Moving to Provisional. Please have one or more other runtime engineers review the CSR before it is Finalized for the second phase of CSR review.
09-02-2023