RFR: 8365932: Implementation of JEP 516: Ahead-of-Time Object Caching with Any GC [v3]
Erik Österlund
eosterlund at openjdk.org
Tue Oct 21 15:20:32 UTC 2025
On Mon, 20 Oct 2025 21:36:54 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> Okay. @stefank is having a look at fixing this.
>
> I've sort-of implemented the second part of your comment about removing the tri-state:
> https://github.com/fisk/jdk/compare/8326035_JEP_object_streaming_v6...stefank:jdk:8326035_JEP_object_streaming_v6_stefank
>
> The enum had *four* states (`uninitialized`, `mapping`, `streaming`, `none`) and while reading this PR the `none` part seemed unusual. After working on the proposed patch I think the `none` value served a purpose, but let's see if a patch without it makes sense.
>
> The patch still uses an enum instead of two booleans. I think that makes the code more readable because we encode the three states with three enum values. If we were to use two booleans we would end up with four possible states and then the reader of the code would have to figure out which of the forth state is an invalid state. I think what we have in the patch above is safer. (Let's revisit this if this still is a blocker)
>
> So, I removed `none` and added strict asserts that you are not allowed to ask for the mode until the variable has transitioned away from `uninitialized` to either `mapping` or `streaming`. After the variable has been initialized the questions are binary and you have `is_loading_mapping_mode() == !is_loading_streaming_mode()`. I don't see the appeal to remove one of these two functions, because that just adds negation to the code and that makes it harder to read. (We can experiment with that in a separate patch)
>
> I tried to take it even further and join these `is_loading` functions with the `is_archived_heap_in_use` and the `is_in_use` properties, but that hit problems with error-edge cases. I have a feeling that the `none` value dealt with these problems.
>
> Some more details about the edge-case problems:
>
> During loading of the heap archive there is a duration where we have decided that we are going to load the archived heap and what loading mode to use, but the archive has not been loaded enough have the `is_archived_heap_in_use` function return `true`. During these early stages we must use the `is_loading` and `is_loading_` functions. After `is_archived_heap_in_use` starts to return the correct value, the code likely needs to use that function instead of the `is_loading` functions. Some parts of the AOT/CDS code will bug out if you try to use the `is_loading` functions after an error.
>
> So, I've tried to keep the usages of `is_loading` functions inside AOT/CDS code. The `is_archived_heap_in_use`, or one of it's sub-components (E.g. `AOTMappedHeapLoader:is_in_use()`), are...
Thank you for the contribution, @stefank. I added it to the PR. @iklam hope this addresses your concerns.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27732#discussion_r2448732105
More information about the serviceability-dev
mailing list