RFR (XS): Move implementation of process_grey_object to concurrentMark.inline.hpp

Kim Barrett kim.barrett at oracle.com
Wed Aug 12 19:57:48 UTC 2015


On Aug 12, 2015, at 2:26 PM, Volker Simonis <volker.simonis at gmail.com> wrote:
> 
> Hi Kim,
> 
> thanks for looking into this issue.
> 
> We've mulled over the "slightly odd" coding for a while as well and
> our explanation was that you may have got unsatisfied references to
> the instantiations of process_grey_object() as well because they are
> only defined in the .cpp file and that may have been the reason why
> you have added the implicit instantiations.
> 
> Regards,
> Volker

That's part of it.  I wrote a much longer reply, but decided it was
tl;dr.  The short version is that I think the proposed change is an
improvement.  That it dodges what appears to be a compiler bug is a
happy bonus.  (If Axel has time, I'm mildly curious whether just
removing the inline qualifier also works around the problem.)

So I think the change looks good.

I have one optional change to suggest, which is to move the inline
definition of scan_object from the .hpp file to the .inline.hpp file.
I think putting it in the .hpp file was a mistake on my part.  Axel,
let me know whether you want to make that additional change.


>>> On Thu, Aug 6, 2015 at 3:49 PM, Siebenborn, Axel
>>> <axel.siebenborn at sap.com> wrote:
>>>> Hi,
>>>> Could I have reviews and a sponsor for this small change
>>>> 
>>>> We had a linker problem with a C++ - compiler that inlined process_grey_object at all uses but removed the symbol and the actual implementation.
>>>> As there are explicit instantiations of this method in concurrentMark.cpp, this probably should not happen and can be considered of a bug of the C++ compiler. Though, it seems to be pointless to mark that method inline if there are explicit instantiations.
>>>> 
>>>> Considering performance, it would be better to move the implementation of process_grey_object to concurrentMark.inline.hpp in order to inline the method in other compilation units.
>>>> 
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8133121
>>>> 
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~asiebenborn/8133121/webrev/
>>>> 
>>>> 
>>>> Thanks,
>>>> Axel




More information about the hotspot-gc-dev mailing list