RFR(L): 8146395: Add inline qualifier in oop.hpp and fix inlining in gc files
Stefan Karlsson
stefan.karlsson at oracle.com
Tue Jan 19 15:56:11 UTC 2016
Hi Goetz,
Would you mind sending an incremental webrev of the difference between
webrev.01 and webrev.02?
Thanks,
StefanK
On 2016-01-19 16:36, Lindenmaier, Goetz wrote:
> Hi,
>
> I incorporated your proposals into the change. I also added one more #include,
> as precompiled headers showed a build issue.
> Further I fixed inlines in g1OopClosures.hpp. Because of the very same issue
> our VS2010 build failed (see below). I think this fits well into this change.
> I rebased the webrev to todays rt repository and resolved issues with Davids
> Change.
>
> New webrev:
> http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.02/
>
> This is now tested with all our tests, including hs-jtreg, linux/nt/bsd x86_64,
> solarissparc and our 3 ppc platforms.
>
> Best regards,
> Goetz
>
> VS2010 error:
> g1RemSet.obj : error LNK2019: unresolved external symbol "public: void __cdecl G1TriggerClosure::do_oop_work<class oopDesc *>(class oopDesc * *)" (??$do_oop_work at PEAVoopDesc@@@G1TriggerClosure@@QEAAXPEAPEAVoopDesc@@@Z) referenced in function "public: virtual void __cdecl G1TriggerClosure::do_oop(class oopDesc * *)" (?do_oop at G1TriggerClosure@@UEAAXPEAPEAVoopDesc@@@Z)
> g1RemSet.obj : error LNK2019: unresolved external symbol "public: void __cdecl G1TriggerClosure::do_oop_work<unsigned int>(unsigned int *)" (??$do_oop_work at I@G1TriggerClosure@@QEAAXPEAI at Z) referenced in function "public: virtual void __cdecl G1TriggerClosure::do_oop(unsigned int *)" (?do_oop at G1TriggerClosure@@UEAAXPEAI at Z)
> g1RemSet.obj : error LNK2019: unresolved external symbol "public: void __cdecl G1InvokeIfNotTriggeredClosure::do_oop_work<class oopDesc *>(class oopDesc * *)" (??$do_oop_work at PEAVoopDesc@@@G1InvokeIfNotTriggeredClosure@@QEAAXPEAPEAVoopDesc@@@Z) referenced in function "public: virtual void __cdecl G1InvokeIfNotTriggeredClosure::do_oop(class oopDesc * *)" (?do_oop at G1InvokeIfNotTriggeredClosure@@UEAAXPEAPEAVoopDesc@@@Z)
> g1RemSet.obj : error LNK2019: unresolved external symbol "public: void __cdecl G1InvokeIfNotTriggeredClosure::do_oop_work<unsigned int>(unsigned int *)" (??$do_oop_work at I@G1InvokeIfNotTriggeredClosure@@QEAAXPEAI at Z) referenced in function "public: virtual void __cdecl G1InvokeIfNotTriggeredClosure::do_oop(unsigned int *)" (?do_oop at G1InvokeIfNotTriggeredClosure@@UEAAXPEAI at Z)
> g1RemSet.obj : error LNK2019: unresolved external symbol "public: void __cdecl G1Mux2Closure::do_oop_work<class oopDesc *>(class oopDesc * *)" (??$do_oop_work at PEAVoopDesc@@@G1Mux2Closure@@QEAAXPEAPEAVoopDesc@@@Z) referenced in function "public: virtual void __cdecl G1Mux2Closure::do_oop(class oopDesc * *)" (?do_oop at G1Mux2Closure@@UEAAXPEAPEAVoopDesc@@@Z)
> g1RemSet.obj : error LNK2019: unresolved external symbol "public: void __cdecl G1Mux2Closure::do_oop_work<unsigned int>(unsigned int *)" (??$do_oop_work at I@G1Mux2Closure@@QEAAXPEAI at Z) referenced in function "public: virtual void __cdecl G1Mux2Closure::do_oop(unsigned int *)" (?do_oop at G1Mux2Closure@@UEAAXPEAI at Z)
> g1RemSet.obj : error LNK2019: unresolved external symbol "public: void __cdecl G1UpdateRSOrPushRefOopClosure::do_oop_work<unsigned int>(unsigned int *)" (??$do_oop_work at I@G1UpdateRSOrPushRefOopClosure@@QEAAXPEAI at Z) referenced in function "public: virtual void __cdecl G1UpdateRSOrPushRefOopClosure::do_oop(unsigned int *)" (?do_oop at G1UpdateRSOrPushRefOopClosure@@UEAAXPEAI at Z)
> g1RemSet.obj : error LNK2019: unresolved external symbol "public: void __cdecl G1UpdateRSOrPushRefOopClosure::do_oop_work<class oopDesc *>(class oopDesc * *)" (??$do_oop_work at PEAVoopDesc@@@G1UpdateRSOrPushRefOopClosure@@QEAAXPEAPEAVoopDesc@@@Z) referenced in function "public: virtual void __cdecl G1UpdateRSOrPushRefOopClosure::do_oop(class oopDesc * *)" (?do_oop at G1UpdateRSOrPushRefOopClosure@@UEAAXPEAPEAVoopDesc@@@Z)
> jvm.dll : fatal error LNK1120: 8 unresolved externals
>
>
>
>> -----Original Message-----
>> From: Stefan Karlsson [mailto:stefan.karlsson at oracle.com]
>> Sent: Donnerstag, 14. Januar 2016 10:12
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-gc-
>> dev at openjdk.java.net
>> Subject: Re: RFR(L): 8146395: Add inline qualifier in oop.hpp and fix inlining in
>> gc files
>>
>> 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> 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.htm
>> l
>>
>> 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.ht
>> ml
>>
>> // 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.)
>>
More information about the hotspot-gc-dev
mailing list