On Thu, 24 Oct 2024 18:14:25 GMT, Chris Plummer <cjplum...@openjdk.org> 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

Reply via email to