RFR(L): 8146395: Add inline qualifier in oop.hpp and fix inlining in gc files

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Tue Jan 19 15:36:56 UTC 2016


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