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

[Coleen Phillimore]

On 5/19/2011 11:00 AM, Daniel D. Daugherty wrote:
> Thumbs up! My comments are all editorial so feel free to accept
> them or ignore them. Your call.
>
Dan - thank you for the code review.

> Dan
>
>
> src/share/vm/interpreter/rewriter.cpp
>    line 70: spacing: 'len-1' -> 'len - 1'
>    And thanks for switching to the more traditional count-down style.
No problem, I prefer the traditional style but I don't like too many 
spaces so I left it in my favored compressed style :)
>
>    line 219, 223 - no text in the assert. Yes, I see that line 202 and
>    line 206 do that also, but I think "sanity check" would be better
>    than nothing.
I fixed all the empty asserts in the file to be something meaningful.  
thanks for the motivation.
>
>    line 403: spacing: 'len-1' -> 'len - 1'
>
>    line 419: "leaving the state bytecodes"
>         State bytecodes? As opposed to Federal bytecodes? :-)
>         Maybe drop "state".
oops, I was in the middle of typing something else and changed it, 
leaving the state behind!

Thanks,
Coleen
>
>    line 432: spacing: 'len-1' -> 'len - 1'
>
> src/share/vm/interpreter/rewriter.hpp
>    No comments.
>
> src/share/vm/oops/instanceKlass.cpp
>    No comments.
>
> src/share/vm/oops/instanceKlass.hpp
>    No comments.
>
> src/share/vm/oops/methodOop.cpp
>    No comments.
>
> src/share/vm/prims/jvmtiRedefineClasses.cpp
>    No comments.
>
> src/share/vm/prims/methodHandleWalk.cpp
>    No comments.
>
>
>
> On 5/18/2011 2: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
>>