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

Igor Ignatyev igor.ignatyev at oracle.com
Fri Apr 19 09:38:24 PDT 2013 

> Can you save command's output (stderr and stdout) into a file and grep
> it instead?
i did it, even i don't see any reason for it. could you explain the 
cause of your request?

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

Best regards,
Igor Ignatyev

On 04/19/2013 02:25 AM, Vladimir Kozlov wrote:
> 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