Proposal for adding O_DIRECT support into JDK 9

[Volker Simonis]

Hi Lucy,

On Sat, Sep 2, 2017 at 6:05 PM, Lu, Yingqi <yingqi.lu at intel.com> wrote:
> Hi Brian/Volker,
>
>
>
> I have already addressed all the existing issues into webrev.08. However, I
> found after applying Volker’s patch, JDK does not compile on Linux. Error
> message is listed below:
>
>
>
> error: cannot find symbol
>
>             oflags |= O_DIRECT;
>
>                       ^
>
>   symbol:   variable O_DIRECT
>
>   location: class UnixChannelFactory
>

Sorry for the trouble :(
I've just run into this myself now. I've initially tested my proposal
on AIX only (because it was just a proposal). Now that it seems to get
accepted, I'll have to look at it more generally :)

The problem on Linux is caused by the fact that the build doesn't use
"-D_GNU_SOURCE" when processing UnixConstants.java.template and
therefore "O_DIRECT" isn't visible. In contrast, the compilation of
"FileDispatcherImpl.c" always uses "-D_GNU_SOURCE", so you can freely
access "O_DIRECT".

A quick fix is the following:

diff -r 0c49baac8805 make/gensrc/GensrcMisc.gmk
--- a/make/gensrc/GensrcMisc.gmk        Wed Aug 30 18:54:50 2017 +0200
+++ b/make/gensrc/GensrcMisc.gmk        Mon Sep 04 19:37:35 2017 +0200
@@ -63,7 +63,7 @@
 define generate-preproc-src
        $(call MakeDir, $(@D))
        ( $(NAWK) '/@@END_COPYRIGHT@@/{exit}1' $< && \
-         $(CPP) $(CPP_FLAGS) $(SYSROOT_CFLAGS) $< \
+         $(CPP) $(CPP_FLAGS) $(SYSROOT_CFLAGS) $(CFLAGS_JDKLIB) $< \
              2> >($(GREP) -v '^$(<F)$$' >&2) \
              | $(NAWK) '/@@START_HERE@@/,0' \
              |  $(SED) -e 's/@@START_HERE@@/\/\/ AUTOMATICALLY
GENERATED FILE - DO NOT EDIT/' \

which worked for me on Linux/Solaris/AIX (haven't tested on Mac).

I think it was always a mistake that we haven't generated the
UnixConstants with the same C-Flags that we've also used to compile
the files which used them. It was probably just luck that we've not
run into problems until now.

I'll try your newest version tomorrow and also check the tests on AIX.

>
> In addition, I have two more questions:
>
> 1.      UnixChannelFactory.java is a common class for all the UNIX based OS,
> MacOS and Solaris do not have O_DIRECT flag defined. If we specifically use
> O_DIRECT, will it be an issue?
>

I think it depends. If we only use it on top of the current fcntl()
approach it will be probably fine (but we'll have to define O_DIRECT
to '0' on platforms which don't know it in that case). Otherwise we'll
have to use the corresponding platform flags in
UnixChannelFactory.java

> 2.      Does this approach has conflict with the current implementation with
> fcntl and ReOpenFile() or it is OK to use it on top of the current approach?
>

I'll leave this question up to Brian :)

>
> Thanks,
>
> Lucy
>
>
>
> From: Brian Burkhalter [mailto:brian.burkhalter at oracle.com]
> Sent: Friday, September 01, 2017 4:11 PM
> To: Lu, Yingqi <yingqi.lu at intel.com>
> Cc: nio-dev at openjdk.java.net
> Subject: Re: Proposal for adding O_DIRECT support into JDK 9
>
>
>
> Hi Lucy,
>
>
>
> On Sep 1, 2017, at 7:46 AM, Brian Burkhalter <brian.burkhalter at oracle.com>
> wrote:
>
>
>
> Please take a look at this version and let us know your feedback. I would
> also like to learn what will be a proper approach to solve the AIX issue and
> what kind of details we need to provide for the exception messages. I will
> try to incorporate all of those changes into the next version.
>
> I’ll try to get back to you on these points today.
>
>
>
> Given that the only API-level call which can create a direct FileChannel
> requires the path and opens the channel then perhaps Volker’s patch is OK
> as-is. The use of fcntl() and ReOpenFile() were intended to provide a more
> flexible approach better to accommodate potential future changes.
>
>
>
> As for the exception messages, for example in the case of a number of bytes
> not equalling the block size, it might not be a bad idea to include the
> actual values of the number of bytes and the block size.
>
>
>
> On Sep 1, 2017, at 9:13 AM, Lu, Yingqi <yingqi.lu at intel.com> wrote:
>
>
>
> I will add the Solaris check for all the tests now and wait for your
> feedback on the additional tests. All these changes will go into webrev.08.
>
>
>
> As of now the tests look to be sufficient but I should take another look
> next week after the holiday weekend.
>
>
>
> Thanks,
>
>
>
> Brian