RFR: 8342857: SA: Heap iterator makes incorrect assumptions about TLAB layout [v3]

[Chris Plummer]

On Thu, 24 Oct 2024 12:16:22 GMT, Stefan Karlsson <stefank at 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