Request for review (XS): 7091366 re-enable quicksort tests
Vladimir Kozlov
vladimir.kozlov at oracle.com
Mon Sep 26 21:27:08 UTC 2011
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