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 16:23:08 UTC 2016
Hi Stefan,
I had to redo the old change, thus you see parts of the changes caused
by Davids change. And I fixed the Copyrights ... you will see :(
http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.02-incr/
Best regards,
Goetz.
> -----Original Message-----
> From: Stefan Karlsson [mailto:stefan.karlsson at oracle.com]
> Sent: Dienstag, 19. Januar 2016 16:56
> 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,
>
> 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@@QEAAXPEAPE
> AVoopDesc@@@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@@QEAAXPEAPEA
> VoopDesc@@@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@
> 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