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

Igor Ignatyev igor.ignatyev at oracle.com
Mon Apr 22 06:58:55 PDT 2013 

I have rerun JPRT:
  - TestSA.sh passed on macosx
  - TestSA.sh failed only due to known bugs in SA.
  - other ciReplay tests passed

Best regards,
Igor Ignatyev

On 04/19/2013 08:38 PM, Igor Ignatyev wrote:
>> 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