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

[Karen Kinnear]

Coleen,

Code looks good. Thank you for the latest update and testing.

Question - why did you remove the comments in instanceKlass.cpp lines  
368-375
for rewrite_class? Are those three cases no longer true? I did think  
there were perhaps
more cases if you include redefineclasses and the new rewrite/un- 
rewrite/rewrite on retry :-)

rewriter.cpp: line 412 - I think the comment would be "if there are  
exceptions rewriting bytecodes
or allocating the cpcache, not "or relocating bytecodes"

thanks,
Karen

On May 18, 2011, at 4: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
>