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

Wed Apr 4 13:16:26 UTC 2018

Hi Vladimir,

our test results look good with the rebased webrev.03, but we don't build with JVMCI. So you could still see test failures which don't show up in our environment.

Do you know if there are still things missing or known problems with webrev.03?

Best regards,
Martin

-----Original Message-----
From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Doerr, Martin
Sent: Samstag, 31. März 2018 16:49
To: Vladimir Kozlov <vladimir.kozlov at oracle.com>
Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
Subject: [CAUTION] RE: RFR(M): 8198756: Lazy allocation of compiler threads

Hi Vladimir,

I have added your changes, but switched on ReduceNumberOfCompilerThreads by default:
http://cr.openjdk.java.net/~mdoerr/8198756_CompilerCount/webrev.03/

I have also added a fix for the stack size test. I will run testing and look at further issues on Tuesday (the next business day in Germany).

Best regards,
Martin

-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Donnerstag, 29. März 2018 19:53
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

Okay. I posted webrev.02 testing failures in bug report.

Thanks,
Vladimir K

On 3/29/18 10:15 AM, Doerr, Martin wrote:
> Hi Vladimir,
> 
> sorry, I had missed your proposals you have added to the bug while I was sick. webrev.02 is only based on the emails.
> I'll think about the ideas which were written in the bug next week.
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Donnerstag, 29. März 2018 18:42
> 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,
> 
> 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
>>>>>