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

[Siebenborn, Axel]

Hi Kim,

On 12.08.2015, at 21:57, Kim Barrett wrote:
> 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.)
We were curious too and  indeed, removing the inline qualifier would 
work 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.
Yes, there is the new webrev:

http://cr.openjdk.java.net/~asiebenborn/8133121_0/webrev/

>
>
>>>> 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
Thanks for reviewing and sponsoring,
Axel