RFR 8209645: Split ClassLoaderData and ClassLoaderDataGraph into separate files

[Thomas Stüfe]

On Thu, Sep 27, 2018 at 11:09 PM,  <coleen.phillimore at oracle.com> wrote:
>
>
> On 9/27/18 4:31 PM, Thomas Stüfe wrote:
>>
>> Hi Coleen,
>>
>> this looks good (not an easy patch to review, but I think all that
>> code just moved without changing).
>>
>> Are all includes in the new classLoaderDataGraph.cpp really needed ? I
>> found e.g. no reference to SymbolTable, so I am not sure
>> symbolTable.hpp is needed. I may be off though.
>
>
> When I did this origially, I tried to minimize #includes, but I'll see if
> there's any I can remove.
> Yes, I can remove systemDictionary.hpp and symbolTable.hpp.
>
>>
>> Does ClassLoaderDataGraphMetaspaceIterator have to be implemented
>> fully in a header (classLoaderDataGraph.hpp)? But this preceedes your
>> change.
>
> from grep:
> memory/metaspace.cpp:  ClassLoaderDataGraphMetaspaceIterator iter;
>

Sorry, I was not clear. What I oringally meant was whether the
implementations of ClassLoaderDataGraphMetaspaceIterator methods have
to be inline or can be moved to a cpp file, in order to get rid of
"#include metaspace.hpp" in classLoaderDataGraph.hpp.

However, I did not think this thru. In classLoaderDataGraph.hpp, if
all we need from metaspace.hpp is ClassLoaderMetaspace* we could just
forward-declare ClassLoaderMetaspace instead of including
metaspace.hpp. No need to move method bodies to cpp files.

Oh, and the patch looks fine as it is to me, I do not need to see
another webrev. If you manage to comb unnecessary includes out, that
would be just an added bonus.

Thanks, Thomas

> I think it has to be in the header file.
>
> Thanks!
> Coleen
>
>>
>> Thanks for making classLoaderData.cpp smaller :)
>>
>> Cheers, Thomas
>>
>>
>>
>>
>> On Thu, Sep 27, 2018 at 7:51 PM,  <coleen.phillimore at oracle.com> wrote:
>>>
>>> I think this might be as good of a time as any to split these files.
>>> This
>>> should help with too much coupling between the graph and the class loader
>>> data itself.  The only function changed and not moved is added an API in
>>> ClassLoaderDataGraph.
>>>
>>>   110   static void adjust_saved_class(ClassLoaderData* cld);
>>>   111   static void adjust_saved_class(Klass* klass);
>>>
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8209645.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8209645
>>>
>>> Tested for mach5 hs-tier1-3
>>>
>>> Thanks,
>>> Coleen
>
>