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

[Stefan Karlsson]

On 2015-03-26 10:34, Mikael Gerdin wrote:
> 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.

Yes, you're right. This was introduced when I extracted this patch out 
from the JDK-8075955 patch.

This fails immediately when I start SPECjbb2005.

>
> The rest of the change looks good.
>
> Again, excellent cleanups!

Thanks, Mikael!

StefanK

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