RFR: 8073389: Remove the include of resourceArea.hpp from classFileParser.hpp

[Stefan Karlsson]

Hi  David,

On 2015-02-18 12:36, David Holmes wrote:
> On 18/02/2015 7:35 PM, Stefan Karlsson wrote:
>> Hi,
>>
>> Please review this patch to get rid of the inclusion of resourceArea.hpp
>> from classFileParser.hpp.
>>
>>  From the RFE:
>>
>> The inclusion of resourceArea.hpp in classFileParser.hpp is causing
>> cyclic dependencies when I'm changing unrelated code. The main reason
>> for this is that a lot of implementation is put inside the
>> resourceArea.hpp file instead of a .cpp file.
>
> I must be missing something here - the implementation in the .hpp file 
> is because all of the functions are implicitly inline. No guarantee 
> they will be inlined of course but at least in product mode many of 
> them should be. So assuming this is a good thing and we want to keep 
> that for performance then the fix would be to introduce a .inline.hpp 
> file, not to move stuff to a .cpp file.

Yes, you're right, if we assume that these functions are performance 
critical. Personally, I'm not so sure that they were put in the header 
file for performance reasons. But since I'm not changing 
resourceArea.hpp I don't have to figure it out at this point.

>
>
>> I've opted to go the easy route now and get rid of the the
>> resourceArea.hpp dependency from classFileParser.hpp, but eventually it
>> would be good to fix that file.
>>
>> This patch has to add explicit includes of resourceArea.hpp to other
>> .hpp files, that used to get their include from classFileParser.hpp. I
>> could have gotten rid of those dependencies as well, but I chose to not
>> do that for this patch.
>>
>> http://cr.openjdk.java.net/~stefank/8073389/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8073389
>
> What was classFileParser.hpp using from resourceArea.hpp ?
>
> How does the change to src/share/vm/services/runtimeService.cpp fit in ??

There is an include path that was removed when I cleaned up 
classFileParser.hpp:

runtimeServices.cpp
  classLoader.hpp
   classFileParser.hpp
    --- The following branch was pruned ---
    resourceArea.hpp
     thread.inline.hpp
      thread.hpp
       threadLocalAllocBuffer.hpp
        vm_version.hpp

So, I added vm_version.hpp to runtimeService.cpp since it uses 
Abstract_VM_Version in vm_version.hpp.

>
> Seems okay - the proof is in the building as always. Need to check 
> with and without precompiled headers.

It passes JPRT, which runs without PCH on Solaris. I've compiled with 
and without PCH on Linux x64.

> And copyright dates need updating.

Sure.

Thanks,
StefanK

>
> Cheers,
> David
>
>> Thanks,
>> StefanK