RFR: 8377307: Refactor code for AOT cache pointer compression [v2]

[Ioi Lam]

On Fri, 6 Feb 2026 12:21:43 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - restored assert
>>  - @jdksjolen review comments
>
> src/hotspot/share/cds/aotCompressedPointers.hpp line 52:
> 
>> 50:   static narrowPtr cast_to_narrowPtr(T narrowp) {
>> 51:     return checked_cast<narrowPtr>(narrowp);
>> 52:   }
> 
> This is an escape hatch that let's you encode anything into a `narrowPtr`, maybe it should be private? It doesn't seem to be used outside of the header (did a quick Ctrl+F).

Now I have only `cast_from_u4` and `cast_to_u4` and don't allow arbitrary types anymore.

> src/hotspot/share/cds/aotCompressedPointers.hpp line 68:
> 
>> 66:   }
>> 67: 
>> 68:   static narrowPtr null_narrowPtr() {
> 
> Nit: I think you can just call this `null` and that's fine and a bit prettier, but up to you.

Done.

> src/hotspot/share/cds/aotCompressedPointers.hpp line 103:
> 
>> 101: 
>> 102:   template <typename T>
>> 103:   static narrowPtr encode_null_or_address_cache(T ptr) { // may be null
> 
> For everything else we have `X` and `X_not_null` but for this one we have `X` and `X_or_null`, so here we do the inverse. Is that on purpose?

I changed the name to `encode_address_in_cache_or_null()`.

I think only `encode_not_null()` and  `encode_address_in_cache_or_null()` make sense. Yes, one of them is `or` and the other is `not`. However, the opposite doesn't work:

- If we used `encode_address_in_cache(p1)` and `encode_address_in_cache_not_null(p2)` -- `p1` will never be null if it's inside the cache, so the two functions are equivalent. Both `p1` and `p2` cannot be null.
- If we used `encode(p1)` and `encode_or_null(p2)`, it's not clear that `p1` must not be null.

> src/hotspot/share/cds/aotCompressedPointers.hpp line 145:
> 
>> 143:       return decode_not_null<T>(base_address, narrowp);
>> 144:     }
>> 145:   }
> 
> We don't wanna do this with the `base_address` being an optional argument at the end instead so we just have one function def?

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29590#discussion_r2775591084
PR Review Comment: https://git.openjdk.org/jdk/pull/29590#discussion_r2775593035
PR Review Comment: https://git.openjdk.org/jdk/pull/29590#discussion_r2775619425
PR Review Comment: https://git.openjdk.org/jdk/pull/29590#discussion_r2775620252