Request review: 4360113: Evict nmethods when code cache gets full

[Eric Caspole]

On Jan 6, 2010, at 3:42 PM, Vladimir Kozlov wrote:

> Eric,
>
> I like these changes. The only question left:
>
> Is it possible to avoid VM exit if (CompileTheWorld ||  
> ExitOnFullCodeCache)
> before code cache cleanup attempt? If it requires a lot of changes  
> leave as it is.
>

I have not considered working with these flags yet, I'll look at it  
and get back to you. If cleaning is on, would you still want  
ExitOnFullCodeCache to exit? I am not sure what this flag is used for.

> In compileBroker.cpp don't mix int with boolean (some compilers may  
> complain):
>
> + volatile jint CompileBroker::_should_compile_new_jobs = 1;
>
> +   static bool should_compile_new_jobs() { return UseCompiler &&  
> (_should_compile_new_jobs == 1); }

(_should_compile_new_jobs == 1) is a boolean, right? I really want  
_should_compile_new_jobs to be a boolean, but there are no atomic  
boolean setters.

>
> Thanks,
> Vladimir
>
> Eric Caspole wrote:
>> Hi Vladimir,
>> I updated my webrev to .02: I added a new field into CompileBroker  
>> to use for toggling the compiler on/off when the code cache gets  
>> full or when enough space is later cleared, to avoid changing  
>> UseCompiler on the fly. Also I consolidated all the places that  
>> had some "turn off compiler" code into one place in CompileBroker  
>> as we discussed before the holidays.
>> Regards,
>> Eric
>> On Dec 16, 2009, at 3:29 PM, Vladimir Kozlov wrote:
>>>
>>> Eric Caspole wrote:
>>>> I see what you mean. If there are other already disconnected  
>>>> methods (nm->method()->code() != nm) due to normal operation,  
>>>> can I safely assume they are already marked not entrant and they  
>>>> will go away by normal sweeping? Or should I hurry them along  
>>>> here? I  see how not
>>>
>>> The assert(nm->method()->code()->is_not_entrant() is enough, I  
>>> think.
>>>
>>>> having the extra check means I might reset the counters many  
>>>> times for no reason, I'll change that.
>>>
>>>>> It seems other thread set UseCompiler to false between the  
>>>>> check and assert.
>>>> So that means this could have happened in the existing system if  
>>>> the broker shut off the compiler due to fullness, racing with  
>>>> the java thread making this call. Yuck.
>>>
>>> Yes. With your changes it could happened more frequently.
>>> I think, it is related to what we talked about today on meeting:
>>>
>>> # Consolidate "shut off compiler" logic into one place, and  
>>> reconsider logic to turn compilers on and off.  Currently this  
>>> logic is spread across three different places even without the  
>>> unloading change.
>>>
>>> May be we should use separate flag SuspendCompilation when we think
>>> codecache could be freed and switch of UseCompiler when we give up.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>>
>>>>> Vladimir
>>>>>
>>>>>>> % java -XX:ReservedCodeCacheSize=6M -XX: 
>>>>>>> +PrintCodeCacheExtension -XX:+UseCodeCacheFlushing - 
>>>>>>> XX:MinCodeCacheFlushingInterval=300 -XX:+PrintCompilation -XX: 
>>>>>>> +PrintMethodFlushing -jar SPECjvm2008.jar -ikv  
>>>>>>> compiler.sunflow compiler.compiler
>>>>>>>
>>>>>>> #  Internal Error (/net/irkutsk/export/home/kvn/work2/hg/ 
>>>>>>> 4360113/src/share/vm/runtime/compilationPolicy.cpp:124),  
>>>>>>> pid=26531, tid=51
>>>>>>> #  Error: assert(UseCompiler || CompileTheWorld,"UseCompiler  
>>>>>>> should be set by now.")
>>>>>>>
>>>>>>> Current thread (0x08c84400):  JavaThread "BenchmarkThread  
>>>>>>> compiler.sunflow 6" [_thread_in_vm, id=51, stack 
>>>>>>> (0xfa2c9000,0xfa319000)]
>>>>>>>
>>>>>>> Stack: [0xfa2c9000,0xfa319000],  sp=0xfa317ad8,  free  
>>>>>>> space=13afa319000k
>>>>>>> Native frames: (J=compiled Java code, j=interpreted, Vv=VM  
>>>>>>> code, C=native code)
>>>>>>> V  [libjvm.so+0x170235a] void VMError::report(outputStream*)  
>>>>>>> + 0x5be
>>>>>>> V  [libjvm.so+0x1703336] void VMError::report_and_die() + 0x586
>>>>>>> V  [libjvm.so+0x7bf95d] void report_assertion_failure(const  
>>>>>>> char*,int,const char*) + 0x61
>>>>>>> V  [libjvm.so+0x6ccd4e] void  
>>>>>>> SimpleCompPolicy::method_invocation_event 
>>>>>>> (methodHandle,Thread*) + 0x20a
>>>>>>> V  [libjvm.so+0xa68949]  
>>>>>>> nmethod*InterpreterRuntime::frequency_counter_overflow_inner 
>>>>>>> (JavaThread*,unsigned char*) + 0x1551
>>>>>>>
>>>>>>>
>>>>>>> Eric Caspole wrote:
>>>>>>>> 4360113: Evict nmethods when code cache gets full
>>>>>>>> http://cr.openjdk.java.net/~ecaspole/4360113/
>>>>>>>> In this change, under a flag and off by default, when  
>>>>>>>> compilers notice the code cache is getting full, they will  
>>>>>>>> call a vm op that calls new code that attempts to  
>>>>>>>> speculatively unload the oldest half of the nmethods (based  
>>>>>>>> on the compile job id) by hiding the methodOop's ref to the  
>>>>>>>> nmethod in the new _saved_code field. Then execution  
>>>>>>>> resumes. After inline cache cleaning, callers will have to  
>>>>>>>> go back to the methodOop to get the verified entry point.
>>>>>>>> At that point, the methodOop's _code field is restored from  
>>>>>>>> the _saved_code field and the methodOop/nmethod go back to  
>>>>>>>> their normal state.
>>>>>>>> If a method so marked is not called by the second sweep  
>>>>>>>> cycle after the one where forced unloading happened, the  
>>>>>>>> nmethod will be marked non-entrant and got rid of by normal  
>>>>>>>> sweeping. That gives the app a few seconds to make progress  
>>>>>>>> and call its hottest methods.
>>>>>>>> We chose to target the oldest half of nmethods due to a  
>>>>>>>> customer experience with a long-running app server, and  
>>>>>>>> despite multiple redeployments of the same web app,  
>>>>>>>> something was preventing old instances of the web app from  
>>>>>>>> ever getting unloaded. In that case, they ran into the code  
>>>>>>>> cache full problem so the most recent redeployment was  
>>>>>>>> running interpreter only. We have also observed that for  
>>>>>>>> many applications a lot of methods get compiled and used  
>>>>>>>> during the startup phase that are never used again.
>>>>>>>> In this change there is also a timer based backoff, default  
>>>>>>>> of 30 seconds, so that if the normal state of the app is  
>>>>>>>> constantly triggering unloading, the unloading will stop and  
>>>>>>>> it will fall back to the existing situation of disabling the  
>>>>>>>> compiler.
>>>>>>>> In my testing, this allows the program to quickly resume  
>>>>>>>> normal operation with no noticeable performance degradation.
>>>>>>>> Thanks for your comments,
>>>>>>>> Eric
>>>>>>>
>>>>>
>>>
>