RFR(L): 8146401: Clean up oop.hpp: add inline directives and fix header files

[Lindenmaier, Goetz]

Hi Coleen, 

the following webrev includes the changes proposed by you:
http://cr.openjdk.java.net/~goetz/wr16/8146401-oopInline-rt/webrev.02/
Also, I removed the empty file.

Thanks for sponsoring,
  Goetz.

> -----Original Message-----
> From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com]
> Sent: Freitag, 8. Januar 2016 23:22
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: RFR(L): 8146401: Clean up oop.hpp: add inline directives and fix
> header files
> 
> 
> Goetz,
> Thanks for the reply.
> 
> On 1/7/16 10:41 AM, Lindenmaier, Goetz wrote:
> > Hi Coleen,
> >
> > thanks for looking at this!
> >
> >> http://cr.openjdk.java.net/~goetz/wr16/8146401-oopInline-
> >> rt/webrev.01/src/share/vm/code/dependencies.cpp.udiff.html
> >>
> >> Can you fix the preexisting weird indentation of
> >> + assert(_call_site() ->is_a(SystemDictionary::CallSite_klass()), "must
> >> be");
> > Done.
> >
> >> http://cr.openjdk.java.net/~goetz/wr16/8146401-oopInline-
> >> rt/webrev.01/src/share/vm/jvmci/jvmciJavaClasses.hpp.udiff.html
> >>
> >> Don't have an opinion about including the .inline.hpp files in these.
> >> The compiler team should probably clean this up.
> > Yes, I think so too.  It should be done in the compiler repo to avoid
> > clashes.
> >
> >> http://cr.openjdk.java.net/~goetz/wr16/8146401-oopInline-
> >> rt/webrev.01/src/share/vm/oops/objArrayOop.hpp.udiff.html
> >>
> >> Why isn't inline in the declaration for obj_at_put ?
> >> Why is inline commented out for some?
> > I have a follow-up change with a lot of cleanups in gc coding.
> > In that change, I will fix the remaining inlines.  I wanted to
> > make an extra change for gc to simplify reviewing.
> 
> Ah, thank you for explaining this mystery.  I couldn't think of why this
> was.
> >
> >> + inline bool is_instance() const;
> >> + /*inline*/ bool is_array() const;
> >>
> >>
> >> http://cr.openjdk.java.net/~goetz/wr16/8146401-oopInline-
> >> rt/webrev.01/src/share/vm/oops/oop.inline.hpp.udiff.html
> >>
> >> I'm not really happy with the re-sorting because it may make backporting
> >> difficult, but it's not that many that were moved and the order of these
> >> functions has always seeemed random to me.  So I guess it's ok.
> >>
> >> Can you line up the '\' in the OOP_ITERATE macros at the end?
> >>
> >> http://cr.openjdk.java.net/~goetz/wr16/8146401-oopInline-
> >> rt/webrev.01/src/share/vm/oops/typeArrayOop.cpp.udiff.html
> > I think that's an artefact of udiff.  The patch and my files look fine.
> >
> >> This non-inline function could affect performance.   It may be called
> >> from GC.  We might need a typeArrayOop.inline.hpp for it.
> > I'll make a new file to be on the safe side.
> >
> > Why is there the empty typeArrayOop.cpp file?
> >
> 
> Oh, heaven knows why there was an empty file!   Maybe an artifact of
> includeDB_junk.    Thanks for making this change.
> 
> >> I can sponsor this, pending further review.
> > Thanks!
> 
> You have replied to Dan and David.  I agree with you that this is a
> worthwhile change.  I think there are some platforms (windows maybe?)
> that will just call 0 and crash if the definition of a function is
> missing, which you can do with a missing .inline.hpp include.   More
> compile time diagnostics makes the code less difficult to work on and
> more robust.
> 
> Thanks,
> Coleen
> 
> >
> > Best regards,
> >     Goetz.
> >
> >
> >
> >> Coleen
> >>
> >>
> >> On 1/4/16 10:44 AM, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> Several recent changes cleaned up includes of oop.inline.hpp in real .hpp
> >> header file.
> >>> Unfortunately, the 'inline' qualifier is added to the function
> >> implementations
> >>> in oop.inline.hpp instead of to the declarations in oop.hpp. Therefore,
> the
> >>> compiler can not detect failing inlines properly.
> >>>
> >>> This change moves the inline directive from oop.inline.hpp to oop.hpp.
> >> Also
> >>> it sorts the methods in oop.inline.hpp as they are sorted in oop.hpp.
> >>>
> >>> Further, it moves a row of calls from hpp files to inline.hpp or .cpp files.
> >>>
> >>> I put oop.inline.hpp into serviceUtil.hpp.  This is not clean, but this is a
> >>> very small .hpp file and no .cpp file exists.  So I think this is acceptable.
> >>>
> >>> Also, I put oop.inline.hpp into jvmciJavaClasses.hpp.  I don't want to do
> >>> bigger changes to this file in the rt repo, because jvmci is subject to
> >>> freqent changes recently.
> >>>
> >>> The following methods were moved to .cpp files:
> >>>
> >>> ProtectionDomainCacheTable::compute_hash()
> >>> ProtectionDomainCacheTable::index_for()
> >>> typeArrayOopDesc::object_size()
> >>> This is called only once outside .cpp file:
> >>> CallSiteDepChange::CallSiteDepChange()
> >>> This is only called in .cpp file
> >>> ConstantPool::string_at_put()
> >>>
> >>> If someone considers not inlining these harmful to performance,
> >>> I will add a new .inline.hpp file for them.
> >>>
> >>> Please review this change.  I please need a sponsor.
> >>> There are no functional edits, so it should be simple to review.
> >>> http://cr.openjdk.java.net/~goetz/wr16/8146401-oopInline-
> rt/webrev.01/
> >>>
> >>> Best regards,
> >>>     Goetz.