8371549: [CRaC] Implement C/R in Selector on non-Linux #276
Conversation
|
👋 Welcome back rvansa! A progress list of the required criteria for merging this PR into |
|
@rvansa This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
openjdk
bot
added
ready
Webrevs
|
|
Sorry, the change is big and I haven't had enough spare time to review it recently. Hopefully will get to it in upcoming weeks, if nobody else will. I believe this is not a priority and does not block anything? |
|
@TimPushkin I think we had it pop somewhere but it's not a high priority. Ideally if you can review in early January. |
TimPushkin
left a comment
TimPushkin
left a comment
There was a problem hiding this comment.
I haven't read through SelectorCRaCSupport very carefully since it's mainly pre-existing code
| * @test id=ALT_LINUX | ||
| * @library /test/lib | ||
| * @modules java.base/jdk.internal.crac:+open | ||
| * @requires (os.family == "linux") |
There was a problem hiding this comment.
I haven't tested this but PollSelectorProvider is in unix directory so it should be available on other POSIX platforms. So maybe os.family != "windows"?
| @@ -0,0 +1,200 @@ | |||
| package sun.nio.ch; | |||
|
|
||
| @Override | ||
| protected Collection<FileDescriptor> claimFileDescriptors() { | ||
| // What about WEPoll eph? |
There was a problem hiding this comment.
Looks like this become a TODO/FIXME
| // On Windows the native FDs check is a no-op, but we need to set | ||
| // the FD to prevent assertion failure on Descriptor.getFd() |
There was a problem hiding this comment.
Maybe allow -1 as a value? Having 0 may be confusing as it is a valid FD value.
In the future, it looks like we need a more general abstraction for FD/handle.
2 participants
Historically we have an extensive set of tests for java.nio.channels.Selector. However this runs only on Linux (using CRIU) rather than being platform agnostic and tested on all platforms. On Linux, it does not exercise PollSelectorImpl (this can be preferred using a system property) either - just the default EpollSelectorImpl.
This PR enables testing on all available Selector implementations, cleans up the implementation of those tests and introduces the C/R implementations.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/crac.git pull/276/head:pull/276$ git checkout pull/276Update a local copy of the PR:
$ git checkout pull/276$ git pull https://git.openjdk.org/crac.git pull/276/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 276View PR using the GUI difftool:
$ git pr show -t 276Using diff file
Download this PR as a diff file:
https://git.openjdk.org/crac/pull/276.diff
Using Webrev
Link to Webrev Comment