RFR: 8075957: Reduce calls to the GC specific object visitors in oopDesc

[Per Liden]

Hi,

Inline

On 2015-03-25 16:02, Stefan Karlsson wrote:
> Hi,
>
> Please review this refactoring patch:
>   http://cr.openjdk.java.net/~stefank/8075957/webrev.01/
>   https://bugs.openjdk.java.net/browse/JDK-8075957
>
> Some of our GCs have GC specific entry points and code placed in the
> oopDesc and Klass files. They are used to implement marking, scavenging
> and adjusting of oops in the the objects. See the following functions:
>
> Mark Sweep:
>    void follow_contents(void);
>    int adjust_pointers();
>
> Parallel Scavenge
>    void push_contents(PSPromotionManager* pm);
>
> Parallel Old
>    void update_contents(ParCompactionManager* cm);
>    void follow_contents(ParCompactionManager* cm);
>
> With JDK-8075955 we'll start to use parts of the oop_oop_iterate
> framework to implement these functions, and as a preparation patch I'd
> like to minimize the number of places where these functions are directly
> called from Mark Sweep, Parallel Scavenge and Parallel Old.
>
> The proposal is to add thin wrapper functions to the three GCs, and let
> these wrappers be the only callers of the functions above.
>
>
> Detailed description of the patch:
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp.udiff.html
>
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/g1/g1ParScanThreadState.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/cardTableExtension.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/pcTasks.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psCompactionManager.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psCompactionManager.hpp.udiff.html
>
> - Wrapper declarations
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psCompactionManager.inline.hpp.udiff.html
>
> - Wrapper definitions
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.cpp.udiff.html
>
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
> - Call to GC wrapper function
> - Moved definition of do_addr since it was only used in the .cpp, placed
> in the .hpp file, and used other .inline.hpp files.
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.hpp.udiff.html
>
> - Moved definition of do_addr since it was only used in the .cpp, placed
> in the .hpp file, and used other .inline.hpp files.
> - Removed dead code (follow_root) that referred to one of the object
> visitors.
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psPromotionManager.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psPromotionManager.hpp.udiff.html
>
> - Wrapper declaration
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psPromotionManager.inline.hpp.udiff.html
>
> - Wrapper declarations
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/parallelScavenge/psTasks.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/shared/markSweep.cpp.udiff.html
>
> - Wrapper definition
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/shared/markSweep.hpp.udiff.html
>
> - Wrapper declarations
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/gc_implementation/shared/markSweep.inline.hpp.udiff.html
>
> - Wrapper definitions

   61 inline void MarkSweep::follow_object(oop obj) {
   62   obj->follow_contents();
   63 }
   64
   65 template <class T> inline void MarkSweep::follow_root(T* p) {
   66   assert(!Universe::heap()->is_in_reserved(p),
   67          "roots shouldn't be things within the heap");
   68   T heap_oop = oopDesc::load_heap_oop(p);
   69   if (!oopDesc::is_null(heap_oop)) {
   70     oop obj = oopDesc::decode_heap_oop_not_null(heap_oop);
   71     if (!obj->mark()->is_marked()) {
   72       mark_object(obj);
   73       obj->follow_contents();
   74     }
   75   }
   76   follow_stack();
   77 }

Line 73 should be a call to follow_object(obj)

Other than that the change looks good!

/Per

>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/memory/space.inline.hpp.udiff.html
>
> - Call to GC wrapper function
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/oops/instanceClassLoaderKlass.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/oops/instanceKlass.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/oops/instanceMirrorKlass.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/oops/instanceRefKlass.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> http://cr.openjdk.java.net/~stefank/8075957/webrev.01/src/share/vm/oops/objArrayKlass.cpp.udiff.html
>
> - Include of oop.pcgc.inline.hpp not needed
>
> No copyright years have been updated.
>
> Thanks,
> StefanK
>