On Fri, 20 Sep 2024 17:46:21 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> Roman Kennke has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'lilliput/JEP-450-temporary-fix-branch-2' >> into JDK-8305895-v4 >> - review feedback > > src/hotspot/share/memory/metaspace.cpp line 799: > >> 797: >> 798: // Set up compressed class pointer encoding. >> 799: // In CDS=off mode, we give the JVM some leeway to choose a >> favorable base/shift combination. > > I don't know why this comment is here. Seems out of place. Its not, but maybe too vague. There are two ways to initialize CompressedKlassPointers : - `CompressedKlassPointers::initialize(address, size)` - called here - is used for no CDS case and allows the JVM to freely pick encoding base and shift. - `CompressedKlassPointers::initialize_for_given_encoding` is called when encoding base and shift are predetermined (when using CDS). Then, the JVM has no freedom at all, it just does sanity checks. The comment basically says "since here we are not using CDS, we are calling CompressedKlassPointers::initialize(address, size) to give the JVM some freedom when choosing encoding base and shift". Is this clearer? Should I just remove the code? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1808683312
