RFR: JDK-8152666: The new Hotspot Build System
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Thu Apr 7 09:57:21 UTC 2016
On 2016-04-06 11:10, Erik Joelsson wrote:
> Hello Dan and thank you for the review! I know it's a lot to chew
> through.
>
> I have incorporated your changes and published a new webrev:
> http://cr.openjdk.java.net/~erikj/8152666/webrev.02/
I'm not sure if I'm formally allowed to be a reviewer, since I've wrote
the absolute majority of the code myself.
Nevertheless, I've looked through the webrev carefully, including the
latest changes by you, and it looks good to me. Ship it! :-)
Just a few minor comments:
In compare.sh.in:
Why the added export of DEBUG_LEVEL? I can't find any reference to it in
the changes in compare.sh. Was it references in some earlier change and
we missed to export it?
In flags.m4/platform.m4:
It is unfortunate that we needed to make the build/target duplication in
this change. It makes the messy addition of the JVM_CFLAGS even messier.
:( But then again, we've always planned a follow-up restructuring of the
flag handling after the integration of the new Hotspot build system. It
just got a bit more urgent.
/Magnus
>
> On 2016-04-05 20:10, Daniel D. Daugherty wrote:
>>
>>
>> > The new build supports the following variants:
>> >
>> > * server (C1+C2)
>>
>> The above "server" variant is the "tiered server". Does the new
>> build system support the "C2 server" variant? What about the
>> 32-bit server and 64-bit server build variants? For example,
>> on Linux you can have:
>>
>> * C1/Client, 32-bit
>> * C2/Server, 32-bit
>> * Tiered (C1 & C2), 32-bit
>> * C2/Server, 64-bit
>> * Tiered (C1 + C2), 64-bit
>>
>> The above wide range of variants is also true for Win*.
>>
> There is a way to achieve this even if it's not as straight forward.
> It's controlled through the new "jvm-feature" setting. To build a
> completely custom set of features for a jvm, you set the
> --with-jvm-variants=custom and then define the full feature set using
> --with-jvm-features=compiler2,... For "server, client, core, minimal,
> zero and zeroshark" there is a predefined set of features while the
> custom variant has no features by default.
>>
>> General
>> Please make sure all the copyrights are updated.
>>
> Done
>>
>> common/autoconf/basics.m4
>> No comments.
>>
>> common/autoconf/build-performance.m4
>> No comments.
>>
>> common/autoconf/buildjdk-spec.gmk.in
>> No comments.
>>
>> common/autoconf/compare.sh.in
>> No comments.
>>
>> common/autoconf/configure
>> No comments.
>>
>> common/autoconf/configure.ac
>> No comments.
>>
>> common/autoconf/flags.m4
>> L274: SHARED_LIBRARY_FLAGS="-dynamiclib
>> -compatibility_version 1.0.0 -current_version 1.0.0 $PICFLAG"
>> L275: JVM_CFLAGS="$JVM_CFLAGS -fPIC"
>>
>> L275 is new, but seeing it next to L274 makes me wonder if
>> $PICFLAG should be used instead of the literal '-fPIC'?
> Fixed
>>
>> L303: JVM_CFLAGS="$JVM_CFLAGS -fPIC"
>> Same question about literal '-fPIC'.
>>
> Not sure, leaving for now. It seems we leave the PICFLAG empty for the
> JDK build and only add it to the hotspot build. This should be
> addressed in a followup where we try to align flag usage more between
> the different libraries.
>> For most of the changes to flags.m4, I can't see how any of it
>> relates to the new HotSpot build.
>>
>> Update: Now I'm wondering if this is one of those files that
>> we typically don't review because it is auto generated.
>> Sorry, don't remember for sure.
> It's a file that should be reviewed, only generated-configure.sh can
> be ignored. The majority of the changes in here are related to cross
> compiling in the modular world. When cross compiling now, we need to
> also build a jvm for the build platform in order to run jlink and jmod
> when building images. With the old hotspot build, that was simpler,
> just invoke the hotspot build with some ARCH and compiler related
> variables set. For the rest of the JDK build, an approximation of
> flags used was enough so the problem was never fully solved.
>
> In the new build, we derive all the compiler options in configure so I
> had to introduce a more proper solution. I did this by parameterizing
> some macros in flags.m4 and platform.m4 so that we can run them twice,
> once for the "target" toolchain" and one for the "build" toolchain.
> These are the majority of the changes you are seeing. I also removed
> the old hard coded "build" versions of certain flag and platform
> variables.
>> common/autoconf/generated-configure.sh
>> 2642 lines changed... I think this is one of those files
>> you're supposed to skip in build-dev review... :-|
> Yes, please do.
>>
>> common/autoconf/help.m4
>> L179: $PRINTF "Which are valid to use depends on the target
>> platform.\n "
>> L180: $PRINTF "%s " $VALID_JVM_FEATURES
>> Why are there blanks after the last '\n' on L179 instead of
>> at the beginning of L180?
>>
> If you do $PRINTF " %s " $VALID_JVM_FEATURES, it adds those spaces
> between every element in VALID_JVM_FEATURES.
>> common/autoconf/hotspot-spec.gmk.in
>> No comments.
>>
>> common/autoconf/hotspot.m4
>> L46: # Check if the specified JVM features are explicitely
>> enabled. To be used in
>> Typo: 'explicitely' -> 'explicitly'
>>
>> L59: # server: normal interpreter, and a tiered C1/C2 compiler
>> So no support for a C2-only server config?
>>
>> L77: # Have the user listed more than one variant?
>> Typo: 'Have' -> 'Has'
>>
> fixed
>> common/autoconf/jdk-options.m4
>> No comments other than to say thanks for keeping support
>> for 'optimized' builds.
>>
>> common/autoconf/jdk-version.m4
>> No comments.
>>
>> common/autoconf/lib-std.m4
>> No comments.
>>
>> common/autoconf/libraries.m4
>> No comments.
>>
>> common/autoconf/platform.m4
>> No comments, but mind numbing amount of diffs.
>>
> Same explanation as for flags.m4
>> common/autoconf/spec.gmk.in
>> No comments.
>>
>> common/autoconf/toolchain.m4
>> No comments.
>>
>> common/autoconf/version-numbers
>> No comments.
>>
>> common/bin/compare.sh
>> No comments.
>>
>> common/bin/compare_exceptions.sh.incl
>> No comments.
>>
>> make/Jprt.gmk
>> No comments.
>>
>> make/Main.gmk
>> No comments other than the 'hotspot-ide-project' target
>> looks interesting...
>>
> This is the replacement for the visual studio project generator. We
> currently only support VS here.
>> make/common/MakeBase.gmk
>> No comments.
>>
>> make/common/NativeCompilation.gmk
>> L649: else ifeq (LOW, $$($1_OPTIMIZATION))
>> L650: $1_OPT_CFLAGS := $(C_O_FLAG_NORM)
>> L651: $1_OPT_CXXFLAGS := $(CXX_O_FLAG_NORM)
>> Instead of "_NORM", I was expecting "_LOW".
>>
>> L652: else ifeq (HIGH, $$($1_OPTIMIZATION))
>> L653: $1_OPT_CFLAGS := $(C_O_FLAG_HI)
>> L654: $1_OPT_CXXFLAGS := $(CXX_O_FLAG_HI)
>> Instead of "_HI" I was expecting "_HIGH".
>>
> The names here were defined way back when we did build infra for the
> JDK build. I wouldn't mind better alignment in naming the optimization
> levels.
>> make/jprt.properties
>> L136: # Don't disable precompiled headers on windows. It's simply
>> too slow.
>> This is a surprise. Not the slowness part, but not being
>> able to do a non-PCH JPRT build on Win*. IMHO, it's a
>> little too much motherhood...
>>
> Actually, the old hotspot build does not allow disabling of PCH for
> windows at all. The flag is simply ignored. In the new build, we treat
> the flag the same on all platforms, so disabling precompiled headers
> works on Windows. In the current JPRT config, we disable precompiled
> headers on all fastdebug builds as a way of making sure we aren't
> breaking that build configuration. We noticed a major build time
> regression on Windows fastdebug builds in JPRT until we figured out it
> was caused by this. Since we aren't currently disabling precompiled
> header on Windows, I see no reason to start now. The build time
> regression for just building hotspot is around 2m->12m.
>> jdk/make/Import.gmk
>> No comments.
>>
>> jdk/make/copy/Copy-java.base.gmk
>> No comments.
>>
>> jdk/make/lib/CoreLibraries.gmk
>> No comments.
>>
>> hotspot/makefiles/BuildHotspot.gmk
>> No comments.
>>
>> hotspot/makefiles/Dist.gmk
>> L52: define macosx_universalize
>> I thought MacOS X universal support was going away?
>>
>> Update: OK, I see the mention of 8069540 ahead...
>>
> Yeah, we need to be binary the same as the old build for now.
> Hopefully we can get rid of the universal stuff soon.
>> L120: # these files are identical, and just pick one arbitrarily
>> to use as souce.
>> Typo: 'souce' -> 'source'
>>
>> L139: # This might have been defined in a custom extenstion
>> Typo: 'extenstion' -> 'extension'
>>
> fixed
>> L168: # NOTE: In the old build, this file was not copied on Windows.
>> L169: ifneq ($(OPENJDK_TARGET_OS), windows)
>> L170: $(eval $(call SetupCopyFiles, COPY_JVMTI_HTML, \
>> I'm not quite sure why the jvmti.html work is done for
>> more than a single platform.
>>
>> Update: Thinking about this more... I vaguely remember that
>> JVM/TI tracing used to be disabled in Client VMs. Don't know
>> if that's still the case.
> The jvmti.html file is just copied into the docs bundle later. IMO,
> the docs bundle should be the same regardless of platform. In practice
> we only publish the bundle from one build platform anyway.
>
> /Erik
>>
>> hotspot/makefiles/HotspotCommon.gmk
>> No comments.
>>
>> hotspot/makefiles/gensrc/GenerateSources.gmk
>> No comments.
>>
>> hotspot/makefiles/gensrc/GensrcAdlc.gmk
>> L98: # NOTE: Windows adlc flags was different in the old
>> build. Is this really
>> L99: # correct?
>> John Rose may know the answer to this historical question.
>>
>> hotspot/makefiles/gensrc/GensrcDtrace.gmk
>> No comments.
>>
>> hotspot/makefiles/gensrc/GensrcJvmti.gmk
>> No comments.
>>
>> hotspot/makefiles/ide/CreateVSProject.gmk
>> No comments.
>>
>> hotspot/makefiles/lib/CompileDtracePostJvm.gmk
>> No comments.
>>
>> hotspot/makefiles/lib/CompileDtracePreJvm.gmk
>> No comments.
>>
>> hotspot/makefiles/lib/CompileJvm.gmk
>> No comments.
>>
>> hotspot/makefiles/lib/CompileLibjsig.gmk
>> No comments.
>>
>> hotspot/makefiles/lib/CompileLibraries.gmk
>> No comments.
>>
>> hotspot/makefiles/lib/JvmFeatures.gmk
>> No comments.
>>
>> hotspot/makefiles/lib/JvmMapfile.gmk
>> No comments.
>>
>> hotspot/makefiles/lib/JvmOverrideFiles.gmk
>> No comments.
>>
>> hotspot/makefiles/mapfiles/libjsig/mapfile-vers-solaris
>> hotspot/makefiles/mapfiles/libjvm_db/mapfile-vers
>> hotspot/makefiles/mapfiles/libjvm_dtrace/mapfile-vers
>> No comments on the mapfiles.
>>
>> hotspot/makefiles/symbols/symbols-aix
>> hotspot/makefiles/symbols/symbols-aix-debug
>> hotspot/makefiles/symbols/symbols-linux
>> hotspot/makefiles/symbols/symbols-macosx
>> hotspot/makefiles/symbols/symbols-shared
>> hotspot/makefiles/symbols/symbols-solaris
>> hotspot/makefiles/symbols/symbols-solaris-dtrace-compiler1
>> hotspot/makefiles/symbols/symbols-solaris-dtrace-compiler2
>> hotspot/makefiles/symbols/symbols-unix
>> No comments on the symbol files.
>>
>>
>> Thumbs up on this fix; I don't think that anything I noted
>> above is a show stopper for this changeset.
>>
>> Dan
>>
>>
>>>
>>> /Erik
>>
>
More information about the build-dev
mailing list