JDK-8262074: Consolidate the default value of MetaspaceSize

[tstuefe]

I was looking at whether the default values for MetaspaceSize (the initial threshold to start off a metaspace-motivated GC) still make sense after JEP-387.

The default is dependent on compiler tier and bitness. It is also spread across all platforms.

In addition to that, it also may get modified after Metaspace::ergo_initialize() in client-compiler-emulation-mode:

jdk/src/hotspot/share/compiler/compilerDefinitions.cpp

Lines 194 to 196 in 2b00367

	if (FLAG_IS_DEFAULT(MetaspaceSize)) {
	FLAG_SET_ERGO(MetaspaceSize, MIN2(12*M, MaxMetaspaceSize));
	}

which is unexpected and causes confusion (eg JDK-8261907, JDK-8261907).

The reasons for this seem to originate from PermGen times:
https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2021-February/045536.html

---

Today, MetaspaceSize defaults to:

• no compiler (eg Zero): 4M (32bit) 5.19M (64bit)
• C1-only build: 12M
• C1+C2 build (standard): 16M (32bit) 20,75M (64bit)

I was surprised to see that they do not depend on any compiler runtime switches. It only depends on build time decisions.

---

How much do we use? I analyzed a simple java app to see the difference VM settings make on initial metaspace consumption. Committed space, used in brackets:

(Note:                  (used)          committed
CDS on:

64bit:       		(181,58 KB)     384 KB  (a)
64bit tier1 only:       (170,04 KB)     384 KB
64bit Xint:		(16,62 KB)      256 KB

32bit    		(178 KB)        256 KB
32bit tier1 only: 	(144 KB)        256 KB
32bit Xint: 		(11 KB) (b)     128 KB  

CDS off:

64bit:			(5,06 MB)       5.62 MB
64bit tier1 only:       (5,00 MB)       5,56 MB
64bit Xint:		(4,84 MB)       5.44 MB

32bit    		(3,69 MB)       3.75 MB
32bit tier1 only: 	(3,65 MB)       3.75 MB
32bit Xint: 		(3,52 MB)       3.62 MB

Class space on/off

CDS off, 64bit, +CompressedClassPointers: 5.44M
CDS off, 64bit, -CompressedClassPointers: 5.38M

Notes:
(a) Since JEP-387, with CDS=on, we pay very little committed footprint upfront (384K). For comparison, JDK 15 commits here 5.75M.
(b) The seemingly high difference between Xint and C1+C2 - 11K vs 178K - is misleading: All initial classes get compiled, but since most of their metadata live in CDS, not in Metaspace, all we allocate at the start are MethodCounters. Hence, with -Xint, we almost allocate nothing. That changes as soon as we start loading application classes.

Conclusions:

• CDS=off increases metaspace footprint by a flat amount, in my case ~5MB, which makes sense.
• Running with (any) compiler has not much influence once we start using Metaspace for real. The difference between C1-only and C1+C2 is neglectible, the difference between Xint and C1+C2 amounts to about 2% wrt to initial metaspace consumption.
• Running with or without compressed Klass pointers makes not much difference. With class space, we pay for certain overhead twice, but at this early stage this is not noticeable.
• The difference between 64bit and 32bit is more like 1.4-1.5, not the 1.3 factor we currently assume

---

Proposal:

1. I propose to make MetaspaceSize independent from compiler. For one, if the intention was to have a lower threshold with compilers deactivated, that has never worked. E.g. on 64bit we always had a threshold of 20.75MB regardless of Xint/TieredStopAtLevel. Even if it worked, the compiler does not make that much difference in metaspace footprint.

2. I propose to slightly lower MetaspaceSize - on 32bit from 16M to 14M, on 64bit from 20.75M to 20M. This takes the slightly lower metaspace footprint since JEP 387 into account (less waste) and the scale I found to be higher than 1.3.

This is all very cautious. For the standard VM, very little changes, so this is mainly a cleanup patch. We could probably tune MetaspaceSize down to much lower levels. And/or make size it differently depending on UseSharedSpaces. However, atm I don't have time to hunt regressions due to too early GCs.

---

Tests: GA, nightlies at SAP

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8262074: Consolidate the default value of MetaspaceSize

Reviewers

• Ioi Lam (@iklam - Reviewer) ⚠️ Review applies to 6b3c81a
• Coleen Phillimore (@coleenp - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2675/head:pull/2675
$ git checkout pull/2675

[bridgekeeper]

👋 Welcome back stuefe! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@tstuefe The following label will be automatically applied to this pull request:

• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (152ec61)
• 00: Full (6b3c81a)

[iklam]

Looks good to me. I would suggest changing the RFE title to something like "Consolidate the default value of MetaspaceSize".

[openjdk]

@tstuefe This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8262074: Consolidate the default value of MetaspaceSize

Reviewed-by: iklam, coleenp

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 61 new commits pushed to the master branch:

• 8256517: 8262421: doclint warnings in jdk.compiler module
• 29c603f: 8262227: Change SystemDictionary::find() to return an InstanceKlass*.
• 35c0a69: 8262416: ProblemList TestHeapDumpForLargeArray.java due to JDK-8262386
• 228c285: 8261170: Upgrade to freetype 2.10.4
• ded96dd: 8139348: Deprecate 3DES and RC4 in Kerberos
• 5a9b701: 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides
• 7d4f60b: 8260403: javap should be more robust in the face of invalid class files
• 674be87: 8261203: Incorrectly escaped javadoc html with type annotations
• 2eca17d: 8261457: test/langtools/tools/javac/T8187978 can fail if ArrayList class is modified
• c54724d: 8257234: Add gz option to SA jmap to write a gzipped heap dump
• ... and 51 more: https://git.openjdk.java.net/jdk/compare/26c1db901b4f9e61d9bf917a7857742c0051c90d...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[tstuefe]

Looks good to me.
Thanks!
I would suggest changing the RFE title to something like "Consolidate the default value of MetaspaceSize".
Done.

[mlbridge]

Mailing list message from Stefan Karlsson on hotspot-dev:

Hi Thomas,

On 2021-02-23 06:31, Thomas Stuefe wrote:

I was looking at whether the default values for MetaspaceSize (the initial threshold to start off a metaspace-motivated GC) still make sense after JEP-387.

The default is dependent on compiler tier and bitness. It is also spread across all platforms.

In addition to that, it also may get modified after Metaspace::ergo_initialize() in client-compiler-emulation-mode:

https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/hotspot/share/compiler/compilerDefinitions.cpp#L194-L196

which is unexpected and causes confusion (eg JDK-8261907, JDK-8261907).

The reasons for this seem to originate from PermGen times:
https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2021-February/045536.html

----

Today, MetaspaceSize defaults to:

- no compiler (eg Zero): **4M** (32bit) **5.19M** (64bit)
- C1-only build: **12M**
- C1+C2 build (standard): **16M** (32bit) **20,75M** (64bit)

I was surprised to see that they do not depend on any compiler *runtime* switches. It only depends on build time decisions.

---

How much do we use? I analyzed a simple java app to see the difference VM settings make on initial metaspace consumption. Committed space, used in brackets:

(Note: (used) committed
CDS on:

64bit: (181,58 KB) 384 KB (a)
64bit tier1 only: (170,04 KB) 384 KB
64bit Xint: (16,62 KB) 256 KB

32bit (178 KB) 256 KB
32bit tier1 only: (144 KB) 256 KB
32bit Xint: (11 KB) (b) 128 KB

CDS off:

64bit: (5,06 MB) 5.62 MB
64bit tier1 only: (5,00 MB) 5,56 MB
64bit Xint: (4,84 MB) 5.44 MB

32bit (3,69 MB) 3.75 MB
32bit tier1 only: (3,65 MB) 3.75 MB
32bit Xint: (3,52 MB) 3.62 MB

Class space on/off

CDS off, 64bit, +CompressedClassPointers: 5.44M
CDS off, 64bit, -CompressedClassPointers: 5.38M

_Notes:
(a) Since JEP-387, with CDS=on, we pay very little committed footprint upfront (384K). For comparison, JDK 15 commits here 5.75M.
(b) The seemingly high difference between Xint and C1+C2 - 11K vs 178K - is misleading: All initial classes get compiled, but since most of their metadata live in CDS, not in Metaspace, all we allocate at the start are MethodCounters. Hence, with -Xint, we almost allocate nothing. That changes as soon as we start loading application classes._

Conclusions:
- CDS=off increases metaspace footprint by a flat amount, in my case ~5MB, which makes sense.
- Running with (any) compiler has not much influence once we start using Metaspace for real. The difference between C1-only and C1+C2 is neglectible, the difference between Xint and C1+C2 amounts to about 2% wrt to initial metaspace consumption.
- Running with or without compressed Klass pointers makes not much difference. With class space, we pay for certain overhead twice, but at this early stage this is not noticeable.
- The difference between 64bit and 32bit is more like 1.4-1.5, not the 1.3 factor we currently assume

-----

Proposal:

1) I propose to make MetaspaceSize independent from compiler. For one, if the intention was to have a lower threshold with compilers deactivated, that has never worked. E.g. on 64bit we always had a threshold of 20.75MB regardless of Xint/TieredStopAtLevel. Even if it worked, the compiler does not make that much difference in metaspace footprint.

I think it makes sense to get rid of this difference between build
configurations.

2) I propose to slightly lower MetaspaceSize - on 32bit from 16M to 14M, on 64bit from 20.75M to 20M. This takes the slightly lower metaspace footprint since JEP 387 into account (less waste) and the scale I found to be higher than 1.3.

This is all very cautious. For the standard VM, very little changes, so this is mainly a cleanup patch. We could probably tune MetaspaceSize down to much lower levels. And/or make size it differently depending on UseSharedSpaces. However, atm I don't have time to hunt regressions due to too early GCs.

What would the motivation be for wanting to lower the size of
MetaspaceSize? I've gotten feedback from others reporting that we
trigger GCs too early, because of a (for them) too low MetaspaceSize.

Thanks,
StefanK

[tstuefe]

Hi Thomas,

(skip)

---

Proposal:

1. I propose to make MetaspaceSize independent from compiler. For one, if the intention was to have a lower threshold with compilers deactivated, that has never worked. E.g. on 64bit we always had a threshold of 20.75MB regardless of Xint/TieredStopAtLevel. Even if it worked, the compiler does not make that much difference in metaspace footprint.

I think it makes sense to get rid of this difference between build
configurations.

2. I propose to slightly lower MetaspaceSize - on 32bit from 16M to 14M, on 64bit from 20.75M to 20M. This takes the slightly lower metaspace footprint since JEP 387 into account (less waste) and the scale I found to be higher than 1.3.
   This is all very cautious. For the standard VM, very little changes, so this is mainly a cleanup patch. We could probably tune MetaspaceSize down to much lower levels. And/or make size it differently depending on UseSharedSpaces. However, atm I don't have time to hunt regressions due to too early GCs.

What would the motivation be for wanting to lower the size of
MetaspaceSize? I've gotten feedback from others reporting that we
trigger GCs too early, because of a (for them) too low MetaspaceSize.

Initial metaspace usage is probably much lower than it was when these sizes were thought up. For one, due to the CDS, which is usually on. Then, due to the reduced waste in metaspace. See measurements. This may matter for low-consumption JVMs which do class unloading - since you would never hit the threshold, you may never clean up, unless the GC runs due to other reasons.

But I admit that I do not have a good base upon which to decide which size is right. Once could say "you must be able to load as many classes as you did when this limit was thought up" which would mean we could lower the limit much more aggressivly than I did. Or we could say "dont shake the boat" and leave the limits as they are. I tried to find a middle way.

However, it also makes sense to keep the limits as they are. Then this patch is truly just a cleanup.

I now set the 32bit limit to 16M, which is what we did have before; the 64bit threshold is set to 21M, which is 0.25M more than before, just to have a nice round number.

Thanks,
StefanK

Thanks, Thomas

[coleenp]

This looks good. I appreciate the more conservative approach and doing this as a cleanup.

[tstuefe]

This looks good. I appreciate the more conservative approach and doing this as a cleanup.

Thank you Coleen!

[tstuefe]

/integrate

[openjdk]

@tstuefe Since your change was applied there have been 75 commits pushed to the master branch:

• 05c11bc: 8262426: Change TRAPS to Thread* for find_constrained_instance_or_array_klass()
• d06d6f5: 8262402: Make CATCH macro assert not fatal
• 47a0842: 8241403: JavaThread::get_thread_name() should be ThreadSMR-aware
• 3088e1e: 8262430: doclint warnings in java.base module
• 67b9e5a: 8262420: typo: @implnote in java.desktop module
• 240f2a1: 8260366: ExtendedSocketOptions can deadlock in some circumstances
• de3f519: 8258897: wrong translation of capturing local classes inside nested lambdas
• d7efb4c: 8262199: issue in jli args.c
• 7603278: 8260198: TypeInstPtr::dump2() emits multiple lines if Verbose is set
• 0a4e710: 8261954: Dependencies: Improve iteration over class hierarchy under context class
• ... and 65 more: https://git.openjdk.java.net/jdk/compare/26c1db901b4f9e61d9bf917a7857742c0051c90d...master

Your commit was automatically rebased without conflicts.

Pushed as commit c9e9189.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.