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

[Daniel D. Daugherty]

Thumbs up! My comments are all editorial so feel free to accept
them or ignore them. Your call.

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.

    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.

    line 403: spacing: 'len-1' -> 'len - 1'

    line 419: "leaving the state bytecodes"
         State bytecodes? As opposed to Federal bytecodes? :-)
         Maybe drop "state".

    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
>