RFR: 8259894: refactor parts of jvm.h into jvm_io.h and jvm_constants.h

[David Holmes]

On Wed, 20 Jan 2021 04:33:56 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> jvm.h is over 1000 lines long. It is included by 940 out of 1000 HotSpot .o files. However, most of the HotSpot source files just use a small fraction of jvm.h:
> 
> - the jio_xxxprintf() functions
> - the JVM_RECOGNIZED_XXX_MODIFIERS defines
> 
> We should move these two parts into jvm_io.h and jvm_constants.h. This reduces the files that include jvm.h to about 82 files. Build time of HotSpot is reduced by about 0.4%.
> 
> Testing: mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

Hi Ioi,

This was less noisy than I expected but I have a few comments on it.

Comments on jvm_constants.h apply to jvm_io.h too.
Thanks,
David

src/hotspot/share/include/jvm_constants.h line 8:

> 6:  * under the terms of the GNU General Public License version 2 only, as
> 7:  * published by the Free Software Foundation.  Oracle designates this
> 8:  * particular file as subject to the "Classpath" exception as provided

Classpath exception is wrong here - that is only for (mainly Java) sources that applications may link against. It's existing use in jvm.h is incorrect.

src/hotspot/share/include/jvm_constants.h line 26:

> 24:  */
> 25: 
> 26: #ifndef _JAVASOFT_JVM_CONSTANTS_H_

We don't need to keep the legacy JAVASOFT defines.

src/hotspot/share/logging/logSelection.cpp line 25:

> 23:  */
> 24: #include "precompiled.hpp"
> 25: #include "jvm.h"

Not jvm_io.h ??

src/hotspot/share/include/jvm.h line 34:

> 32: #include "jvm_md.h"
> 33: #include "jvm_constants.h"
> 34: #include "jvm_io.h"

Note that the comment block starting at line 40 needs to be updated now you have refactored things.

src/hotspot/share/classfile/javaClasses.cpp line 26:

> 24: 
> 25: #include "precompiled.hpp"
> 26: #include "jvm.h"

Does this really need jvm.h or just jvm_constants.h and jvm_io.h?

src/hotspot/share/classfile/vmIntrinsics.cpp line 26:

> 24: 
> 25: #include "precompiled.hpp"
> 26: #include "jvm.h"

Does this really need jvm.h or just jvm_constants.h and jvm_io.h?

src/hotspot/share/runtime/os.hpp line 28:

> 26: #define SHARE_RUNTIME_OS_HPP
> 27: 
> 28: #include "jvm_md.h"

jvm_md.h should not be included directly by any file - it is an internal implementation detail.

-------------

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2154