Looks good to me. /Magnus
> 3 aug. 2018 kl. 00:21 skrev David Holmes <david.hol...@oracle.com>: > >> On 3/08/2018 4:59 PM, Baesken, Matthias wrote: >> Thank you David , can the change be pushed , or do I need a second >> review for an XS change ? >> (any way a second review would be good 😊 ) > > Need a review from official build team member :) > > Cheers, > David > >> Best regards, Matthias >>> -----Original Message----- >>> From: David Holmes <david.hol...@oracle.com> >>> Sent: Freitag, 3. August 2018 08:18 >>> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie >>> <magnus.ihse.bur...@oracle.com> >>> Cc: build-dev@openjdk.java.net >>> Subject: Re: RFR [XS]: 8208744: remove unneeded -DUSE_MMAP settings for >>> JDK native libs builds -was : RE: unneeded -DUSE_MMAP in JDK native libs >>> builds >>> >>> Looks fine to me. >>> >>> Thanks, >>> David >>> >>>> On 3/08/2018 4:13 PM, Baesken, Matthias wrote: >>>> Hello, I can confirm what David said . >>>> Additionally I grepped through /usr/include on my Linux machine and did >>> not find any USE_MMAP occurences. >>>> >>>> I created this webrev + bug : >>>> >>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8208744/ >>>> >>>> https://bugs.openjdk.java.net/browse/JDK-8208744 >>>> >>>> >>>> Please review ! >>>> >>>> >>>> Thanks, Matthias >>>> >>>> >>>>> -----Original Message----- >>>>> From: David Holmes <david.hol...@oracle.com> >>>>> Sent: Freitag, 3. August 2018 02:56 >>>>> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, >>>>> Matthias <matthias.baes...@sap.com> >>>>> Cc: build-dev@openjdk.java.net >>>>> Subject: Re: unneeded -DUSE_MMAP in JDK native libs builds >>>>> >>>>>> On 3/08/2018 3:44 AM, Magnus Ihse Bursie wrote: >>>>>> >>>>>>> 2 aug. 2018 kl. 05:33 skrev Baesken, Matthias >>>>> <matthias.baes...@sap.com>: >>>>>>> >>>>>>> Hello I noticed that -DUSE_MMAP is set for these JDK native libs, >>>>>>> but >>> I >>>>> cannot find it in the code (for libzip it is different there the flag >>>>> shows up >>> in >>>>> the code too ) : >>>>>> >>>>>> I've always assumed that this was used to control the behavior in some >>>>> imported header files. Have you verified that this is not the case? >>>>> >>>>> It is only used in >>>>> >>>>> ./java.base/share/native/libzip/zip_util.c >>>>> ./java.base/share/native/libzip/zip_util.h >>>>> >>>>> and the header itself is only included in the .c file. So unless this >>>>> source file is used for the other libraries (which I don't see) then we >>>>> don't need -DUSE_MMAP when building them. >>>>> >>>>> AFAIKS the use of this for BUILD_LIBDT_SOCKET and >>> BUILD_LIBDT_SHMEM >>>>> dates back to the build-infra changes in JDK 8. >>>>> >>>>> http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/9d3d01aca52c >>>>> >>>>> But I can't see why it was introduced there. >>>>> >>>>> In JDK 7 it's only applied to the zip library. >>>>> >>>>> Cheers, >>>>> David >>>>> ----- >>>>> >>>>>> >>>>>> /Magnus >>>>>> >>>>>>> >>>>>>> Lib-jdk.jdi.gmk : >>>>>>> >>>>>>> 32 $(eval $(call SetupJdkLibrary, BUILD_LIBDT_SHMEM, \ >>>>>>> ...... >>>>>>> 35 CFLAGS := $(CFLAGS_JDKLIB) -DUSE_MMAP, \ >>>>>>> >>>>>>> >>>>>>> Lib-jdk.jdwp.agent.gmk : >>>>>>> >>>>>>> 30 $(eval $(call SetupJdkLibrary, BUILD_LIBDT_SOCKET, \ >>>>>>> ...... >>>>>>> 33 CFLAGS := $(CFLAGS_JDKLIB) -DUSE_MMAP \ >>>>>>> >>>>>>> >>>>>>> Any objections to remove those 2 -DUSE_MMAP settings ? >>>>>>> If it is fine to remove I would prepare a webrev . >>>>>>> >>>>>>> Thanks, Matthias >>>>>>
