[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