RFR: 8075957: Reduce calls to the GC specific object visitors in oopDesc
Stefan Karlsson
stefan.karlsson at oracle.com
Thu Mar 26 10:16:34 UTC 2015
On 2015-03-26 11:05, Per Liden wrote:
> 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)
Yes, I missed to bring that over from the original patch.
>
> Other than that the change looks good!
Thanks, Per!
StefanK
>
> /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
>>
More information about the hotspot-gc-dev
mailing list