RFR: 8288627: [AIX] Implement WatchService using system library
Alan Bateman
alanb at openjdk.org
Mon Jul 4 17:34:38 UTC 2022
On Fri, 24 Jun 2022 15:17:47 GMT, Tyler Steele <tsteele at openjdk.org> wrote:
> This PR begins an effort to re-implement the WatchService API on AIX using 'AIX Event Infrastructure' (AHAFS). The initial motivation for doing so was to address errors found by some internal testing in the implementation based on PollingWatchService.java.
>
> I am submitting these changes before they are fully complete because (1) it represents a fairly large change that mostly works well, and could easily be improved upon in the future; (2) to get some early feedback on the changes so the final review process is quicker. My expected plan is to merge these changes after review, add any remaining test failures to a problem list, and return to them soon.
>
> ### Testing
> I am waiting for confirmation that this re-implementation passes the internal tests we saw failing. Initial results look promising. In addition, I see the following failing tests from `test/jdk/java/nio/file/WatchService`:
>
> - Basic.java fails at testTwoWatchers. Having two WatchService instances registered with the same file, will require some additional work to be supported with AHAFS. This is currently a limitation of this implementation.
> - DeleteInterference.java fails for the same reason as above.
> - UpdateInterference.java fails for no good reason that I can deduce. Logging the test's progress shows that it doesn't seem to leave [the main loop](https://github.com/openjdk/jdk/blob/master/test/jdk/java/nio/file/WatchService/UpdateInterference.java#L97-L111), but that it makes is to within 1ms of doing so.
I don't know AIX or the /aha file system so the following are just a few general comments to help with this feature.
I added AbstractPoller a long time ago as the base class for implementation so my reading of this patch is that it works here too.
If I understand correctly then watching a directory involves create a mirror in /aha. I see the patch uses Files.createDirectories for that. I suspect you really want toRealPath rather than normalise + toAbsolutePath. Calling Files means you are calling the API from the provider which will be problematic if the default provider is configured to be something else. Also I'm quite sure this won't work with a security manager as the code is missing a doPrivileged.
A general point about the NIO area is that we've always tried to keep the native methods as simple as possible, preferably just call one syscall. The loops in the native methods in the native methods in this patch can all be hoisted to the Java code which will make it much easier to maintain.
A general point of code style is to just to keep it as simple as possible. There are some strange formatting in many areas. Also lines of line 170+ characters are a pain for future side-by-side diffs so keeping lines to sane line would be helpful.
-------------
PR: https://git.openjdk.org/jdk/pull/9281
More information about the nio-dev
mailing list