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

[Igor Ignatyev]

Vladimir,

thank you for review.

> 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.
ok, i will file a bug for it.

Best regards,
Igor Ignatyev

On 04/15/2013 11:38 PM, Vladimir Kozlov wrote:
> 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