RFR: 8323273: AArch64: Strengthen CompressedClassPointers initialization check for base

[Thomas Stuefe]

On Mon, 19 Feb 2024 10:38:31 GMT, Yude Lin <duke at openjdk.org> wrote:

> I think the strangest thing to me is if the reservation fails (admittedly very rare if ever), the code choose to fire the error at stub generation. It's rather random---Maybe in another day, when initialization code changes and something that also decodes klass moves ahead of stub generation, the error could happen there. We would need time to associate all the appearances of the error with the root cause of the error.

Okay, you convinced me. I'm still against changes to CompressedClassSpaceBaseAddress and the removal of reserve-anywhere patch, though.

> So why not add assertions right after reservation?

Yes, without code duplication.

1) The most straightforward and least invasive possibility would be to improve the assertion messages and/or add comments in `MacroAssembler::klass_decode_mode()`.

2) The next simple possibility would be to reuse `MacroAssembler::klass_decode_mode()` as part of verification after initialization.

> 
> You might argue that this nearly never happens in product and not worth the complication. I currently cannot argue with that because I haven't observe one myself, and I will drop the patch. But note that a single assertion would help. Perhaps consider adding one in incoming class-space refactoring?

A single assertion means I still need a way to know when it fires, so its condition needs to guess at the decoding logic that is hidden in MacroAssembler. I don't like that duplication.

> 
> > .. strange return code contract of eor_compatible_klass_encoding.
> 
> It returns the number of bits allowed to encode a klass. So 0 means there is no space left for klass encoding. Thanks for the feedbacks though. I understand the frustration of seeing those platform dependent code in shared code. I'll try not to write anything like that again.

Sorry for your frustration.

You get strong pushback here because this code, in particular, has been seeing platform ifdefs blooming like mushrooms over the years - it had often been easier to add one extra block than to look at the base structure. Which made the code complex and brittle over the years. We restructured this coding recently with https://bugs.openjdk.org/browse/JDK-8312018, and I would like to keep it somewhat clean and readable.

This coding is also rather tricky because it lives at the interjunction of three VM subsystems: metaspace/class space, JIT compiler (with its various CPU-specific idiosyncrasies) and CDS.

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

PR Comment: https://git.openjdk.org/jdk/pull/17437#issuecomment-1952365534