review (XS) for 6956958: assert(is_clean() || is_call_to_compiled() || is_call_to_interpreted() || is_optimized() || is_megam

[Vladimir Kozlov]

Good.

Vladimir

Tom Rodriguez wrote:
> Oops, left that out when I peeled it out of another workspace.  It's there now.
> 
> tom
> 
> On Jun 18, 2010, at 1:45 PM, Vladimir Kozlov wrote:
> 
>> Static init still has type CodeBlob*:
>>
>>  29 CodeBlob* NMethodSweeper::_current = NULL;   // Current nmethod
>>
>> Otherwise looks good.
>>
>> Vladimir
>>
>> Tom Rodriguez wrote:
>>> http://cr.openjdk.java.net/~never/6956958
>>> 6956958: assert(is_clean() || is_call_to_compiled() || is_call_to_interpreted() || is_optimized() || is_megam
>>> Reviewed-by:
>>> When the sweeper scans the code cache it divides it up into
>>> NMethodSweepFraction chunks and processes one chunk per sweep.  Some
>>> changes late in the review of the concurrent sweeper changed the
>>> interation to skip by nmethods instead of CodeBlobs and during a
>>> change late in the review some code which was only supposed to make
>>> sure we had stopped on an nmethod was changed into skipping to the
>>> next nmethod.  This meant that sweeper skipped processing the last
>>> nmethod of each chunk.  Each nmethod is processed multiple times so
>>> most of the time this was fine but occasionally the stars aligned and
>>> the same nmethod was skipped every time.  This could leave it pointing
>>> at a dead nmethod which could cause failures of the sort we were
>>> seeing.  Logging code I added for a debug build showed precisely these
>>> events occurring when the crash happened.  The fix is simply to remove
>>> the extra call to next.
>>> While investigating this I found several other issues that are
>>> potential problems but they all appeared pretty benign.  I'm going to
>>> fix those issues another another bug since they are somewhat larger
>>> changes.
>>> src/share/vm/runtime/sweeper.hpp
>>> src/share/vm/runtime/sweeper.cpp
>