On Thu, 24 Oct 2024 18:14:25 GMT, Chris Plummer <[email protected]> wrote:
>> `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.
OK. I did some cleanups. Could you take a look and see if this is good enough
to get this integrated?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21662#discussion_r1815573383
