RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v21]

[Amit Kumar]

On Thu, 24 Oct 2024 09:22:34 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> This code causes test errors in `CompressedClassPointersEncodingScheme.java` on s390 and PPC64. It forces the shift to `log_cacheline` which is 7 on PPC64 and 9 on s390. The test passes when we remove "s > log_cacheline && " from the condition below.
>> 
>> It's a bit late. We are close to pushing. While it should be harmless to drop below alignment to below cache line size, this would be a change affecting all platforms and would require all tests repeated. 
>> 
>> PPC/s390 are not targeted by the JEP. There had never been a discussion I am aware of that these platforms have to be clean with +COH. While it's nice that the changes had been contributed, I don't think that test errors on these platforms should hold up pushing this RFE. Therefore, if needed, we should just omit +COH part of the test for PPC/S390.
>> 
>> But then, what exactly is the error? If it's just the test assuming that cache line size is log 6, then the test should be fixed for ppc, not hotspot.
>> 
>>> In addition, it doesn't fit to the comment which claims we should avoid shifts larger than the cacheline size. This enforces shifts to be larger (or equal to) than the cacheline size.
>> 
>> ?? The comment is correct. We try to avoid hyper alignment, hence we drop the shift to - if possible - log 2 cache line size. If it's equal to log 2 cache line size, we succeeded.
>
>> But then, what exactly is the error? If it's just the test assuming that cache line size is log 6, then the test should be fixed for ppc, not hotspot.
> 
> that is the problem, test assumes log2 of 6 for chacheline size

PPC log2 will be `7` (`DEFAULT_CACHE_LINE_SIZE  = 128`) and for S390x it will be `8` (`DEFAULT_CACHE_LINE_SIZE  = 256`). 

So I guess this change should be fine for now : 

diff --git a/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java b/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java
index e04e716315a..c1be59e77ab 100644
--- a/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java
+++ b/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java
@@ -108,7 +108,9 @@ public static void main(String[] args) throws Exception {
 
         long ccsSize = 128 * M;
         int expectedShift = 6;
-        test(forceAddress, true, ccsSize, forceAddress, expectedShift);
+        if (!Platform.isPPC() && !Platform.isS390x()) {
+            test(forceAddress, true, ccsSize, forceAddress, expectedShift);
+        }
 
         ccsSize = 512 * M;
         expectedShift = 8;

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1814627120