RFR(S/M): 8150646: Add support for blocking compiles through whitebox API

Volker Simonis volker.simonis at gmail.com
Tue Mar 1 18:31:20 UTC 2016


Hi Pavel, Nils, Vladimir,

sorry, but I was busy the last days so I couldn't answer your mails.

Thanks a lot for your input and your suggestions. I'll look into this
tomorrow and hopefully I'll be able to address all your concerns.

Regards,
Volker


On Tue, Mar 1, 2016 at 6:24 PM, Vladimir Kozlov
<vladimir.kozlov at oracle.com> wrote:
> Nils, please answer Pavel's questions.
>
> Thanks,
> Vladimir
>
>
> On 3/1/16 6:24 AM, Nils Eliasson wrote:
>>
>> Hi Volker,
>>
>> An excellent proposition. This is how it should be used.
>>
>> I polished a few rough edges:
>> * CompilerBroker.cpp - The directives was already access in
>> compile_method - but hidden incompilation_is_prohibited. I moved it out
>> so we only have a single directive access. Wrapped compile_method to
>> make sure the release of the directive doesn't get lost.
>> * Let WB_AddCompilerDirective return a bool for success. Also fixed the
>> state - need to be in native to get string, but then need to be in VM
>> when parsing directive.
>>
>> And some comments:
>> * I am against adding new compile option commands (At least until the
>> stringly typeness is fixed). Lets add good ways too use compiler
>> directives instead.
>>
>> I need to look at the stale task removal code tomorrow - hopefully we
>> could save the blocking info in the task so we don't need to access the
>> directive in the policy.
>>
>> All in here:
>> Webrev: http://cr.openjdk.java.net/~neliasso/8150646/webrev.03/
>>
>> The code runs fine with the test I fixed for JDK-8073793:
>> http://cr.openjdk.java.net/~neliasso/8073793/webrev.02/
>>
>> Best regards,
>> Nils Eliasson
>>
>> On 2016-02-26 19:47, Volker Simonis wrote:
>>>
>>> Hi,
>>>
>>> so I want to propose the following solution for this problem:
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8150646_toplevel
>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8150646_hotspot/
>>>
>>> I've started from the opposite site and made the BackgroundCompilation
>>> manageable through the compiler directives framework. Once this works
>>> (and it's actually trivial due to the nice design of the
>>> CompilerDirectives framework :), we get the possibility to set the
>>> BackgroundCompilation option on a per method base on the command line
>>> via the CompileCommand option for free:
>>>
>>>
>>> -XX:CompileCommand="option,java.lang.String::charAt,bool,BackgroundCompilation,false"
>>>
>>>
>>> And of course we can also use it directly as a compiler directive:
>>>
>>> [{ match: "java.lang.String::charAt", BackgroundCompilation: false }]
>>>
>>> It also becomes possible to use this directly from the Whitebox API
>>> through the DiagnosticCommand.compilerDirectivesAdd command.
>>> Unfortunately, this command takes a file with compiler directives as
>>> argument. I think this would be overkill in this context. So because
>>> it was so easy and convenient, I added the following two new Whitebox
>>> methods:
>>>
>>>    public native void addCompilerDirective(String compDirect);
>>>    public native void removeCompilerDirective();
>>>
>>> which can now be used to set arbitrary CompilerDirective command
>>> directly from within the WhiteBox API. (The implementation of these
>>> two methods is trivial as you can see in whitebox.cpp).
>>> v
>>> The blocking versions of enqueueMethodForCompilation() now become
>>> simple wrappers around the existing methods without the need of any
>>> code changes in their native implementation. This is good, because it
>>> keeps the WhiteBox API stable!
>>>
>>> Finally some words about the implementation of the per-method
>>> BackgroundCompilation functionality. It actually only requires two
>>> small changes:
>>>
>>> 1. extending CompileBroker::is_compile_blocking() to take the method
>>> and compilation level as arguments and use them to query the
>>> DirectivesStack for the corresponding BackgroundCompilation value.
>>>
>>> 2. changing AdvancedThresholdPolicy::select_task() such that it
>>> prefers blocking compilations. This is not only necessary, because it
>>> decreases the time we have to wait for a blocking compilation, but
>>> also because it prevents blocking compiles from getting stale. This
>>> could otherwise easily happen in AdvancedThresholdPolicy::is_stale()
>>> for methods which only get artificially compiled during a test because
>>> their invocations counters are usually too small.
>>>
>>> There's still a small probability that a blocking compilation will be
>>> not blocking. This can happen if a method for which we request the
>>> blocking compilation is already in the compilation queue (see the
>>> check 'compilation_is_in_queue(method)' in
>>> CompileBroker::compile_method_base()). In testing scenarios this will
>>> rarely happen because methods which are manually compiled shouldn't
>>> get called that many times to implicitly place them into the compile
>>> queue. But we can even completely avoid this problem by using
>>> WB.isMethodQueuedForCompilation() to make sure that a method is not in
>>> the queue before we request a blocking compilation.
>>>
>>> I've also added a small regression test to demonstrate and verify the
>>> new functionality.
>>>
>>> Regards,
>>> Volker
>>
>> On Fri, Feb 26, 2016 at 9:36 AM, Nils Eliasson
>> <nils.eliasson at oracle.com> wrote:
>>>>
>>>> Hi Vladimir,
>>>>
>>>> WhiteBox::compilation_locked is a global state that temporarily stops
>>>> all
>>>> compilations. I this case I just want to achieve blocking compilation
>>>> for a
>>>> single compile without affecting the rest of the system. The tests
>>>> using it
>>>> will continue executing as soon as that compile is finished, saving time
>>>> where wait-loops is used today. It adds nice determinism to tests.
>>>>
>>>> Best regards,
>>>> Nils Eliasson
>>>>
>>>>
>>>> On 2016-02-25 22:14, Vladimir Kozlov wrote:
>>>>>
>>>>> You are adding parameter which is used only for testing.
>>>>> Can we have callback(or check field) into WB instead? Similar to
>>>>> WhiteBox::compilation_locked.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 2/25/16 7:01 AM, Nils Eliasson wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please review this change that adds support for blocking compiles
>>>>>> in the
>>>>>> whitebox API. This enables simpler less time consuming tests.
>>>>>>
>>>>>> Motivation:
>>>>>> * -XX:-BackgroundCompilation is a global flag and can be time
>>>>>> consuming
>>>>>> * Blocking compiles removes the need for waiting on the compile
>>>>>> queue to
>>>>>> complete
>>>>>> * Compiles put in the queue may be evicted if the queue grows to big -
>>>>>> causing indeterminism in the test
>>>>>> * Less VM-flags allows for more tests in the same VM
>>>>>>
>>>>>> Testing:
>>>>>> Posting a separate RFR for test fix that uses this change. They
>>>>>> will be
>>>>>> pushed at the same time.
>>>>>>
>>>>>> RFE: https://bugs.openjdk.java.net/browse/JDK-8150646
>>>>>> JDK rev: http://cr.openjdk.java.net/~neliasso/8150646/webrev_jdk.01/
>>>>>> Hotspot rev: http://cr.openjdk.java.net/~neliasso/8150646/webrev.02/
>>>>>>
>>>>>> Best regards,
>>>>>> Nils Eliasson
>>>>
>>>>
>>
>


More information about the hotspot-compiler-dev mailing list