Request for review (XS): 7091366 re-enable quicksort tests

Bengt Rutisson bengt.rutisson at oracle.com
Tue Sep 27 04:49:47 PDT 2011


Thanks  Vladimir! All set to push this now.

Bengt


On 2011-09-26 23:27, Vladimir Kozlov wrote:
> I am fine with your current code since it is compiled with all (old 
> and new) versions of compilers. It was my main concern.
>
> Thanks,
> Vladimir
>
> Bengt Rutisson wrote:
>>
>> Vladimir,
>>
>> Thanks for the comments!
>>
>> You are right. If I remove the parentheses I have to also remove the 
>> "static" keyword. On the other hand, if I keep the parentheses I can 
>> also keep the "static" keyword. This compiles with all the compilers 
>> for both jdk6 and jdk7.
>>
>> I first made the change the way you suggested, but then I saw that 
>> the old code (that I had removed with my change) was using the style 
>> with parentheses and "static" keyword.
>> http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/diff/04760e41b01e/src/share/vm/oops/methodOop.cpp 
>>
>>
>> I actually don't know exactly what the difference is or which one is 
>> more correct. But I went for the one with the parentheses and static 
>> since that seems to have worked before.
>>
>> I am happy to change it either way. Any thoughts on which one that is 
>> more correct?
>>
>> Thanks,
>> Bengt
>>
>> On 2011-09-26 18:35, Vladimir Kozlov wrote:
>>> Bengt,
>>>
>>> You need to remove "static" keyword, otherwise new compilers will 
>>> complain about combination "extern static" and you don't need 
>>> parentheses since it is only one method:
>>>
>>> -static int test_stdlib_comparator(const void* a, const void* b) {
>>> +extern "C" int test_stdlib_comparator(const void* a, const void* b) {
>>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 9/26/11 6:06 AM, Bengt Rutisson wrote:
>>>>
>>>> Hi all,
>>>>
>>>> This is a small hs23 fix. I hope that we have most of the hs22 work 
>>>> behind us, so I thought I'd send this out for review.
>>>>
>>>> http://cr.openjdk.java.net/~brutisso/7091366/webrev/
>>>>
>>>> Background:
>>>> One of the tests in the quicksort implementation that I did before 
>>>> the summer uses the stdlib::qsort() to verify that my
>>>> implementation sorts the same way that the stdlib does. The tests 
>>>> needs to pass a function pointer to stdlib::qsort()
>>>> and it seems that the older Solaris compilers that JDK6 uses 
>>>> requires the function to be declared extern "C" for this to
>>>> work.
>>>>
>>>> John Coomes recently had to make a quick fix to get this to build 
>>>> with JDK6 at all. He disabled the tests all together.
>>>> With my fix now I am re-enabling the tests and declaring the call 
>>>> back function extern "C".
>>>>
>>>> I have tested this with jprt and "-relase jdk6". Seems to build 
>>>> fine now.
>>>> http://prt-web.us.oracle.com//archive/2011/09/2011-09-26-082420.brutisso.hs-gc-qsfix//JobStatus.txt 
>>>>
>>>>
>>>> With the recent discussions about the Hotspot express model, it 
>>>> seems like we might not be supplying this code into JDK6
>>>> after all, but I would still like to get this fixed.
>>>>
>>>> CR:
>>>> 7091366 re-enable quicksort tests
>>>> http://monaco.us.oracle.com/detail.jsf?cr=7091366
>>>>
>>>> Jesper already looked at the changes, but I need at least one more 
>>>> review.
>>>>
>>>> Thanks,
>>>> Bengt
>>



More information about the hotspot-gc-dev mailing list