Does openjdk8u need pre-barrier for jweak?

[Liu, Xin]

Hi, Kim and Aleksey, 
Thank you for answering me.  I am new in both hotspot and GC. 
 
@Alekey, 
In your way, you miss the chance to clear JNI weak reference here. 
http://hg.openjdk.java.net/jdk8u/jdk8u/hotspot/file/b4ee249eb1c4/src/share/vm/runtime/jniHandles.cpp#l393

Our service generates many MemberName objects  on-the-fly. They are cached in MemberNameTable.  
http://hg.openjdk.java.net/jdk8u/jdk8u/hotspot/file/b4ee249eb1c4/src/share/vm/prims/methodHandles.cpp#l954
it will create memory leak right? Because the table is bigger and bigger. 

Except JVM exit, I can't find any chance to release those oops in table. Can full GC collect them?

Tag JNI reference is an Arch/GC-independent way. I found jdk12 repo uses C++ metaprogramming to emit proper barriers for different GCs.
 We can use if/elif for both G1 and Shenandoah if you don’t want to keep simple.  

Can we consider my backported patch? I don't have permission to submit sandbox repo, But I ran SPECjvm2008 and didn't observe any performance impact. 
 
@Kim, 
Thank you give me the point to the test. Yes, indeed, it's basing on whitebox. 
I can mimic the race condition by generating many global weak references, but it cannot be deterministic. I don't know how to write a reproducible and deterministic jtreg test.

Thanks,
--lx

On 8/14/18, 10:49 AM, "Kim Barrett" <kim.barrett at oracle.com> wrote:

    > On Aug 14, 2018, at 5:04 AM, Aleksey Shipilev <shade at redhat.com> wrote:
    > 
    > On 08/14/2018 02:30 AM, Liu, Xin wrote:
    >> Another question is which regression test can trigger the issue? Why did you only identify the
    >> problem on jdk9+?
    > 
    > This sounds familiar. In Shenandoah JDK 8 backport, we "just" disable jweak processing wholesale
    > because of bugs around jweak handling. It seems risky to backport the fixes to 8u. Shenandoah's
    > workaround also includes the test that fails:
    >  http://hg.openjdk.java.net/shenandoah/jdk8u/hotspot/rev/ba692039a40a
    > 
    > Maybe the way to avoid G1 crashes would be to workaround the bug the same way for G1?
    > 
    > -Aleksey
    
    The lack of a SATB barrier for jweak was a day1 G1 bug, so it is
    indeed present in JDK 8.  I'm not sure why 8 wasn't listed in the
    original bug's Affects Version.
    
    There is a regression test for this:
    test/hotspot/jtreg/gc/g1/TestJNIWeakG1/TestJNIWeakG1.java
    However, this test uses a WhiteBox feature that was added in JDK 10
    (requestConcurrentGCPhase and friends).
    
    I agree with Aleksey that backporting all the relevant stuff to JDK 8
    would be pretty risky.  The workaround described for Shenandoah seems
    okay to me (treat the JNI weak globals just like the normal JNI
    globals for concurrent marking, e.g. also scan the weak globals in the
    initial mark pause).  This may prevent reclamation of some objects
    that otherwise could be, but that's safer than premature reclamation.
    (I think the Shenandoah patch's implementation of the workaround is
    wrong, but that's a different issue.)
    
    I suspect that at the time we expected JDK 8 to soon be supplanted by
    JDK 9 and later, so didn't worry about addressing the problem in JDK
    8.  With an extended lifetime for JDK 8, some decisions like that may
    need to be revisited.
    
    

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SIM-JDK-802-backport-add-pre-write-barrier-for-jweak.patch
Type: application/octet-stream
Size: 66040 bytes
Desc: 0001-SIM-JDK-802-backport-add-pre-write-barrier-for-jweak.patch
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20180815/eadce237/0001-SIM-JDK-802-backport-add-pre-write-barrier-for-jweak.patch>