Proposal for adding O_DIRECT support into JDK 9

[Lu, Yingqi]

Hi Volker,

Thank you for the quick reply. I will try your fix today and I am sure it will work!

Please hold on to the testing since webrev.08 will be out hopefully tomorrow. In the new version, we fixed a handful issues Brain, you and Hamlin pointed out. You might want to directly test the coming new version :)

Thanks to everyone for all your great feedback. Really appreciate!!

Thanks!
Lucy

>-----Original Message-----
>From: Volker Simonis [mailto:volker.simonis at gmail.com]
>Sent: Monday, September 04, 2017 10:42 AM
>To: Lu, Yingqi <yingqi.lu at intel.com>
>Cc: Brian Burkhalter <brian.burkhalter at oracle.com>; nio-dev at openjdk.java.net
>Subject: Re: Proposal for adding O_DIRECT support into JDK 9
>
>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