RFR(M): 8006683: Add WhiteBox API to testing of compiler

[Igor Ignatyev]

Hi Mikael,

> Also, is it really necessary to have constructs like:
> return (value ? JNI_TRUE : JNI_FALSE);
This construct was added after I had found it in jni.cpp, unsafe.cpp.
I will remove it from my patch. And I think that '(jboolean)value' is 
unnecessary, too.

> I think the test should be renamed to something like
> CompilerWhiteBoxTest, to make it clearer that it only uses compiler
> functions.
ok, I will rename it

> create a utility function for tests that can
> check the value of a VM flag through the HotSpotDiagnosticMXBean
I thought about it, but decided that it might unnecessarily complicate test.

Best regards,
Igor Ignatyev

On 01/30/2013 01:48 PM, Mikael Gerdin wrote:
> Hi Igor,
>
> It's really nice to see more uses of the WhiteBox API!
> This is exactly the kind of use the API is intended for.
>
> On 01/29/2013 09:31 PM, Igor Ignatyev wrote:
>> Hi all,
>>
>> Please review the patch
>>
>> 1. added new functions to WhiteBox:
>>    public native void    deoptimizeAll();
>>    public native boolean isMethodCompiled(Method method);
>>    public native boolean isMethodCompilable(Method method);
>>    public native boolean isMethodQueuedForCompilation(Method method);
>>    public native int     deoptimizeMethod(Method method);
>>    public native boolean setDontInlineMethod(Method method, boolean
>> value);
>>    public native int     getCompileQueuesSize();
>
> The changes to WhiteBox.java, parserTests.hpp, whitebox.hpp look good.
>
> The actual calls to the compiler code in whitebox.cpp should be reviewed
> for correctness by the compiler developers but as for the style of the
> code I think it's a bit "compact" and lines like:
>
> +   return ((code != NULL && code->is_alive() &&
> !code->is_marked_for_deoptimization()) ? JNI_TRUE : JNI_FALSE);
>
> don't improve readability.
> Also, is it really necessary to have constructs like:
> return (value ? JNI_TRUE : JNI_FALSE);
> can you instead write
> return (jboolean)value;
>
>>
>> 2. added test 'test/compiler/WBApiTest.java' which exercises these
>> functions
>
> I think the test should be renamed to something like
> CompilerWhiteBoxTest, to make it clearer that it only uses compiler
> functions.
> Could you create a whitebox function to ask the VM for the
> COMPILE_THRESHOLD instead of having it as a constant in the test?
> Or perhaps even better, create a utility function for tests that can
> check the value of a VM flag through the HotSpotDiagnosticMXBean?
> Then you could dynamically check the value of CompileThreshold without
> needing to add a redundant whitebox function.
>
> Other than that the test looks good to me.
>
>>
>> 3. added result type to macro WB_METHOD_DECLARE
>
> Good.
>
>>
>> webrev: http://cr.openjdk.java.net/~vlivanov/8006683/webrev.00/
>
> /Mikael
>