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

Stefan Karlsson stefank at openjdk.org
Thu Oct 24 19:09:42 UTC 2024


On Thu, 24 Oct 2024 18:14:25 GMT, Chris Plummer <cjplummer at 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


More information about the hotspot-gc-dev mailing list