[crac] RFR: Introduce per-Priority Context with different policies [v4]

Anton Kozlov akozlov at openjdk.org
Wed May 31 08:51:26 UTC 2023


On Wed, 31 May 2023 05:58:00 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> Anton Kozlov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update
>
> src/java.base/share/classes/jdk/crac/Core.java line 124:
> 
>> 122: 
>> 123:         try {
>> 124:             jdk.internal.crac.Core.getJDKContext().beforeCheckpoint(null);
> 
> Why don't you register this as a resource? Besides, the way you did it does not work correctly if exceptions are thrown.

This part was intentionally omitted. JDKContext does not throw.

> The JDKContext is now just a holder of ClaimedFDs, I'll address this in a follow-up that depends on this Context follow-up.

> src/java.base/share/classes/jdk/crac/impl/OrderedContext.java line 38:
> 
>> 36:  * @param <R>
>> 37:  */
>> 38: public class OrderedContext<R extends Resource> extends AbstractContext<R> {
> 
> This class should not be used anywhere directly, and is not exposed to users => should be abstract.

By it's own it's a valid Contex implementation.

> src/java.base/share/classes/jdk/internal/crac/JDKContext.java line 46:
> 
>> 44: import java.util.WeakHashMap;
>> 45: 
>> 46: public class JDKContext implements JDKResource {
> 
> Since this is not a context anymore I believe that in the current form the class would deserve a rename (`ClaimedFileDescriptors`?)

See other comment https://github.com/openjdk/crac/pull/74#discussion_r1211309286

> test/jdk/jdk/crac/ContextOrderTest.java line 102:
> 
>> 100:     private static void testRegisterBlocks() throws Exception {
>> 101:         var recorder = new LinkedList<String>();
>> 102:         BlockingOrderedContext<Resource> blockingCtx = new BlockingOrderedContext();
> 
> We don't need separate contexts now that global one is blocking.

The test checks blocking properties, while blocking behavior of the global context  is not specified.

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

PR Review Comment: https://git.openjdk.org/crac/pull/74#discussion_r1211309286
PR Review Comment: https://git.openjdk.org/crac/pull/74#discussion_r1211310150
PR Review Comment: https://git.openjdk.org/crac/pull/74#discussion_r1211310555
PR Review Comment: https://git.openjdk.org/crac/pull/74#discussion_r1211311935


More information about the crac-dev mailing list