RFR: JDK-8205055: Fix compilation optimization errors with new Solaris toolchain
Locally tested the fix below and via mach5 jobs. The ~50 failed tests all completed successfully. It'd be good for Erik to repeat his full testing with this patch included in case it blocked other issues from being observed. This fix can be integrated after the sync from jdk/client. Issue: https://bugs.openjdk.java.net/browse/JDK-8205055 Proposed fix: diff --git a/make/hotspot/lib/JvmOverrideFiles.gmk b/make/hotspot/lib/JvmOverrideFiles.gmk --- a/make/hotspot/lib/JvmOverrideFiles.gmk +++ b/make/hotspot/lib/JvmOverrideFiles.gmk @@ -74,6 +74,8 @@ else ifeq ($(OPENJDK_TARGET_OS), solaris) ifneq ($(DEBUG_LEVEL), slowdebug) + # dev studio 12.6 workaround + BUILD_LIBJVM_arguments.cpp_OPTIMIZATION := LOW
Re: RFR: JDK-8204973: Add build support for filtering translations
On 2018-06-14 12:49, Magnus Ihse Bursie wrote: Would it have been to problematic to reverse the the logic, i.e. having a "include translation" rather than an exclude? Feels more brittle; if someone adds a translation the exclude list needs to be updated. Also, a include mechanism could possibly be used, much simpler, by someone who only wants to build e.g. a chinese jdk. I agree that include would have been easier to use, and I was initially trying to implement it that way, but couldn't come up with an effective solution in make. You need to identify all source files that have locale suffixes and only filter among them. With the exclude, at least the build logic is reasonably straight forward since the user defines the locale strings that define a translation that warrants action. This is why I added a test that uses include logic, hoping that the combination would prove resilient enough to catch such changes. The test is still a bit brittle since it can easily catch false positives. But I realize the contraints by the somewhat odd request from the bug report, to deliver just a subset of the available translations. So I'm okay with the patch. Thanks! /Erik /Magnus On 2018-06-14 20:52, Erik Joelsson wrote: Hello, Here is a new version of the patch: http://cr.openjdk.java.net/~erikj/8204973/webrev.02/ Changes from last time: * Made the regexp for finding locales more correct. It still does not try to match 3 letter language strings because doing so triggers a large amount of false positives in our souce tree. * Added another accepted locale (en_US_POSIX) that is now matched by the improved regexp. * Added more locales to the exclude list as they were now discovered by the improved regexp. /Erik On 2018-06-13 12:47, Erik Joelsson wrote: Hello, Oracle will reduce the number of languages that it maintains translations of JDK resources for. The current translations will remain in the source for now, but we need a way to filter out a set of translations at build time so that we only include the ones we support. This patch adds such a configuration option. It also changes how Oracle builds by using the option to exclude all translations except English, Japanese, Simplified Chinese and Traditional Chinese. Anyone else building OpenJDK will by default include all translations present in the source, just as before. I added a test that verifies this for builds with the "IMPLEMENTOR" field in the release file set to "Oracle Corporation". The test will not be run for other OpenJDK builds. I had to modify an existing test for java.logging which used various translations to verify localized log messages to only use translations that Oracle chooses to include. Since this is the second test that specifically verifies build behavior, I moved the previous such test together with this new test into a common top level test directory "build", under the jdk test root. I put these tests in the jdk tier3 test group. I have run all tier1, 2 and 3 tests in Mach 5 as well as specifically looked for tests that use the java.util.Locale class and ran them locally. Webrev: http://cr.openjdk.java.net/~erikj/8204973/webrev.01/index.html Bug: https://bugs.openjdk.java.net/browse/JDK-8204973 /Erik
Re: RFR: JDK-8204973: Add build support for filtering translations
Looks good to me. Naoto On 6/14/18 11:52 AM, Erik Joelsson wrote: Hello, Here is a new version of the patch: http://cr.openjdk.java.net/~erikj/8204973/webrev.02/ Changes from last time: * Made the regexp for finding locales more correct. It still does not try to match 3 letter language strings because doing so triggers a large amount of false positives in our souce tree. * Added another accepted locale (en_US_POSIX) that is now matched by the improved regexp. * Added more locales to the exclude list as they were now discovered by the improved regexp. /Erik On 2018-06-13 12:47, Erik Joelsson wrote: Hello, Oracle will reduce the number of languages that it maintains translations of JDK resources for. The current translations will remain in the source for now, but we need a way to filter out a set of translations at build time so that we only include the ones we support. This patch adds such a configuration option. It also changes how Oracle builds by using the option to exclude all translations except English, Japanese, Simplified Chinese and Traditional Chinese. Anyone else building OpenJDK will by default include all translations present in the source, just as before. I added a test that verifies this for builds with the "IMPLEMENTOR" field in the release file set to "Oracle Corporation". The test will not be run for other OpenJDK builds. I had to modify an existing test for java.logging which used various translations to verify localized log messages to only use translations that Oracle chooses to include. Since this is the second test that specifically verifies build behavior, I moved the previous such test together with this new test into a common top level test directory "build", under the jdk test root. I put these tests in the jdk tier3 test group. I have run all tier1, 2 and 3 tests in Mach 5 as well as specifically looked for tests that use the java.util.Locale class and ran them locally. Webrev: http://cr.openjdk.java.net/~erikj/8204973/webrev.01/index.html Bug: https://bugs.openjdk.java.net/browse/JDK-8204973 /Erik
Re: RFR: JDK-8204973: Add build support for filtering translations
Would it have been to problematic to reverse the the logic, i.e. having a "include translation" rather than an exclude? Feels more brittle; if someone adds a translation the exclude list needs to be updated. Also, a include mechanism could possibly be used, much simpler, by someone who only wants to build e.g. a chinese jdk. But I realize the contraints by the somewhat odd request from the bug report, to deliver just a subset of the available translations. So I'm okay with the patch. /Magnus On 2018-06-14 20:52, Erik Joelsson wrote: Hello, Here is a new version of the patch: http://cr.openjdk.java.net/~erikj/8204973/webrev.02/ Changes from last time: * Made the regexp for finding locales more correct. It still does not try to match 3 letter language strings because doing so triggers a large amount of false positives in our souce tree. * Added another accepted locale (en_US_POSIX) that is now matched by the improved regexp. * Added more locales to the exclude list as they were now discovered by the improved regexp. /Erik On 2018-06-13 12:47, Erik Joelsson wrote: Hello, Oracle will reduce the number of languages that it maintains translations of JDK resources for. The current translations will remain in the source for now, but we need a way to filter out a set of translations at build time so that we only include the ones we support. This patch adds such a configuration option. It also changes how Oracle builds by using the option to exclude all translations except English, Japanese, Simplified Chinese and Traditional Chinese. Anyone else building OpenJDK will by default include all translations present in the source, just as before. I added a test that verifies this for builds with the "IMPLEMENTOR" field in the release file set to "Oracle Corporation". The test will not be run for other OpenJDK builds. I had to modify an existing test for java.logging which used various translations to verify localized log messages to only use translations that Oracle chooses to include. Since this is the second test that specifically verifies build behavior, I moved the previous such test together with this new test into a common top level test directory "build", under the jdk test root. I put these tests in the jdk tier3 test group. I have run all tier1, 2 and 3 tests in Mach 5 as well as specifically looked for tests that use the java.util.Locale class and ran them locally. Webrev: http://cr.openjdk.java.net/~erikj/8204973/webrev.01/index.html Bug: https://bugs.openjdk.java.net/browse/JDK-8204973 /Erik
Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default
Hi Volker,
http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/make/autoconf/hotspot.m4.udiff.html
Seems like a roundabout way to have a platform specific default value.
Why not determine a default value beforehand:
if test "x$OPENJDK_TARGET_OS" = "xaix"; then
ENABLE_CDS_DEFAULT="false"
else
ENABLE_CDS_DEFAULT=true"
fi
AC_ARG_ENABLE([cds], [AS_HELP_STRING([--enable-cds@<:@=yes/no/auto@:>@],
[enable class data sharing feature in non-minimal VM. Default is
${ENABLE_CDS_DEFAULT}.])])
and so on?
See also what we did for "8202325: [aix] disable warnings-as-errors by default".
--
http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/src/hotspot/share/classfile/javaClasses.cpp.udiff.html
Here, do we really need to exclude this from compiling,
DumpSharedSpaces = false is not enough?
Best Regards, Thomas
On Thu, Jun 14, 2018 at 4:26 PM, Volker Simonis
wrote:
> Hi,
>
> can I please have a review for the following fix:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
> https://bugs.openjdk.java.net/browse/JDK-8204965
>
> CDS does currently not work on AIX because of the way how we
> reserve/commit memory on AIX. The problem is that we're using a
> combination of shmat/mmap depending on the page size and the size of
> the memory chunk to reserve. This makes it impossible to reliably
> reserve the memory for the CDS archive and later on map the various
> parts of the archive into these regions.
>
> In order to fix this we would have to completely rework the memory
> reserve/commit/uncommit logic on AIX which is currently out of our
> scope because of resource limitations.
>
> Unfortunately, I could not simply disable CDS in the configure step
> because some of the shared code apparently relies on parts of the CDS
> code which gets excluded from the build when CDS is disabled. So I
> also fixed the offending parts in hotspot and cleaned up the configure
> logic for CDS.
>
> Thank you and best regards,
> Volker
>
> PS: I did run the job through the submit forest
> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
> weren't really useful because they mention build failures on linux-x64
> which I can't reproduce locally.
Re: RFR: JDK-8204973: Add build support for filtering translations
Hello, Here is a new version of the patch: http://cr.openjdk.java.net/~erikj/8204973/webrev.02/ Changes from last time: * Made the regexp for finding locales more correct. It still does not try to match 3 letter language strings because doing so triggers a large amount of false positives in our souce tree. * Added another accepted locale (en_US_POSIX) that is now matched by the improved regexp. * Added more locales to the exclude list as they were now discovered by the improved regexp. /Erik On 2018-06-13 12:47, Erik Joelsson wrote: Hello, Oracle will reduce the number of languages that it maintains translations of JDK resources for. The current translations will remain in the source for now, but we need a way to filter out a set of translations at build time so that we only include the ones we support. This patch adds such a configuration option. It also changes how Oracle builds by using the option to exclude all translations except English, Japanese, Simplified Chinese and Traditional Chinese. Anyone else building OpenJDK will by default include all translations present in the source, just as before. I added a test that verifies this for builds with the "IMPLEMENTOR" field in the release file set to "Oracle Corporation". The test will not be run for other OpenJDK builds. I had to modify an existing test for java.logging which used various translations to verify localized log messages to only use translations that Oracle chooses to include. Since this is the second test that specifically verifies build behavior, I moved the previous such test together with this new test into a common top level test directory "build", under the jdk test root. I put these tests in the jdk tier3 test group. I have run all tier1, 2 and 3 tests in Mach 5 as well as specifically looked for tests that use the java.util.Locale class and ran them locally. Webrev: http://cr.openjdk.java.net/~erikj/8204973/webrev.01/index.html Bug: https://bugs.openjdk.java.net/browse/JDK-8204973 /Erik
Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default
Hi Volker, The changes look good to me overall. I’ll refer to the JVMTI experts for jvmtiEnv.cpp change. I have a question for the change in vmStructs.cpp. Any reason why only _current_info needs CDS_ONLY? // \ /* FileMapInfo fields (CDS archive related) */ \ // \ \ nonstatic_field(FileMapInfo, _header, FileMapInfo::FileMapHeader*) \ - static_field(FileMapInfo, _current_info, FileMapInfo*) \ + CDS_ONLY(static_field(FileMapInfo,_current_info, FileMapInfo*)) \ nonstatic_field(FileMapInfo::FileMapHeader, _space[0], FileMapInfo::FileMapHeader::space_info)\ nonstatic_field(FileMapInfo::FileMapHeader::space_info, _addr._base, char*) \ nonstatic_field(FileMapInfo::FileMapHeader::space_info, _used, size_t)\ \ Thanks, Jiangli > On Jun 14, 2018, at 7:26 AM, Volker Simonis wrote: > > Hi, > > can I please have a review for the following fix: > > http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/ > https://bugs.openjdk.java.net/browse/JDK-8204965 > > CDS does currently not work on AIX because of the way how we > reserve/commit memory on AIX. The problem is that we're using a > combination of shmat/mmap depending on the page size and the size of > the memory chunk to reserve. This makes it impossible to reliably > reserve the memory for the CDS archive and later on map the various > parts of the archive into these regions. > > In order to fix this we would have to completely rework the memory > reserve/commit/uncommit logic on AIX which is currently out of our > scope because of resource limitations. > > Unfortunately, I could not simply disable CDS in the configure step > because some of the shared code apparently relies on parts of the CDS > code which gets excluded from the build when CDS is disabled. So I > also fixed the offending parts in hotspot and cleaned up the configure > logic for CDS. > > Thank you and best regards, > Volker > > PS: I did run the job through the submit forest > (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results > weren't really useful because they mention build failures on linux-x64 > which I can't reproduce locally.
Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default
Build changes look ok. /Erik On 2018-06-14 07:26, Volker Simonis wrote: Hi, can I please have a review for the following fix: http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/ https://bugs.openjdk.java.net/browse/JDK-8204965 CDS does currently not work on AIX because of the way how we reserve/commit memory on AIX. The problem is that we're using a combination of shmat/mmap depending on the page size and the size of the memory chunk to reserve. This makes it impossible to reliably reserve the memory for the CDS archive and later on map the various parts of the archive into these regions. In order to fix this we would have to completely rework the memory reserve/commit/uncommit logic on AIX which is currently out of our scope because of resource limitations. Unfortunately, I could not simply disable CDS in the configure step because some of the shared code apparently relies on parts of the CDS code which gets excluded from the build when CDS is disabled. So I also fixed the offending parts in hotspot and cleaned up the configure logic for CDS. Thank you and best regards, Volker PS: I did run the job through the submit forest (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results weren't really useful because they mention build failures on linux-x64 which I can't reproduce locally.
RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default
Hi, can I please have a review for the following fix: http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/ https://bugs.openjdk.java.net/browse/JDK-8204965 CDS does currently not work on AIX because of the way how we reserve/commit memory on AIX. The problem is that we're using a combination of shmat/mmap depending on the page size and the size of the memory chunk to reserve. This makes it impossible to reliably reserve the memory for the CDS archive and later on map the various parts of the archive into these regions. In order to fix this we would have to completely rework the memory reserve/commit/uncommit logic on AIX which is currently out of our scope because of resource limitations. Unfortunately, I could not simply disable CDS in the configure step because some of the shared code apparently relies on parts of the CDS code which gets excluded from the build when CDS is disabled. So I also fixed the offending parts in hotspot and cleaned up the configure logic for CDS. Thank you and best regards, Volker PS: I did run the job through the submit forest (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results weren't really useful because they mention build failures on linux-x64 which I can't reproduce locally.
