On Thu, 24 Oct 2024 12:16:22 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/VM.java line 443:
>>
>>> 441:
>>> 442: Type collectedHeap = db.lookupType("CollectedHeap");
>>> 443: CIntegerType sizeType = (CIntegerType) db.lookupType("size_t");
>>
>> I think you can use getSizet() here.
>
> `getSizet()` seems to be a function in `Flags`, so I don't see a direct way
> to use it. I could probably use the `sizetType` instead of
> `db.lookupType("size_t")`, however when I tested the tests failed because
> sizetType had not been initialized yet.
It looks like `sizetType` is initialized further down in the constructor. You
could move that up (and the other 6 types also) or move your new code down. And
speaking of the new code, it is misplaced in the try block. Both the comment
for that block and the exception thrown have to do with getting the version.
The `reserveForAllocationPrefetch` code just above is also misplaced for the
same reason. That got added by
[JDK-8004710](https://bugs.openjdk.org/browse/JDK-8004710). Perhaps some
cleanup is in order here. I don't think a try/catch/throw is needed for the
tlab code. There are other places in the constructor that can throw an
exception, and if that happens, it should be obvious from the stack trace where
it happened. That is all you need to debug the issue. I think maybe the
VMVersion section was special cased since it is the most likely failure point
if something is really wrong that will result in a failure of SA to startup.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21662#discussion_r1815517050
