Thanks, Alex!

Jc,

I'll push it if you send me a patch.

Thanks,
Serguei


On 7/12/18 11:30, Alex Menkov wrote:
Looks good to me as well.

--alex

On 07/12/2018 07:25, JC Beyler wrote:
Thanks Serguei!

Anybody motivated to give this a review please?

Thanks!
Jc

On Wed, Jul 11, 2018 at 2:42 PM serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com> <serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com>> wrote:

    Hi Jc,

    The fix looks good.
    I'll sponsor a push once it has been reviewed.

    Thanks,
    Serguei


    On 7/11/18 10:04, JC Beyler wrote:
    Hi all,

    Could someone review the small-ish webrev for the bug:
    https://bugs.openjdk.java.net/browse/JDK-8206960

    The webrev is here:
    http://cr.openjdk.java.net/~jcbeyler/8206960/webrev.00/
<http://cr.openjdk.java.net/%7Ejcbeyler/8206960/webrev.00/>

    Basically, the tests were failing for two reasons:
      - VMEventTest was failing because Graal does not support
    DisableIntrinsic required by the test, I disabled testing the test
    with Graal in this case
      - The other tests were failing because the BCI <-> source code
    line numbers are not always correct when using Graal via uncommon
    traps; therefore the tests now check if Graal is being used and,
    if so, only checks the method names. This allows us to still have
    tests working with Graal, albeit a bit more coarse.

    This passes all the HeapMonitor tests
    with -vmoptions:"-XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI
    -XX:+TieredCompilation -XX:+UseJVMCICompiler -Djvmci.Compiler=graal"

    (Except the GCCMS one which is being fixed via the one-liner for
    JDK-8205643).

    Let me know what you think,
    Jc



--

Thanks,
Jc

Reply via email to