[crac] RFR: Selectable Global Context implementation
Anton Kozlov
akozlov at openjdk.org
Wed Jun 28 12:36:36 UTC 2023
On Wed, 28 Jun 2023 11:39:58 GMT, Radim Vansa <rvansa at openjdk.org> wrote:
>> A number of problems were caused by switching to BlockingOrderedContext in the JDK code. To prevent the same in the potential users of EA builds, I temporarily switch the default implementation to the previous OrderedContext. The BlockingContext is still available via a command line option. After an EA build and transition of CRaC development to track newer version openjdk/jdk, the default implementation will be changed.
>>
>> To make properties available, it was required to delay Reference resource registration. Otherwise, the GlobalContext implementation decision had to be done too early, in the Reference initialization during JDK bootstrapping, before Properties are available.
>
> src/java.base/share/classes/jdk/crac/Core.java line 77:
>
>> 75: private static final Context<Resource> globalContext = GlobalContext.createGlobalContextImpl();
>> 76:
>> 77: private static class ReferenceHandlerResource implements JDKResource {
>
> I wouldn't pollute the `Core` class with domain-specific code; if the 'reliability' of registration is of concern I think it's fine to expose the static `ensureRegistered`/`register` method.
> Or, at least refactor this into its own file.
I considered register() interface. The waitForReferenceProcessing is an internal JDK interface. The reference processing itself does not require the special managment on checkpoint, but it's checkpoint requirement as a procedure that no reference processing is in progress at some certain point.
The class is a boilerplate code to call two methods, it is not shared and unlikely will, so a separate file is not justified.
> src/java.base/share/classes/jdk/crac/impl/GlobalContext.java line 8:
>
>> 6:
>> 7: public class GlobalContext {
>> 8: private static final String GLOBAL_CONTEXT_IMPL_PROP = "jdk.crac.globalContext.impl";
>
> As mentioned in off-GH conversation, I don't think this option would be ever used, and if you are going to change the default I'd rather drop it to keep the code simple.
Since there is no way to employ BlockingContext other than as the global context implementation, I prefer to make it selectable.
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/87#discussion_r1245141056
PR Review Comment: https://git.openjdk.org/crac/pull/87#discussion_r1245108228
More information about the crac-dev
mailing list