On Fri, 6 Sep 2024 11:08:02 GMT, Thomas Stuefe <[email protected]> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add function in Metaspace to tell you if Klass pointer is in compressible
>> space.
>
> src/hotspot/share/memory/metaspace.hpp line 165:
>
>> 163: return using_class_space() && (is_in_class_space(k) ||
>> is_in_shared_metaspace(k));
>> 164: }
>> 165:
>
> I propose to drop this, and instead add a utility function to
> `CompressedKlassPointers` like this:
>
>
> // Returns true if p falls into the narrow Klass encoding range
> inline bool CompressedKlassPointers::is_in_encoding_range(const void* p) {
> return _base != nullptr && p >= _base && p < (_base + _range);
> }
>
> (Probably the `_base != nullptr` could even be left out, since `_range==0`
> and `_base==nullptr` for -UseCompressedClassPointers)
>
> And then use that function in `jfrTraceIdKlass.cpp`. That file needs to use
> `CompressedKlassPointers` anyway because it needs to encode the Klass*.
>
> This avoids having to rely on the exact composition of the memory regions
> inside the encoding range. What counts is whether the Klass pointer points
> into the narrow Klass encoding range.
>
> Essentially, with CDS, memory looks like this:
>
>
> encoding
> encoding
> base end
> |-----------------------------------------------------------------------|
> |----CDS---| |--------------------class space---------------------------|
oh yes that's much better.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19157#discussion_r1747241623
