RFR(L): 8146395: Add inline qualifier in oop.hpp and fix inlining in gc files

[Stefan Karlsson]

Hi Goetz,

Thanks for cleaning this up!

You'll have some merge conflicts with:
8146690: Make all classes in GC follow the naming convention

So we might want to coordinate that somehow.

Inlined:

On 2016-01-13 23:54, Lindenmaier, Goetz wrote:
>
> Hi
>
> This change finishes the cleanup started in "8146401: Clean up 
> oop.hpp: add inline directives and fix header files"
>
> by fixing the issues in gc files.  See also 
> https://bugs.openjdk.java.net/browse/JDK-8146395 
> <https://bugs.openjdk.java.net/browse/JDK-8146395> and the discussion
>
> of this change in the rt mailing list: 
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-January/017440.html
>
> Most issues can be fixed by including the proper .inline.hpp header or 
> moving a function there.
>
> I added new files src/share/vm/gc/shared/referenceProcessor.inline.hpp 
> and g1SATBCardTableModRefBS.inline.hpp.
>
> I moved the following methods to their .cpp files. In case someone 
> considers these
>
> performance relevant, I will add an inline.hpp file for them:
>
>   PromotedObject::next and ::setNext --> promotionInfo.cpp
>
>   ObjectStartArray::object_start --> objectStartArray.cpp
>
>   FillClosure::do_addr to --> psParallelCompact.cpp (ok, only used here)
>
>   ageTable::add --> ageTable.cpp
>
>   KlassSizeStats::count_array / ::count --> heapInspection.cpp
>
> Please review this change.  I please need a sponsor.
>
> http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/ 
> <http://cr.openjdk.java.net/%7Egoetz/wr16/8146395-oopInline-gc/webrev.01/>
>


http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/cms/cmsOopClosures.inline.hpp.frames.html

Why did you move trim_queues?

I'm personally not particularly fond of these macro constructs, since 
the makes it harder to navigate the code in IDEs.
   88 DO_OOP_WORK_NV_IMPL(Par_PushOrMarkClosure)


http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/g1/g1SATBCardTableModRefBS.hpp.udiff.html

    // These are the more general virtual versions.
-  virtual void write_ref_field_pre_work(oop* field, oop new_val) {
-    inline_write_ref_field_pre(field, new_val);
-  }
-  virtual void write_ref_field_pre_work(narrowOop* field, oop new_val) {
-    inline_write_ref_field_pre(field, new_val);
-  }
+  inline virtual void write_ref_field_pre_work(oop* field, oop new_val);
+  inline virtual void write_ref_field_pre_work(narrowOop* field, oop 
new_val);

I'm not sure how this is going to work. Have you verified that 
BarrierSet::write_ref_field_pre works when called while running G1?


http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/objectStartArray.cpp.udiff.html
http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/objectStartArray.hpp.udiff.html

I think object_start should be moved to an inline.hpp file, or you need 
to verify that this has no performance impact.

---

-#undef assert_covered_region_contains

^ This seems like an unrelated change.


http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/psScavenge.hpp.udiff.html

There's no need for this function to be inlined.


http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/shared/ageTable.cpp.udiff.html

Move to inline.hpp file.


http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/oops/oop.hpp.udiff.html

Spurious new-line

+  inline uint age() const;
    inline void incr_age();

+
    // mark-sweep support
    void follow_body(int begin, int end);

Thanks,
StefanK

> Best regards,
>
>   Goetz Lindenmaier
>
> (The webrev messed up some change comments, the diffs and patch are 
> fine, though.)
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160114/1f6353ca/attachment.htm>