[crac] RFR: Fix ordering of invocation on Resources [v3]

[Anton Kozlov]

On Tue, 2 May 2023 07:06:48 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/crac/impl/AbstractContextImpl.java line 55:
>> 
>>> 53:         restoreQ.add(resource);
>>> 54:         try {
>>> 55:             resource.beforeCheckpoint(semanticContext());
>> 
>> Does this mean a Resource may get another Context and not the one to which it has been registered? This may be very unexpected for the Resource implementation.
>
> Theoretically, the method could do that. However, here the purpose of `semanticContext()` is to pass the context to which it was registered rather than the subcontext where it is stored (but this is an implementation detail that the resource does not know about).

Just realized that this is required for PriorityContext implementation. But that is the implementation of that class, it's wrong ACI has to care about that.

>> src/java.base/share/classes/jdk/crac/impl/AbstractContextImpl.java line 78:
>> 
>>> 76:     }
>>> 77: 
>>> 78:     protected abstract void runBeforeCheckpoint();
>> 
>> This is intended to be overwritten (becomes a part of the class interface). The intent behind the separate method is not evident. Corresponding runAfterRestore is private though.
>> 
>> After AbstractContexImpl has lost parameter P and comparator, a distinction between AbstractContexImpl and OrderedContext has been lost. Merging AbstractContexImpl into OrderedContext likely will provided clearer code.
>
> ACI is implemented both by OrderedContext and PriorityContext, while PC is quite different from OC.

I'm trying to describe class hierachy and failing. The patch tries to reverse ACI-PC relation. ACI was for partially-ordered Resources (defined by a Comparator), and now it's for totally-ordered Resources (ordered by long). Trying to fit the partially-ordering PC as a subclass of the totally-ordering ACI feels unnatural. Can we have a cleaner hierachy of the classes?

-------------

PR Review Comment: https://git.openjdk.org/crac/pull/60#discussion_r1182349575
PR Review Comment: https://git.openjdk.org/crac/pull/60#discussion_r1182347913