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

[Mikael Gerdin]

Hi Stefan,

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

   55 inline void ParCompactionManager::follow_contents(objArrayOop obj, 
int index) {
   56   ObjArrayKlass* k = (ObjArrayKlass*)obj->klass();
   57   k->oop_follow_contents(obj, index);
   58 }

That should be k->oop_follow_contents(this, obj, index)
Otherwise you end up in the MarkSweep code instead of ParCompact.

The rest of the change looks good.

Again, excellent cleanups!

/Mikael


>
> - 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
>
> 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
>