RFR(S) : 8011675: adding compilation level to replay data

Igor Ignatyev igor.ignatyev at oracle.com
Wed Apr 17 10:08:13 PDT 2013 

http://cr.openjdk.java.net/~iignatyev/8011675/webrev.05/

> TestVM_no_comp_level.sh:
>
>    61     negative_test 10 -XX:-TieredCompilation -client
>    62     negative_test 11 -XX:+TieredCompilation -client
>
replaced by one negative test with just '-client' as option

> common.sh:
>
>   107         negative_test $1 -XX:-TieredCompilation -client
>   108         negative_test `expr $1 + 1` -XX:+TieredCompilation -client
>
also replaced by one negative test with just '-client' as option

> Also instead of number for error code can you use messages? It will be
> very difficult to trace which test failed by number especially since you
> use expressions for it.
added test name to test's output. Using of different exit codes is 
necessary for better bug matching.

> We should disable core files generation and tests on Mac OS. The Nightly
> should be clean.
'TestSA.sh' will be skipped on Mac OS, if it isn't configured for core 
dumping

Best regards,
Igor Ignatyev

On 04/16/2013 09:55 PM, Vladimir Kozlov wrote:
> Sorry, I notice only now that next tests will fail because there is no
> Tiered in Client VM and not because there is a problem in Replay code.
> So it does not test Replay code:
>
> TestVM_no_comp_level.sh:
>
>    61     negative_test 10 -XX:-TieredCompilation -client
>    62     negative_test 11 -XX:+TieredCompilation -client
>
> common.sh:
>
>   107         negative_test $1 -XX:-TieredCompilation -client
>   108         negative_test `expr $1 + 1` -XX:+TieredCompilation -client
>
> Also instead of number for error code can you use messages? It will be
> very difficult to trace which test failed by number especially since you
> use expressions for it.
>
> On 4/16/13 9:06 AM, Igor Ignatyev wrote:
>>  > Okay. Please, test it: generate replay.txt with C1 without recording
>>  > comp_level (old style) and then try to replay it with new code.
>> 'TestVM_no_comp_level.sh' emulates old style of replay data.
>
> Okay.
>
>>
>>  >>> Add tests for tiered with different -XX:TieredStopAtLevel=n.
>>  >> what exactly did you mean?
>>  >> 1) generate replay.txt by VM with different value of
>> 'TieredStopAtLevel'
>>  >> 2) use different 'TieredStopAtLevel' during replay of compilation
>>  >
>>  > I meant 1). VM could crash at any level of compilation because they
>> are
>>  > different so we need to be able replay all of them. So I want to
>> verify
>>  > that it works (positive tests).
>> block 'if [ $is_tiered -eq 1 ]' was added to TestVM.sh
>
> Thanks.
>
>>
>>> Run in JPRT to verify that tests work on all platforms.
>> There are some failures in TestSA.sh due to known bugs in SA
>> (JDK-8011888, JDK-6326653) and environment/infra issue on macosx (core
>> wasn't generated because '/cores' directory doesn't exist or isn't
>> writable by current user[1]).
>
> We should disable core files generation and tests on Mac OS. The Nightly
> should be clean.
>
> Thanks,
> Vladimir
>
>>
>> updated webrev: http://cr.openjdk.java.net/~iignatyev/8011675/webrev.04/
>>
>> [1]
>> http://developer.apple.com/library/mac/technotes/tn2124/_index.html#//apple_ref/doc/uid/DTS10003391-CH1-SECCOREDUMPS
>>
>>
>>
>> Best regards,
>> Igor Ignatyev
>>
>> On 04/16/2013 01:35 AM, Vladimir Kozlov wrote:
>>> On 4/13/13 12:14 PM, Igor Ignatyev wrote:
>>>>> If you replaying C1 compilation next setting will be incorrect. Should
>>>>> we use CompLevel_highest_tier instead so that following check pass?:
>>>>>
>>>>> +     if (had_error() && (error_message() == comp_level_label)) {
>>>>> +       comp_level = CompLevel_full_optimization;
>>>>>
>>>>
>>>> i added this code for compatible with old version and i think that it
>>>> must has the same behavior (use 'CompLevel_full_optimization'). in new
>>>> version we always will have comp_level in replay.txt.
>>>> so i think we shouldn't use CompLevel_highest_tier.
>>>
>>> Okay. Please, test it: generate replay.txt with C1 without recording
>>> comp_level (old style) and then try to replay it with new code.
>>>
>>>>
>>>>> Add tests for tiered with different -XX:TieredStopAtLevel=n.
>>>> what exactly did you mean?
>>>> 1) generate replay.txt by VM with different value of
>>>> 'TieredStopAtLevel'
>>>> 2) use different 'TieredStopAtLevel' during replay of compilation
>>>
>>> I meant 1). VM could crash at any level of compilation because they are
>>> different so we need to be able replay all of them. So I want to verify
>>> that it works (positive tests).
>>>
>>>>
>>>> if 1st, i don't think that test must to iterate through all possible
>>>> values, it must be done by harness, test execution etc
>>>
>>> I agree with you but, unfortunately, we are short on testing hardware to
>>> add an other testing variation. You should know. So adding this testing
>>> here when tiered VM is tested will help us now.
>>>
>>>> if 2nd, current implementation of ciReplay ignore TieredStopAtLevel
>>>> flags, so these tests will be useless.
>>>>
>>>>> You may still not find core file. Or ulimit -c unlimited will not work
>>>>> (windows, OS X?). You need to bailout in such case as success.
>>>> for windows i added '-XX:+CreateMinidumpOnCrash', but whatever case of
>>>> not found core-file need to treat as a environment issue, a test bug or
>>>> may even be a product bug. anyway if it happened, i think it would be
>>>> better to know about this than just skip the test.
>>>>
>>>> i updated webrev according to the rest of your suggestions, but i'm not
>>>> sure about flags to limit core size.
>>>
>>> Run in JPRT to verify that tests work on all platforms.
>>>
>>>>
>>>> also i added one more check of comp_level and moved validation to
>>>> separate method.
>>>
>>> OK.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>
>>>> http://cr.openjdk.java.net/~iignatyev/8011675/webrev.03/
>>>>
>>>> Best regards,
>>>> Igor Ignatyev
>>>>
>>>> On 04/13/2013 02:14 AM, Vladimir Kozlov wrote:
>>>>> I would set CrashAfterCompileId default to -1 as indication it is not
>>>>> set. Then you don't need to check CrashAfterCompileId == 0 since
>>>>> compile_id is positive. By the way, it is very nice idea but add a
>>>>> comment about what that code for. I did not get it first :)
>>>>>
>>>>> About name. For such flags we use CI: CIBreakAt, CIStop. So I would
>>>>> suggest to use CICrashAt. Which means VM crash with that compile id,
>>>>> not
>>>>> when it is greater.
>>>>>
>>>>> If you replaying C1 compilation next setting will be incorrect. Should
>>>>> we use CompLevel_highest_tier instead so that following check pass?:
>>>>>
>>>>> +     if (had_error() && (error_message() == comp_level_label)) {
>>>>> +       comp_level = CompLevel_full_optimization;
>>>>>
>>>>>
>>>>> About tests.
>>>>>
>>>>> Add -Xmx32m, -XX:ParallelGCThreads=2 and other flags (codecache
>>>>> ,metaspace) to limit core size on big machines.
>>>>>
>>>>> Add tests for tiered with different -XX:TieredStopAtLevel=n.
>>>>>
>>>>> I would add more info in output why the test is skipped:
>>>>>       echo TEST SKIPPED
>>>>>
>>>>> typo 'java':
>>>>>   124 is_tiered=`${JAVA} ${TESTVMOPTS} java -XX:+PrintFlagsFinal
>>>>> -version 2>&1 | grep TieredCompilation | grep -c true`
>>>>>
>>>>>
>>>>> I don't like next:
>>>>>
>>>>>   130 ${JAVA} ${TESTVMOPTS} -Xcomp -XX:CrashAfterCompileId=1
>>>>> -XX:+CreateMinidumpOnCrash -XX:+DumpReplayDataOnError -version &
>>>>>   131 crash_pid=$!
>>>>>   132 wait $crash_pid
>>>>>
>>>>>   On windows 'wait $crash_pid' may not work (happened before) because
>>>>> before you execute 'wait' OS may assign the same pid to different
>>>>> process.
>>>>>
>>>>> You may still not find core file. Or ulimit -c unlimited will not work
>>>>> (windows, OS X?). You need to bailout in such case as success. Next
>>>>> code
>>>>> is incorrect for that:
>>>>>
>>>>>    48 if [ -z "${core_file}" -o ! -r "${core_file}" ]
>>>>>    49 then
>>>>>    50     echo TEST FAILED: core wasn\'t generated
>>>>>    51     exit 2
>>>>>    52 fi
>>>>>
>>>>> Thanks
>>>>> Vladimir
>>>>>
>>>>> On 4/12/13 8:27 AM, Igor Ignatyev wrote:
>>>>>> updated webrev:
>>>>>> http://cr.openjdk.java.net/~iignatyev/8011675/webrev.02/
>>>>>> added:
>>>>>>    1. more informative message on unsupported comp_level
>>>>>>    2. not-product flag 'CrashAfterCompileId' and assert in
>>>>>> compileBroker.cpp for assured generation of replay.txt
>>>>>>    2. tests for ciReplay
>>>>>>
>>>>>> Best regards,
>>>>>> Igor Ignatyev
>>>>>>
>>>>>> On 04/09/2013 11:40 AM, Vladimir Kozlov wrote:
>>>>>>> Igor,
>>>>>>>
>>>>>>> Sorry, I may said it not clear. What I meant is to set error (call
>>>>>>> report_error()) in such case and return from process_compile()
>>>>>>> without
>>>>>>> compilation. But let ciReplay::replay() finish and exit.
>>>>>>> replay_impl()
>>>>>>> should print the problem so you don't need to do it in
>>>>>>> process_compile(). And, please, test it.
>>>>>>> The message should be more informative, something like:
>>>>>>> "compilation level %d requires TieredCompilation"
>>>>>>> An other bad cases is when failure happened in Client (C1) or Server
>>>>>>> (C2) but the replay is run with opposite VM.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 4/9/13 12:04 AM, Igor Ignatyev wrote:
>>>>>>>> Vladimir,
>>>>>>>>
>>>>>>>> Thank you for review.
>>>>>>>>
>>>>>>>> updated webrev:
>>>>>>>> http://cr.openjdk.java.net/~iignatyev/8011675/webrev.01/
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Igor Ignatyev
>>>>>>>>
>>>>>>>> On 04/09/2013 04:19 AM, Vladimir Kozlov wrote:
>>>>>>>>> Igor,
>>>>>>>>>
>>>>>>>>> Thank you for fixing and cleaning this.
>>>>>>>>>
>>>>>>>>> In vmStructs.cpp move _comp_level after _compile_id as in nmethod
>>>>>>>>> class.
>>>>>>>>>
>>>>>>>>> Typo in ciReplay.cpp:
>>>>>>>>> +      comp_level != CompLevel_highest_tier;
>>>>>>>>>
>>>>>>>>> Actually it is useless to replay compilation at different
>>>>>>>>> level, it
>>>>>>>>> should exit with error. Could happened because incorrectly run
>>>>>>>>> without
>>>>>>>>> Tiered when the crush happened with Tiered.
>>>>>>>>>
>>>>>>>>> In ciReplay.cpp use parenthesis around equality checks: if (. &&
>>>>>>>>> (. ==
>>>>>>>>> .)).
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Vladimir
>>>>>>>>>
>>>>>>>>> On 4/8/13 12:30 PM, Igor Ignatyev wrote:
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> Please review patch.
>>>>>>>>>>
>>>>>>>>>> Problem:
>>>>>>>>>> Replay of compilations supports c1 and tiered as well as c2, the
>>>>>>>>>> only
>>>>>>>>>> thing missing is comp_level.
>>>>>>>>>>
>>>>>>>>>> Fix:
>>>>>>>>>> 1. added comp_level into dump and usage during replay
>>>>>>>>>> 2. minor changes in agent/doc:
>>>>>>>>>>    - escaped '<', '>'
>>>>>>>>>>    - renamed 'C2 Replay'/'C2 compiler replay' to
>>>>>>>>>> 'Replay'/'Compiler
>>>>>>>>>> replay'
>>>>>>>>>>
>>>>>>>>>> Testing:
>>>>>>>>>> manually launched the replay from JDK-8010934 (assert
>>>>>>>>>> specifically to
>>>>>>>>>> c1) on jvm w/o corresponded fix. assert was triggered w/ patch
>>>>>>>>>> and
>>>>>>>>>> wasn't triggered w/o.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8011675/webrev.00/
>>>>>>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-8011675

More information about the hotspot-compiler-dev mailing list