On 2018-08-28 18:25, Erik Joelsson wrote:
Build changes look good to me (but should probably get review from
someone else).
I'm (as usually) not as happy as Erik. ;-)
In Images.gmk, you have added this rule:
$@ -Xshare:dump -Xmx128M -Xms128M $(LOG_INFO)
It took me a while to grasp. You are relying on $(JIMAGE_TARGET_FILE) to
evaluate to bin/java. But that is only incidentally so. This file is
just picked arbitrarily to represent when the entire image is done, for
make. Please use $(JRE_IMAGE_DIR)/bin/java instead.
The logic in jdk-options.m4 is broken. If indeed it is not possible to
use cds archive with zero, then things will break since it will still be
built if using --enable-cds-archive!
What you should do is to set default to true unless using cross
compilation or zero. If the user explicitly sets --enable-cds-archive,
and it's not possible, a fatal error should be generated.
Apart from this, I'm more on Erik's line. :-)
/Magnus
/Erik
On 2018-08-27 13:33, Jiangli Zhou wrote:
Please review the implementation for JEP JDK-8204247
(https://bugs.openjdk.java.net/browse/JDK-8204247). The goal of the
JEP is to include a default CDS archive in JDK 12 binary distribution
(downloadable from http://jdk.java.net/12/). The default CDS archive
is generated using the default classlist (resides in the lib/
directory) at JDK build time. Any comments/suggestions are highly
appreciated.
All makefile changes in the webrev are contributed by Erik Joelsson
(many thanks!!).
This is a combination of efforts from different teams and
individuals. Thanks to everyone who has been involved in the JEP &
implementation discussions, testing and bug fixing!
JEP: https://bugs.openjdk.java.net/browse/JDK-8204247
RFE: https://bugs.openjdk.java.net/browse/JDK-8202951
webrev: http://cr.openjdk.java.net/~jiangli/8202951/webrev.00/
Two sanity test cases for the default CDS archive are included
test/hotspot/jtreg/runtime/SharedArchiveFile. They are not intended
for in-depth CDS functional testing, which is already covered by the
existing cds/appcds tests and all tiered tests executing with the
default CDS archive enabled.
As part of the webrev,
test/jdk/javax/imageio/plugins/png/ItxtUtf8Test.java is also fixed to
use larger java heap (JDK-8209739
, https://bugs.openjdk.java.net/browse/JDK-8209739).
Tests executed:
- several rounds of tier1 - tier8 via mach5
- JCK lang, api and vm tests via mach5
Thanks!
Calvin, Ioi, Jiangli
