RFR(S) : 8011971 : WB API doesn't accept j.l.reflect.Constructor

[Vladimir Kozlov]

On 4/14/13 10:04 AM, Igor Ignatyev wrote:
>> Please, do. Especially for methods and tests classes.
> i'm really not very good in code's commentation, but i did my best.

I would prefer more detailed comments but current is good enough. Thank you.

>
> in addition to changes according to your suggestions, i added ability to
> specify comp_level in WB::makeMethodNotCompilable() and found some
> strange behavior of 'CompilationPolicy::can_be_compiled':
>
> 124   if (comp_level == CompLevel_all) {
> 125     return !m->is_not_compilable(CompLevel_simple) &&
> !m->is_not_compilable(CompLevel_full_optimization);
> 126   }
>
> if method is 'not_compilable' in C1 or C2, method 'can_be_compiled' will
> return false for 'CompLevel_all' even it is compilable in another level.
> is it expected behavior or it is a bug?

It is bug but only in Tiered where both C1 and C2 are present. Most 
important, I think, we should always pass comp_level to this method 
(don't use default CompLevel_all). I looked and all call site have 
comp_level information.

>
> updated webrev: http://cr.openjdk.java.net/~iignatyev/8011971/webrev.01/

Looks good.

Thanks,
Vladimir

>
> Best regards,
> Igor Ignatyev
>
> On 04/13/2013 12:59 AM, Vladimir Kozlov wrote:
>> On 4/12/13 1:14 PM, Igor Ignatyev wrote:
>>>  > Could you explain why you need j.l.reflect.Constructor in WB compiler
>>>  > tests? As java coding exercise it may be good (I am lost in it).
>>> But how
>>>  > it helps to JIT compiler testing?
>>
>> Here is a good comment ;) :
>>
>>> we have plan to create replacement of current CTW. main goal is to make
>>> available using CTW in product build. but it must be equals to current
>>> implementation in terms of functionality. since CTW compile constructor,
>>> we also must to compile constructor. so we need api for that.
>>>
>>>  > Do you know about 'comments' in a code? :)
>>> yes. i know, but i think that all is clear and self-documented :)
>>> but if you insist, i can add javadoc to all public and protected
>>> methods/fields.
>>
>> Please, do. Especially for methods and tests classes.
>>
>>>
>>>  > Why do you need to set dontinline in costructor?
>>>  > Would be better explicitly call new method from CompilerWhiteBoxTest
>>>  > class when needed:
>>>  >
>>>  > setDontInlineMethod() {
>>>  >    WHITE_BOX.testSetDontInlineMethod(method, true);
>>>  > }
>>> i need to set dontinline in constructor to prevent inlining of 'method'
>>> into others methods (fix for 8008211).
>>
>> I meant, why you need to do it in 'super' constructor?
>> You can call setDontInlineMethod() in local constructor. Test methods
>> will be executed only after constructor executed.
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> Best regards,
>>> Igor Ignatyev
>>>
>>> On 04/12/2013 11:39 PM, Vladimir Kozlov wrote:
>>>> Igor,
>>>>
>>>> Could you explain why you need j.l.reflect.Constructor in WB compiler
>>>> tests? As java coding exercise it may be good (I am lost in it). But
>>>> how
>>>> it helps to JIT compiler testing?
>>>>
>>>> Do you know about 'comments' in a code? :)
>>>>
>>>> Using argument 'true' say nothing what it does since the corresponding
>>>> code is in different class file:
>>>>
>>>> +        super(testCase, true);
>>>>
>>>> Why do you need to set dontinline in costructor?
>>>> Would be better explicitly call new method from CompilerWhiteBoxTest
>>>> class when needed:
>>>>
>>>> setDontInlineMethod() {
>>>>    WHITE_BOX.testSetDontInlineMethod(method, true);
>>>> }
>>>>
>>>> Regards,
>>>> Vladimir
>>>>
>>>> On 4/12/13 8:29 AM, Igor Ignatyev wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review patch.
>>>>>
>>>>> 1. changed signature of all methods where j.l.reflect.Method is used
>>>>> 2. added tests which use j.l.reflect.Constructor
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8011971/webrev.00/
>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-8011971