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

[Volker Simonis]

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


On Wed, Aug 12, 2015 at 7:45 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
> On Aug 11, 2015, at 8:21 AM, Volker Simonis <volker.simonis at gmail.com> wrote:
>>
>> Hi Axel,
>>
>> the change looks good!
>> I think we should always define template functions in .inline.hpp files.
>>
>> Can somebody please sponsor this change?
>>
>> Thank you and best regards,
>> Volker
>>
>>
>> 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
>
> I can sponsor this, once I’ve reviewed it.  I authored the relevant code, and want to look it and the change over and try to recall why it was written the way it was.  The existing code is slightly odd, and I wouldn’t have written it that way without some reason…
>