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

[Coleen Phillimore]

On Fri, 12 May 2023 05:39:47 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.
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8307935-Class-space-argument-processing-can-be-simplified
>  - modify assert
>  - JDK-8307935-Class-space-argument-processing-can-be-simplified

This is ok.  I was going to object thinking that users/tests might have -XX:+UseCompressedOops and not remember to also set UseCompressedClassPointers, but if the latter is default that's fine.

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

Marked as reviewed by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13931#pullrequestreview-1425127727