[9] RFR(S): JDK-8040798: compiler/startup/SmallCodeCacheStartup.java timed out in RT_Baseline

[Vladimir Kozlov]

On 4/23/14 1:22 AM, Albert wrote:
> Igor, Vladimir, thanks for looking at this.
>
> @Igor: You are right. If we do the notify in delete_all(), all threads
> that wait on the lock of a compilation task
> will wake up. I removed the timed wait in
> CompileBroker::wait_for_completion() since I think it is not needed
> anymore.
>
> @Vladimir: Please see comments inline:
>
> Here is the new webrev:
> http://cr.openjdk.java.net/~anoll/8040798/webrev.01/

Looks good.

>
>
> On 04/22/2014 07:36 PM, Vladimir Kozlov wrote:
>> Hi Albert,
>>
>> compileBroker.hpp I think you want:
>>
>>   NOT_PRODUCT( void print(); )
>>
> Done.
>> and you need to use #ifndef PRODUCT in compileBroker.cpp.
>>
> Done.
>> Next comment is not finished:
>> +     // Delete all queued compilation tasks to make
>>
> Done.
>>
>> We have 2 queues (c1 and C2). Does it means that C1 compilation may
>> block C2 compilation too?:
>>
> Yes, I think we have a single bit per Method*.
>> + /**
>> +  * See if this compilation is already requested.
>> +  *
>> +  * Implementation note: there is only a single "is in queue" bit
>> +  * for each method.  This means that the check below is overly
>> +  * conservative in the sense that an osr compilation in the queue
>> +  * will block a normal compilation from entering the queue (and vice
>> +  * versa).  This can be remedied by a full queue search to disambiguate
>> +  * cases.  If it is deemed profitable, this may be done.
>> +  */
>> + bool CompileBroker::compilation_is_in_queue(methodHandle method) {
>>
>>
>> Why next code in CompileBroker::compile_method() is not working?:
>>
> Do you mean why that code does not prevent the deadlock?
>
> I think that code does not cover the following scenario:
> 1) All compiler threads wait for new tasks

I thought not all compiler threads are initialized in this bug case. In 
which case this code should not except tasks. Right?

> 2) We put a blocking task in the compilation queue and wake up the
> compiler thread
> 3) The Java thread that issued the blocking compilation calls wait() on
> the compilation task
> 4) We run out of code cache and shut down compilation
> 5) All compiler threads exit the while loop in CompileQueue::get()
> 6) The Java thread blocks forever.

Yes, the code will not help in this case.

Thanks,
Vladimir

>
> Best,
> Albert
>> // If the compiler is shut off due to code cache getting full
>>     // fail out now so blocking compiles dont hang the java thread
>>     if (!should_compile_new_jobs()) {
>>       CompilationPolicy::policy()->delay_compilation(method());
>>       return NULL;
>>     }
>>     compile_method_base(method, osr_bci, comp_level, hot_method,
>> hot_count, comment, THREAD);
>>
>> Thanks,
>> Vladimir
>>
>> On 4/22/14 6:42 AM, Albert wrote:
>>> Hi,
>>>
>>> could I get reviews for this small change?
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8040798
>>>
>>> Problem:
>>> The test starts the JVM with 3m code cache and 64 compiler threads. As a
>>> result, it is very likely
>>> that not all compiler threads can be started due to insufficient space
>>> in the code cache. It is also
>>> possible that the compiler runtime initialization of C1 and/or C2 fails.
>>> There is a race between a
>>> the blocking compilations (the test is started with -Xcomp) and the
>>> process of shutting down the
>>> compilers. More specifically, it can happen that (1) a blocking
>>> compilation task is put into a compilation
>>> queue, (2) the compilers are shut down, and (3) the application thread
>>> waits forever until the compilation
>>> completes (The compilation will never complete, since all compiler
>>> threads are exited).
>>>
>>> Solution:
>>> Add a timeout to the wait() that waits for a compilation to complete and
>>> check if compilation is shut down
>>> forever. Furthermore, if we shut down the compiler runtimes, make sure
>>> that we use the interpreter to
>>> continue execution. The changes that fix the bug are in the following
>>> functions:
>>>
>>> CompileBroker::wait_for_completion(CompileTask* task)
>>> shutdown_compiler_runtime(AbstractCompiler* comp, CompilerThread*
>>> thread)
>>>
>>>
>>> The patch also removes the destruction of the compile queue objects in
>>> shutdown_compiler_runtime().
>>> The destruction is removed, since it does not reclaim significant
>>> portions of memory, but possible introduces
>>> problems, since pointers to the compile queues can be used at other
>>> locations in the code. An assert ensures
>>> that no more tasks are put into a compilation queue once the compilers
>>> have been shut down.
>>>
>>> In addition, the patch adds two locks in:
>>>
>>> void CompileBroker::mark_on_stack()
>>>
>>>
>>> The two locks ensure that compilation tasks are not removed from the
>>> compilation queue or freed by a
>>> different (compiler) thread while the method mark_on_stack is executed.
>>> I am not really sure if that lock
>>> is needed. However, the lock seems to prevent concurrent read/writes of
>>> the compilation queue.
>>>
>>> The rest of the changes are simple optional refactorings.
>>>
>>> Testing:
>>> The timeout does not occur with the proposed changes. JPRT.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~anoll/8040798/webrev.00/
>>>
>>> Many thanks in advance,
>>> Albert
>>>
>