Request for review (URGENT!!) 7033141: assert(has_cp_cache(i)) failed: oob

[Karen Kinnear]

Looks good to me also. Good catch earlier Tom.

thanks,
Karen

On May 20, 2011, at 11:08 AM, Tom Rodriguez wrote:

> Looks good.
>
> tom
>
> On May 19, 2011, at 3:01 PM, Coleen Phillimore wrote:
>
>>
>> Yes, these should be reversed.  You are right and it makes a  
>> difference on x86.  Fixed and retested:
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/7033141_3/
>> bug link at http://bugs.sun.com/view_bug.do?bug_id=7033141_3
>>
>> I also fixed the null assert strings and comments that Dan and  
>> Karen pointed out.
>>
>> Thank you!
>> Coleen
>>
>> On 5/19/2011 1:15 PM, Tom Rodriguez wrote:
>>> Shouldn't the reversed version of the code swap the get_native/ 
>>> get_Java calls?
>>>
>>> 144 // Rewrite a classfile-order CP index into a native-order CPC  
>>> index.
>>> 145 void Rewriter::rewrite_member_reference(address bcp, int  
>>> offset, bool reverse) {
>>> 146   address p = bcp + offset;
>>> 147   if (!reverse) {
>>> 148     int  cp_index    = Bytes::get_Java_u2(p);
>>> 149     int  cache_index = cp_entry_to_cp_cache(cp_index);
>>> 150     Bytes::put_native_u2(p, cache_index);
>>> 151   } else {
>>> 152     int cache_index = Bytes::get_Java_u2(p);
>>> 153     int pool_index = cp_cache_entry_pool_index(cache_index);
>>> 154     Bytes::put_native_u2(p, pool_index);
>>> 155   }
>>> 156 }
>>>
>>>
>>> tom
>>>
>>> On May 18, 2011, at 1:14 PM, Coleen Phillimore wrote:
>>>
>>>> The last code change I sent out was not complete, and would not  
>>>> work if there was an exception in the relocator.  The code was  
>>>> structured as:
>>>>
>>>>  if klass->is_rewritten() flag {
>>>>      verify - exit if exception
>>>>      rewrite bytecodes to point to (to be created) cpCache rather  
>>>> than constant pool
>>>>      create constant pool cache  - (added restore_bytecodes and)  
>>>> exit if exception
>>>>      run relocator for jsr/ret    - exit if exception
>>>>      link method entry points  - exit if exception
>>>>      set rewritten flag
>>>> }
>>>>
>>>> Any exceptions will cause the whole block to be retried if the  
>>>> exception is handled and class loading is attempted again for  
>>>> this class.  This latest code effectively changes this ordering to:
>>>>
>>>>  if klass->is_rewritten() flag {
>>>>    verify - exit if exception
>>>>    rewrite bytecodes to point to cpCache rather than constant  
>>>> pool entries
>>>>     create constant pool  {
>>>>         if exception restore bytecodes to point to constant pool
>>>>         exit }
>>>>     set rewritten flag
>>>> }
>>>> run relocator for jsr/ret - exit if exception
>>>> link method entry points - exit if exception
>>>>
>>>> If there's an exception in relocator and linking method stages,  
>>>> these two actions are restartable if the VM handles the exception  
>>>> and has to retry loading that class.   The actions under the  
>>>> rewritten flag are not restartable.
>>>>
>>>> This is tested with invokedynamic tests, RedefineClasses tests  
>>>> and class data sharing.  There was special testing to throw OOM  
>>>> at each part and testing to rerun the relocator.
>>>>
>>>> This didn't make the deadline, but please review (again) for the  
>>>> next stage of the jdk7 release.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/7033141_2/
>>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=7033141_2
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>
>>>> On 5/17/2011 4:44 PM, Coleen Phillimore wrote:
>>>>> Summary: Unrewrite bytecodes for OOM error allocating the  
>>>>> constant pool cache.
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/7033141/
>>>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=7033141
>>>>>
>>>>> This bug fix also moves linking methods outside of the rewriting  
>>>>> code so that when the code cache is full, the vm can attempt to  
>>>>> link the methods again.   (also fixes bug 6947901)
>>>>>
>>>>> Tested with oom errors inserted for random cpCache allocation  
>>>>> and also tested with invokedynamic test.
>>>>>
>>>>> This is somewhat urgent.  If acceptable, I would like to put it  
>>>>> back today for code freeze (if not, it'll wait).
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>
>