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

[Lindenmaier, Goetz]

Hi David, 

the documentation you point to makes the point that whether to inline
is an implementation detail, and that that should not be annotated to 
the declaration.  Basically, this is a good point.

But the documentation does not cover the fact that 
we have the implementation in a different file than the declaration.
The .inline.hpp is not included always (as you pointed out). 

Therefore, if we compile with precompiled headers, we get more
inlines than if we do so without.
Further, many functions were moved to oop.linline.hpp during the
recent cleanups, and I think all the #includes I had to add were not
left out deliberately, but just happened because the compiler did 
not complain.
I think if somebody decides not to place a function implementation 
in a .cpp file, it should have the chance to be inlined at all it's calls.
Putting the 'inline' into the .hpp file assures this.

Best regards,
  Goetz.




> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Mittwoch, 6. Januar 2016 05:59
> 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
> 
> Hi Goetz,
> 
> On 5/01/2016 1: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
> 
> This seems contrary to the C++ FAQ:
> 
> https://isocpp.org/wiki/faq/inline-functions
> 
> The declaration in the .hpp should not have inline, only the definition,
> which in our case is in the .inline.hpp file.
> 
> Any code that includes the .inline.hpp will have seen the definition of
> the inline function prior to its use - as long as the includes are correct.
> 
> David
> -----
> 
> 
> > 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.
> >