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

[Coleen Phillimore]

On 2/18/15, 7:17 AM, Stefan Karlsson wrote:
> 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.

I don't see how error reporting functions should be performance 
critical.  I think they're better off in the .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 for doing the copyrights.
Coleen

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