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

[Vladimir Kozlov]

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.

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); }

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
>>>>>>
>>>>
>>
> 
>