RFR (XS): Move implementation of process_grey_object to concurrentMark.inline.hpp
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.
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:
>>>> 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.
More information about the hotspot-gc-dev