Hi Jc,
The fix looks good to me too.
minor note:
564 if (JNI_ENV_PTR(jni)->ExceptionOccurred(JNI_ENV_ARG(jni))) {
565 fprintf(stderr, "fill_native_frames: Exception in jni
NewWeakGlobalRef\n");
566 }
I'd expect error message mention "event_storage_add" instead
"fill_native_frames"
No need to resent webrev.
--alex
On 08/08/2018 10:56, JC Beyler wrote:
Hi Serguei,
Sounds good to me. If someone else could review it, I could add the
metadata to the webrev and, once testing (as you say) confirms nothing
is broken, we could push it in.
If someone is motivated, that would be awesome and much appreciated!
Jc
On Wed, Aug 8, 2018 at 12:08 AM serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com> <serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com>> wrote:
Hi Jc,
This looks good to me especially because we discussed it a lot
internally.
The testing looks pretty good too.
I submitted mach5 jobs for HepMonitor tests repeating 100 times and
also normal tier1, tier2, hs-tier2-5.
But it was not tested with the tiers 6 & 7 yet (most likely, need to
make sure they run well too).
Thanks,
Serguei
On 8/6/18 08:27, JC Beyler wrote:
Hi all,
I updated the version and Serguei tested it for me so this change
should not change the stability of the tests. This has the
advantage of not having to complicate the Java tests at all, while
adding the post-JNI call checks.
Webrev: http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.04/
<http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.04/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8208303
Thanks all for your reviews!
Jc
On Fri, Jul 27, 2018 at 4:36 PM JC Beyler <jcbey...@google.com
<mailto:jcbey...@google.com>> wrote:
Hi all,
I did the new version that calls FatalError if JNI fails a
call. This has the advantage of not having to complicate the
Java tests at all, while adding the post-JNI call checks.
Webrev:
http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.03/
<http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.03/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8208303
Thanks all!
Jc
On Thu, Jul 26, 2018 at 12:52 PM Chris Plummer
<chris.plum...@oracle.com <mailto:chris.plum...@oracle.com>>
wrote:
I'm pretty sure changes that only affect tests can be any
priority. But still, be a lot more cautious the closer we
get to release.
Chris
On 7/26/18 12:15 PM, Daniel D. Daugherty wrote:
We entered RDP2 today (07.26). So only P1 and P2 bug
fixes allowed.
Dan
On 7/26/18 3:14 PM, serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com> wrote:
Yes, of course it has to be well tested before the push.
Does it make sense to plan it to push to 11 (after th
testing is done)?
Thanks,
Serguei
On 7/26/18 12:08, Daniel D. Daugherty wrote:
Please make sure this fix is well tested in Mach5 prior
to pushing.
In particular, I'm focused on reducing the noise in
Mach5 tier[1-3]
so adding any new failures there will make me grumpy :-)
Dan
On 7/26/18 3:03 PM, JC Beyler wrote:
Hi all,
With the FatalError idea, here is the webrev to
consider, note it no longer changes the tests. If a
JNI call fails, then we call FatalError.
Let me know what you think:
Webrev:
http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.01/
<http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.01/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8208303
Thanks!
Jc
On Thu, Jul 26, 2018 at 10:46 AM
serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com>
<serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com>> wrote:
Hi Jc,
Good idea.
I was thinking about something like this.
Thanks,
Serguei
On 7/26/18 10:40, JC Beyler wrote:
Hi Serguei,
As I was looking at another test bug
(https://bugs.openjdk.java.net/browse/JDK-8191519);
the proposal for that bug is to have a JNI call
to FatalError to provoke a failure.
If we went down that route, this webrev is
simpler, no? Instead of setting failure_status
and checking it later; just fail fatally and be
done with it, no? That way, the tests in Java
land don't have to be changed actually, no?
What would we prefer for tests? Remember there
was a failure and test it later or fail fast via
JNI's FatalError?
Thanks,
Jc
On Thu, Jul 26, 2018 at 10:04 AM
serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com>
<serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com>> wrote:
Hi Jc,
It looks good to me.
Thanks,
Serguei
On 7/26/18 09:58, JC Beyler wrote:
Hi all,
The tests in the HeapMonitor subsystem has a
lot of JNI calls. There is a need for
verification and testing if anything in the
JNI subsystem failed unexpectedly.
Here is a webrev that tracks if a JNI call
does fail and the tests will fail if any JNI
call does fail.
Could I have a few reviews please for:
Webrev:
http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.00/
<http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.00/>
Bug:
https://bugs.openjdk.java.net/browse/JDK-8208303
Thanks,
Jc
--
Thanks,
Jc
--
Thanks,
Jc
--
Thanks,
Jc
--
Thanks,
Jc
--
Thanks,
Jc
