RFR: JDK-8322943: runtime/CompressedOops/CompressedClassPointers.java fails on AIX

[Thomas Stuefe]

On Wed, 6 Mar 2024 10:20:38 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>> I don't think we need to cater to the 4K page mode. It was originally added for AIX versions that don't support 64K pages. AIX 4.xx IIRC. The need for it should be long gone.
>> 
>> I am not doing AIX coding anymore, but my advice would be to simplify and require 64k pages always. WRT this change, the 4K page handling is certainly not needed. Just hardcode to SHMLBA. Or 256M, whatever causes the least amount of ifdefs.
>> 
>> Other than that, please slim the fix down to the necessary. The many copypasted sections are really not pretty.
>> 
>> The only really needed parts are the one in os::reserve_memory_inbetween. This is a small issue, let's keep the patch small and confined.
>
> Hi @tstuefe, this is a fix for an existing issue. Changing the behavior of the AIX allocation is not the intention of this fix.  Modernizing the AIX memory allocation is a possible follow-up. Also we need to backport this, so it should be fix-only. Thus I think we should really do the 4K/256M distiction here to match the allocation implemented in os_aix.cpp.

Hi @GoeLin ,

not sure what you understood from my comment, but it was not what I was writing.

> Hi @tstuefe, this is a fix for an existing issue. Changing the behavior of the AIX allocation is not the intention of this fix. Modernizing the AIX memory allocation is a possible follow-up. 

Which I did not propose? Quoting myself here:

"Other than that, please slim the fix down to the necessary...
The only really needed parts are the one in os::reserve_memory_inbetween. T**his is a small issue, let's keep the patch small and confined.**"

> Also we need to backport this, so it should be fix-only. 

Backport to where? `os::reserve_memory_inbetween` was introduced with https://bugs.openjdk.org/browse/JDK-8312018, which was introduced with JDK 22. The hunk affecting `virtualspace.cpp` has nothing to do with this problem, and should not be part of this patch.

> Thus I think we should really do the 4K/256M distiction here to match the allocation implemented in os_aix.cpp.

Which does not follow from "it should be a minimal fix". The 4K scenario is fictional. And even if it were not, aligning reservations to SHMLBA is never wrong, not even if you use mmap. So why the added complexity?

To be clear, I dislike it because of the the n times copy-pasted logic spread over the code base. If it were just one hunk in os.cpp, I would not care about the 4K page mode.

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

PR Comment: https://git.openjdk.org/jdk/pull/18105#issuecomment-1980792314