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

[Vladimir Kozlov]

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