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

Vladimir Kozlov vladimir.kozlov at oracle.com
Thu Apr 18 15:25:16 PDT 2013 

I wish we also record core's location in hs_err file so you can grep it 
instead of stderr output.

Can you save command's output (stderr and stdout) into a file and grep 
it instead?

Thanks,
Vladimir

On 4/18/13 1:48 PM, Igor Ignatyev wrote:
> Vladimir,
> thank you for review.
>
> i have additional small changes for it:
> - improved finding of coredump location, now it correctly works on mac os.
>
> http://cr.openjdk.java.net/~iignatyev/8011675/webrev.06/
>
> Best regards,
> Igor Ignatyev
>
> On 04/17/2013 09:36 PM, Vladimir Kozlov wrote:
>> This looks good.
>>
>> Thanks,
>> Vladimir
>>
>> On 4/17/13 10:08 AM, Igor Ignatyev wrote:
>>> 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