RFR(M): 8198756: Lazy allocation of compiler threads

Thu Mar 29 16:42:14 UTC 2018

Hi Martin,

Thank you for update.

Does using thread->smr_delete() solve runtime/whitebox/WBStackSize.java 
test problem I reported in RFE?

I see you kept _c1_compile_queue->size() / 2. I think it create too many 
C1 threads reaching max number _c1_count very fast.

I will start testing with 02 changes and let you know results.

Thanks,
Vladimir

On 3/28/18 9:02 AM, Doerr, Martin wrote:
> Hi Vladimir,
> 
> I have addressed your proposals with this new webrev:
> http://cr.openjdk.java.net/~mdoerr/8198756_CompilerCount/webrev.02/
> 
> Please take a look.
> 
> Thanks for your support. Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Montag, 26. März 2018 23:15
> To: Doerr, Martin <martin.doerr at sap.com>
> Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: RFR(M): 8198756: Lazy allocation of compiler threads
> 
> Hi Martin,
> 
> We can't delete _log when deleting CompilerThread. Log is referenced
> globally and used on VM exit to generate final log file when
> -XX:+LogCompilation is specified. compilercontrol tests passed after I
> change it:
> 
> +CompilerThread::~CompilerThread() {
> +  // Delete objects which were allocated on heap.
> +  delete _counters;
> +  // _log is referenced in global CompileLog::_first chain and used on
> exit.
> +}
> 
> I also see that we C1 compiler threads are removed too soon which cause
> their re-activation again. This may eat memory:
> 
> $ java -XX:+TraceCompilerThreads -XX:+LogCompilation t
> Added initial compiler thread C2 CompilerThread0
> Added initial compiler thread C1 CompilerThread0
> Warning: TraceDependencies results may be inflated by VerifyDependencies
> Added compiler thread C1 CompilerThread1 (available memory: 37040MB)
> Added compiler thread C1 CompilerThread2 (available memory: 37033MB)
> Added compiler thread C1 CompilerThread3 (available memory: 37032MB)
> Removing compiler thread C1 CompilerThread3
> Removing compiler thread C1 CompilerThread2
> Removing compiler thread C1 CompilerThread1
> Added compiler thread C1 CompilerThread1 (available memory: 37027MB)
> 
> May be we should take into account for how long these threads are not used.
> 
> Thanks,
> Vladimir
> 
> On 3/23/18 5:58 PM, Vladimir Kozlov wrote:
>> On 3/23/18 10:37 AM, Doerr, Martin wrote:
>>> Hi Vladimir,
>>>
>>> thanks for the quick reply. Just a few answers. I'll take a closer
>>> look next week.
>>>
>>>> You can't delete thread when it is NULL
>>> C++ supports calling delete NULL so I think it would be uncommon to
>>> check it. If there's a problem, I think the delete operator should get
>>> fixed.
>>>
>>> "If expression evaluates to a null pointer value, no destructors are
>>> called, and the deallocation function may or may not be called (it's
>>> implementation-defined), but the default deallocation functions are
>>> guaranteed to do nothing when handed a null pointer." [1]
>>
>> I am sure our code analyzing tool, which we use to check code
>> correctness, will compliant about it.
>>
>>>
>>>> We may need to free corresponding java thread object when we remove
>>>> compiler threads.
>>> I think it would be bad to remove the Java Thread objects because we'd
>>> need to recreate them which is rather expensive and violates the
>>> design principle that Compiler Threads are not allowed to call Java.
>>> Removing them wouldn't save much memory. Keeping them in global
>>> handles seems to be beneficial and makes this change easier.
>>
>> Okay.
>>
>>>
>>>> And I thought we would need to add only one threads each time when we
>>>> hit some queue size threshold. At the start queues filled up very
>>>> fast so you may end up creating all compiler threads.
>>> My current formula only creates as much compiler threads so that there
>>> exist 2 compile jobs per thread. I think this is better for startup,
>>> but we can reevaluate this.
>>
>> Would be nice to see graph how number of compiler threads change with
>> time depending on load for some applications (for example, jbb2005 and
>> specjvm2008 if you have them)?
>>
>>>
>>> Thanks for the improvement proposals. I'll implement them next week.
>>> Nevertheless, the current version can already be tested.
>>
>> I started our testing.
>>
>> I just remember that we may need to treat -Xcomp and CTW cases specially.
>>
>> I also ran jtreg testing locally on x64 linux for compiler/jvmci tests.
>> And also tier1 compiler tests with Graal as JIT
>> (-XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI
>> -XX:+TieredCompilation -XX:+UseJVMCICompiler -Djvmci.Compiler=graal).
>> They passed. I think JVMCI code is fine.
>>
>> But I see crash in CompileLog::finish_log_on_error() function in
>> compiler/compilercontrol jtreg tests (they are not in tier1) with normal
>> jtreg runs:
>>
>> FAILED: compiler/compilercontrol/commandfile/LogTest.java
>> FAILED: compiler/compilercontrol/commands/LogTest.java
>> FAILED: compiler/compilercontrol/directives/LogTest.java
>> FAILED: compiler/compilercontrol/jcmd/AddLogTest.java
>> FAILED: compiler/compilercontrol/jcmd/StressAddMultiThreadedTest.java
>> FAILED: compiler/compilercontrol/logcompilation/LogTest.java
>>
>> I started performance testing too.
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> Best regards,
>>> Martin
>>>
>>>
>>> [1] http://en.cppreference.com/w/cpp/language/delete
>>>
>>>
>>> -----Original Message-----
>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>> Sent: Freitag, 23. März 2018 18:17
>>> To: Doerr, Martin <martin.doerr at sap.com>
>>> Cc: Igor Veresov (igor.veresov at oracle.com) <igor.veresov at oracle.com>;
>>> White, Derek <Derek.White at cavium.com>;
>>> 'hotspot-compiler-dev at openjdk.java.net'
>>> <hotspot-compiler-dev at openjdk.java.net>
>>> Subject: Re: RFR(M): 8198756: Lazy allocation of compiler threads
>>>
>>> Very cool!
>>>
>>> Few thoughts.
>>>
>>> You can't delete thread when it is NULL (missing check or refactor code):
>>>
>>>       if (thread == NULL || thread->osthread() == NULL) {
>>> +    if (UseDynamicNumberOfCompilerThreads &&
>>> comp->num_compiler_threads() > 0) {
>>> +      delete thread;
>>>
>>> Why not keep handle instead of returning naked oop from
>>> create_thread_oop()? You create Handle again
>>>
>>> Start fields names with _ to distinguish them from local variable:
>>>
>>> +  static int c1_count, c2_count;
>>>
>>> In possibly_add_compiler_threads() you can use c2_count instead of
>>> calling compile_count() again and array size is fixed
>>> already:
>>>
>>> +    int new_c2_count = MIN3(_c2_compile_queue->size() / 2,
>>> +
>>> CompilationPolicy::policy()->compiler_count(CompLevel_full_optimization),
>>>
>>> And I thought we would need to add only one threads each time when we
>>> hit some queue size threshold. At the start queues
>>> filled up very fast so you may end up creating all compiler threads.
>>> Or we may need more complex formula.
>>>
>>> We may need to free corresponding java thread object when we remove
>>> compiler threads.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 3/23/18 8:04 AM, Doerr, Martin wrote:
>>>> Hi Vladimir,
>>>>
>>>> thanks for updating the RFE. I already had similar ideas so I've
>>>> implemented a prototype.
>>>>
>>>> I'll be glad if you can support this effort.
>>>>
>>>> My implementation starts only one thread per type (C1/C2) initially.
>>>> Compiler threads start additional threads depending
>>>> on the compile queue size, the available memory and the predetermined
>>>> maximum. The Java Thread objects get created
>>>> during startup so the Compiler Threads don't need to call Java.
>>>>
>>>> The heuristics (in possibly_add_compiler_threads()) are just an
>>>> initial proposal and we may want to add tuning
>>>> parameters or different numbers.
>>>>
>>>> Threads get stopped in reverse order as they were created when their
>>>> compile queue is empty for some time.
>>>>
>>>> The feature can be switched by
>>>> -XX:+/-UseDynamicNumberOfCompilerThreads. Thread creating and removal
>>>> can be traced by
>>>> -XX:+TraceCompilerThreads.
>>>>
>>>> Webrev is here:
>>>>
>>>> http://cr.openjdk.java.net/~mdoerr/8198756_CompilerCount/webrev.01/
>>>>
>>>> The following issues need to get addressed, yet:
>>>>
>>>> -Test JVMCI support. (I'm not familiar with it.)
>>>>
>>>> -Possible memory leaks. I've added some delete calls when a thread
>>>> dies, but they may be incomplete.
>>>>
>>>> -Logging.
>>>>
>>>> -Performance and memory consumption evaluation.
>>>>
>>>> It would be great to get support and advice for these issues.
>>>>
>>>> Best regards,
>>>>
>>>> Martin
>>>>