<Beans Dev> Fwd: Re: [PATCH] test/jdk/java/beans/PropertyEditor/Test6397609.java failed in JITed code

[Fu Jie]

Thanks Alan for your review and valuable advice.

Forward this email to beans-dev, and the original email was here [1].
A better patch suggested by Alan is
--------------------------------------------------------------------
diff -r b561ea19a7b9 test/jdk/java/beans/PropertyEditor/Test6397609.java
--- a/test/jdk/java/beans/PropertyEditor/Test6397609.java Wed Jan 02 
15:33:32 2019 -0800
+++ b/test/jdk/java/beans/PropertyEditor/Test6397609.java Thu Jan 03 
09:02:29 2019 +0800
@@ -32,6 +32,7 @@
   */

  import java.beans.PropertyEditorManager;
+import java.lang.ref.Reference;

  public class Test6397609 {
      public static void main(String[] args) throws Exception {
@@ -44,6 +45,7 @@
          if (!isEditorExist(Object.class)) {
              throw new Error("the editor is lost");
          }
+        Reference.reachabilityFence(loader);
          loader = null; // clean the reference
          if (isEditorExist(Object.class)) {
              throw new Error("unexpected editor is found");
--------------------------------------------------------------------

Would you please review it and tell me if it's OK to file on JBS and 
post a webrev.
Thanks.

[1] https://mail.openjdk.java.net/pipermail/jdk-dev/2019-January/002491.html

Best Regards,
Jie


-------- Forwarded Message --------
Subject: 	Re: [PATCH] 
test/jdk/java/beans/PropertyEditor/Test6397609.java failed in JITed code
Date: 	Wed, 2 Jan 2019 10:46:24 +0000
From: 	Alan Bateman <Alan.Bateman at oracle.com>
To: 	Fu Jie <fujie at loongson.cn>, jdk-dev at openjdk.java.net



On 02/01/2019 07:01, Fu Jie wrote:
> Hi all,
>
> test/jdk/java/beans/PropertyEditor/Test6397609.java failed in JITed 
> code of Test6397609::main.
>
> The failure was caused by the lost of the OopMap item for the object 
> loader which was optimized out by the JIT with the liveness analysis 
> optimization.
> For more details, see [1].
>
> It seems that Test6397609.java is only suitable for testing 
> interpreters which is not adapted to JITs at all.
> Different behaviors performed by the interpreter and JITs with the 
> same test case really confused people a lot.
> And I think it's worth making the test also suitable for JITs.
>
> A tiny change can fix this issue
> ---------------------------------
> diff -r b99b41325d89 test/jdk/java/beans/PropertyEditor/Test6397609.java
> --- a/test/jdk/java/beans/PropertyEditor/Test6397609.java Tue Jan 01 
> 20:09:02 2019 -0500
> +++ b/test/jdk/java/beans/PropertyEditor/Test6397609.java Wed Jan 02 
> 12:01:21 2019 +0800
> @@ -44,6 +44,7 @@
>          if (!isEditorExist(Object.class)) {
>              throw new Error("the editor is lost");
>          }
> +        loader.hashCode(); // prevent being optimized out by JIT
>          loader = null; // clean the reference
>          if (isEditorExist(Object.class)) {
>              throw new Error("unexpected editor is found");
The beans-dev list is probably the right place to discuss this change. 
Also you might want to consider using Reference.reachabilityFence to 
keep loader from being GC'ed.

-Alan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/beans-dev/attachments/20190103/2b0987ff/attachment.html>