On 3/25/16 2:03 AM, Erik Joelsson wrote:
Hello,

Here is the initial review for the new Hotspot Build System, as described in " JEP 284: New HotSpot Build System". This patch adds the new build system along side the old and makes the new system the default. The old build system will remain for a (hopefully) short while until we feel confident it is no longer needed. This enables us to iron out any details that we might have missed with minimal disruption for the users. The goal is to remove the old system after one week of the new going in. During that time, both build systems will have to be kept in sync. For that to be possible, all changes touching anything in the make directory need to be reviewed by me.

In this patch, the makefiles for the new build system are located in hotspot/makefiles. When we apply the second phase, where we remove the old build system, the new will move into the proper hotspot/make directory.

To activate the old build system after this patch has been applied, use the configure arg "--disable-new-hotspot-build".

For more information about how the new build works and how to interact with it, Magnus wrote a document that is still relevant: http://hg.openjdk.java.net/build-infra/jdk9/file/tip/support/new-hotspot-build.md

> The hotspot build differs from all other libraries in the JDK in that the
> library is (potentially) built multiple times with different conditions (-D > flags, for instance). The most common combination is building both the 'client' > and 'server' variant, but other combinations are possible. While this state of > affairs is not universally appreciated :-), it still is a use case that we need
> to support, and it affects the entire build system for hotspot.

Thanks for the humor and for retaining this important difference in
the way HotSpot builds versus more sane/simple systems.


> 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*.

The main method of verification for this patch has been running the compare.sh script to verify that the output is equivalent to the old build in as many cases as possible. In most configurations we have reached a high level of confidence that we produce equivalent binaries, but there are some exceptions that should be mentioned:

* Solaris sparcv9 slowdebug produces differences when comparing disassembly output from libjvm.so. I have not been able to find any meaningful differences in compiler or linker flags to explain this. * Windows server jvm.dll ends up with some functions in different order in the disassembly output. From what I can tell, the bits are otherwise equivalent.

We have also run the runtime nightlies with no notable failures.

This is a pretty big patch and I expect it to take some time to get properly reviewed. It contains contributions from Magnus Ihse Bursie, Erik Joelsson and Ingemar Ã…berg.

Bug: https://bugs.openjdk.java.net/browse/JDK-8152666
Webrev: http://cr.openjdk.java.net/~erikj/8152666/webrev.01/index.html

General
Please make sure all the copyrights are updated.


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'?

    L303:         JVM_CFLAGS="$JVM_CFLAGS -fPIC"
        Same question about literal '-fPIC'.

    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.

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... :-|

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?

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'

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.

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...

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".

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...

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...

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'

    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.

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

Reply via email to