[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-dev
Subject:    Re: RFR: 8191860: Add perfData.inline.hpp
From:       Stefan Karlsson <stefan.karlsson () oracle ! com>
Date:       2017-11-29 10:52:34
Message-ID: 6ce735d0-b7b5-ae79-376c-da86e7b15705 () oracle ! com
[Download RAW message or body]

Hi David,

On 2017-11-29 11:11, David Holmes wrote:
> Hi Stefan,
> 
> On 24/11/2017 11:21 PM, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to create a perfData.inline.hpp file and move 
>> inline functions in perfData.hpp that depend on functions in other 
>> .inline.hpp files.
>>
>> http://cr.openjdk.java.net/~stefank/8191860/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8191860
>>
>> Note 1: I consider growableArray.hpp to be an .inline.hpp in disguise, 
>> since it includes allocation.inline.hpp.
> 
> Is that why you didn't #include it instead of forward declaring:
> 
> + template <typename T> class GrowableArray;
> 
> ?

Yes, this was the initial motivation.

However, I do think its nice to only forward declare classes, where it's 
possible, so that we don't have to bring in dependencies against other 
header files unnecessarily. This is extra important given that a lot of 
our header files transitively include more than they need.

> 
>> Note 2: Some .hpp files that used to include perfData.hpp now 
>> explicitly includes allocation.inline.hpp. We should deal with that, 
>> but in another RFE.
> 
> Yes we should deal with that - but the fanout from these changes can get 
> huge. I don't relish trying to refactor arguments.hpp into 
> arguments.inline.hpp for example.


I've created a RFE and a set of patches to do this clean-up:
https://bugs.openjdk.java.net/browse/JDK-8192061

I'll send it out shortly.

Thanks,
StefanK

> 
> Cheers,
> David
> 
>> thanks,
>> StefanK
[prev in list] [next in list] [prev in thread] [next in thread]