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
