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

[Lindenmaier, Goetz]

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.
 
> + 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?
 
> I can sponsor this, pending further review.
Thanks!

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.