RFR: JDK-8307935: Class space argument processing can be simplified

Thomas Stuefe stuefe at openjdk.org
Thu May 11 18:47:34 UTC 2023


On Thu, 11 May 2023 12:53:59 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> [JDK-8299229](https://bugs.openjdk.org/browse/JDK-8299229) removed the last scenario that tied compressed klass pointers to compressed oops.
> 
> In argument.cpp, we can therefore remove comments that refer to this old dependency.
> 
> Furthermore, in `Arguments::set_use_compressed_klass_ptrs()`, we can remove the following code pieces:
> 
> 
>   if (FLAG_IS_DEFAULT(UseCompressedClassPointers)) {
>     FLAG_SET_ERGO(UseCompressedClassPointers, true);
>   }
> 
> 
> I think this is a remnant from the time where compressed class pointers depended on compressed oops. We can now just set UseCompressedClassPointers by default to true for 64-bit platforms, which has the same effect as above code and is more in line of how we maintain default values normally.
> 
> 
>   // Check the CompressedClassSpaceSize to make sure we use compressed klass ptrs.
>   if (UseCompressedClassPointers) {
>     if (CompressedClassSpaceSize > KlassEncodingMetaspaceMax) {
>       warning("CompressedClassSpaceSize is too large for UseCompressedClassPointers");
>       FLAG_SET_DEFAULT(UseCompressedClassPointers, false);
>     }
>   }
> 
> 
> This coding is unnecessary since the CompressedClassSpaceSize option is range-checked and capped at 3GB. KlassEncodingMetaspaceMax is 32GB, so we will never hit the above condition. Moreover, even if that check were effective, it would not be sufficient, since the encoding range size is shared between CDS and class space and therefore the space available depends on the (yet unknown) CDS archive size, modulo internal alignments. To reduce complexity, we may just remove this part.
> 
> (It makes sense to leave an assert here to counter-check the range check which is supposedly done beforehand).
> 
> Tests:
> - manually ran runtime/CommandLine, runtime/Metaspace, gc/Metaspace and all gtest variants on Linux x64 and x86.

> Looks good. Maybe consider renaming `set_use_compressed_klass_ptrs` now that it doesn't _set_ anything.

Thanks @stefank !

I will fill this function with a new purpose setting stuff shortly, in a future patch. So I would prefer it kept this name, otherwise I would have to name it back later.

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

PR Comment: https://git.openjdk.org/jdk/pull/13931#issuecomment-1544505985


More information about the hotspot-runtime-dev mailing list