RFR(M) 8252526 Remove excessive inclusion of jvmti.h and jvmtiExport.hpp

Ioi Lam ioi.lam at oracle.com
Tue Sep 1 01:17:07 UTC 2020



On 8/31/20 4:05 PM, David Holmes wrote:
> Hi Ioi,
>
> I haven't looked at the code changes ...
>
> On 1/09/2020 4:13 am, Ioi Lam wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8252526
>> http://cr.openjdk.java.net/~iklam/jdk16/8252526-fix-jvmti-hpp.v01/
>>
>> (I marked this RFR as "M" because 63 files have changed, but most of 
>> the are
>> just adding a missing #include "prims/jvmtiExport.hpp").
>>
>> jvmti.h is included 905 times and jvmtiExport.hpp is included 776 times
>> (by 971 hotspot .o files). Most of these are unnecessarily included 
>> by the
>> following 3 popular header files:
>>
>> [1] javaClasses.hpp: ThreadStatus is rarely used, and should be moved
>>      to javaThreadStatus.hpp. I also converted the enum to an C++ 11
>>      enum class for better type safety. (see also JDK-8247938).
>>
>> [2] os.hpp: No need to include jvm.h. Use forward declaration
>>      "typedef struct _jvmtiTimerInfo jvmtiTimerInfo;" instead.
>
> That does not seem reasonable to me. It is one thing to do a simple 
> forward declaration of a class but this is an internal detail of JVMTI 
> which os.hpp has no business knowing about.
>

How about changing jvmti.h from:

struct _jvmtiTimerInfo;
typedef struct _jvmtiTimerInfo jvmtiTimerInfo;

to

struct jvmtiTimerInfo;
typedef struct jvmtiTimerInfo jvmtiTimerInfo;

Then os.hpp can declare:

struct jvmtiTimerInfo;

I wonder why we use the _ prefix. It seems like an anachronism to me. Do 
we still support C compilers that cannot handle "typedef struct Foo Foo;"

>> [3] thread.hpp: No need to include jvmExport.hpp. Use forward 
>> declaration
>>      for JvmtiSampledObjectAllocEventCollector and
>>      JvmtiVMObjectAllocEventCollector instead.
>>
>>
>> The total number of includes have reduced from 252033 to 250001. 
>> Build time of
>> slow-debug libjvm.so is reduced from 2:07 to 2:04 on my machine.
>
> I'm not sure why we really care. The build times are not that 
> problematic that it warrants some of these header file hacks IMO. 

My main goal is to speed up incremental builds. Currently we have more 
than 200 header files that are included by more than half of the .o 
files of hotspot. I would often get massive rebuilds after touching a 
header file. Removing the unnecessary dependencies will help developer 
productivity.

Also, currently we do a lot of build validations in tier5 and we had 
consistently missed build problems with the minimal build. I suggested 
to move those builds to tier1, but that was turned down because the 
builds are not fast enough.

> Are those figures with or without PCH?
>
I tested on Linux without PCH.

Thanks
- Ioi

> David
> -----
>
>>
>> Thanks
>> - Ioi
>>



More information about the hotspot-runtime-dev mailing list