Please review: 6805748: Assertion "don't reset to 0 -- could be mistaken for never-executed" in CompilationPolicy

[Tom Rodriguez]

Looks good.

tom

On Mar 19, 2009, at 4:30 PM, Coleen Phillimore - Sun Microsystems wrote:

> Summary: Resetting the invocation counter for a method invocation  
> event was setting count to zero for CompileThreshold=1, making it  
> look like a never executed method.
>
>
> With CompileThreshold=1, the method's invocation counter is reset to  
> CompileThreshold/2 = 0.  The carry bit is also set and is supposed  
> to indicate that it has been executed, even when the count goes to  
> zero.
> But there's a race where the 'carry' bit of the invocation counter  
> is reset to zero and then set again at the point of the assert.   
> I've looked through the C++ compiler generated code for the race  
> that is causing this and found some in the interpreters when  
> incrementing the invocation count, but this test still doesn't pass  
> with the fix to this interpreter race.  I can't find what's  
> resetting the carry bit!
> In the InvocationCounter code there are conditionals that protect  
> the count from going to zero in other cases, and adding the same to  
> set_carry() fixes the bug.  It's also less code disturbed than  
> adding volatile to all the invocationCounter methods for this  
> special case of CompilationThreshold=1.
>
> Webrev:
> http://cr.openjdk.java.net/~coleenp/6805748/src/share/vm/interpreter/invocationCounter.cpp.cdiff.html
>
> Thanks,
> Coleen
>