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

Radim Vansa duke at openjdk.org
Tue May 30 06:22:43 UTC 2023


On Mon, 29 May 2023 17:05:47 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

>> Yes, the behaviour should be specified through javadoc but that does not say anything about registration when the checkpoint is proceeded. Per your logic that behaviour is unspecified and hence the change doesn't matter. Moreover, you've explicitly asked to postpone javadoc changes into #65 which did not get review in 2 weeks.
>> 
>> Rather than changing implementation forth and back you could explain why certain behaviour is more useful. Now you've removed a test for certain codepath - creating resource in global context - if you think it should behave in a different way you should keep it and assert different outcome.
>
> Having that we've met so many self-deadlocks after the blocking was introduced, it seems it will happen with the users who will run into the same problem with the global context, which we suggest by default. I'm still trying to untagle with consequences of #60, with this PR in particular. You can see the policy for the global context is going to be specified by a single line of code. Anyway sorry for reviews taking so long. You can simplify reviewer job by separating functional and non-functional changes. Rephrasing javadoc for readability is non functional, changing the meaning is functional.

One of the points in #60 was to make user errors *visible*, rather than silently passing - that was one reason I pushed that PR hard. Now looking again into `OrderedContext` that's what this PR brings and I strongly disagree. There's two ways to cope with those errors in the order of registration I know of: either throw an error (and I don't mind if it's from the `Core.checkpointRestore` or straight from `register`) or block, and you've chosen the latter option, stating that the problem is clearly visible from the threaddump.

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

PR Review Comment: https://git.openjdk.org/crac/pull/74#discussion_r1209754899


More information about the crac-dev mailing list