8256011: Shenandoah: Don't resurrect finalizably reachable objects #1109

rkennke · 2020-11-07T20:10:14Z

In the weak-LRB we currently return referents when it is 'marked', that is when it's either reachable strongly or through a finalizable object. This means a finalizable object can be resurrected by Reference.get(), which is wrong. Only truly strongly reachable objects should be returned by Reference.get() during weak-reference-processing.

I had to reconsider the way we call into runtime-LRBs from generated code for these reasons:

We need to distinguish phantom, weak and strong reference strength, and native vs in-heap access. Those are two orthogonal dimensions
We can have strong and phantom native referents, and strong and weak in-heap referents
Native referents are never compressed

Note that this depends on PR#1140.

Testing:

hotspot_gc_shenandoah (x86_64, x64_32, aarch64)
tier1 +UseShenandoahGC +ShenandoahVerify
tier2 +UseShenandoahGC +ShenandoahVerify

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Testing

	Linux x64	Linux x86	Windows x64	macOS x64
Build	✔️ (5/5 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)
Test (tier1)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)

Issue

JDK-8256011: Shenandoah: Don't resurrect finalizably reachable objects

Reviewers

Aleksey Shipilev (@shipilev - Reviewer)
Zhengyu Gu (@zhengyu123 - Reviewer) ⚠️ Review applies to 37c9778

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1109/head:pull/1109
$ git checkout pull/1109

bridgekeeper · 2020-11-07T20:10:26Z

👋 Welcome back rkennke! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2020-11-07T20:11:29Z

@rkennke The following labels will be automatically applied to this pull request:

hotspot-gc
shenandoah

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2020-11-07T20:15:00Z

Webrevs

…n on phantom access

shipilev

This makes sense, thanks.

openjdk · 2020-11-08T10:31:22Z

@rkennke This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8256011: Shenandoah: Don't resurrect finalizably reachable objects

Reviewed-by: shade, zgu

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 22 new commits pushed to the master branch:

05b8245: 8256290: javac/lambda/T8031967.java fails with StackOverflowError on x86_32
c3139ab: 8256220: C1: x86_32 fails with -XX:UseSSE=1 after JDK-8210764 due to mishandled lir_neg
1d3d64f: 8255973: Add more logging to debug JDK-8255917
e32a4ea: 8253820: Save test images and dumps with timestamps from client sanity suite
dff26a4: 8256063: Module::getPackages on an unnamed module may return packages that are in a named module
531c56e: 8256278: Shenandoah: Avoid num of dead callback from weak processor in Shenandoah root verifier
b5a9c92: 8256244: java/lang/ProcessHandle/PermissionTest.java fails with TestNG 7.1
90f9a70: 8255546: Missing coverage for javax.smartcardio.CardPermission and ResponseAPDU
1c0b490: 8256201: java/awt/FullScreen/FullscreenWindowProps/FullscreenWindowProps.java failed
3e70aac: 8254162: Implementation of Foreign-Memory Access API (Third Incubator)
... and 12 more: https://git.openjdk.java.net/jdk/compare/59965c17b40d03bbcbcd15b128f01c00aa6c16de...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

zhengyu123

Looks good.

shipilev

I have many questions :)

src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp

src/hotspot/share/gc/shenandoah/c2/shenandoahBarrierSetC2.cpp

src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp

src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp

src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp

src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp

src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp

src/hotspot/share/gc/shenandoah/c2/shenandoahBarrierSetC2.cpp

shipilev

This looks fine to me. Let Zhengyu look through this thoroughly.

src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp

src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetAssembler_aarch64.cpp

shipilev

This looks fine to me. Good to go, assuming the tests pass.

…in-heap

shipilev

Okay, looks good.

rkennke · 2020-11-13T09:46:03Z

/integrate

openjdk · 2020-11-13T09:47:31Z

@rkennke Since your change was applied there have been 25 commits pushed to the master branch:

41139e3: 8255964: Add all details to jstack log in jtreg timeout handler
b4d0186: 8253525: Implement getInstanceSize/sizeOf intrinsics
ea576dd: 8254887: C2: assert(cl->trip_count() > 0) failed: peeling a fully unrolled loop
05b8245: 8256290: javac/lambda/T8031967.java fails with StackOverflowError on x86_32
c3139ab: 8256220: C1: x86_32 fails with -XX:UseSSE=1 after JDK-8210764 due to mishandled lir_neg
1d3d64f: 8255973: Add more logging to debug JDK-8255917
e32a4ea: 8253820: Save test images and dumps with timestamps from client sanity suite
dff26a4: 8256063: Module::getPackages on an unnamed module may return packages that are in a named module
531c56e: 8256278: Shenandoah: Avoid num of dead callback from weak processor in Shenandoah root verifier
b5a9c92: 8256244: java/lang/ProcessHandle/PermissionTest.java fails with TestNG 7.1
... and 15 more: https://git.openjdk.java.net/jdk/compare/59965c17b40d03bbcbcd15b128f01c00aa6c16de...master

Your commit was automatically rebased without conflicts.

Pushed as commit b0c28fa.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Reviewed-by: shade, zgu

8256011: Shenandoah: Don't resurrect finalizably reachable objects

672333c

openjdk bot added the rfr label Nov 7, 2020

openjdk bot added hotspot-gc shenandoah labels Nov 7, 2020

Simplify condition, don't resurrect finalizably reachable objects eve…

189be78

…n on phantom access

shipilev approved these changes Nov 8, 2020

View changes

openjdk bot added the ready label Nov 8, 2020

rkennke added 2 commits Nov 9, 2020

Merge branch 'master' into JDK-8256011

126f159

Merge branch 'master' into JDK-8256011

ab1eda0

zhengyu123 approved these changes Nov 9, 2020

View changes

rkennke mentioned this pull request Nov 10, 2020

8255999: Shenandoah: Invoke native-LRB only on non-strong refs, again #1101

Closed

3 tasks

Ensure correct strength and width of LRB runtime calls

8d979ee

openjdk bot added hotspot and removed hotspot-gc labels Nov 10, 2020

shipilev suggested changes Nov 10, 2020

View changes

rkennke added 5 commits Nov 10, 2020

Give different name to different LRB runtime calls

81b4474

separate code-paths for phantom- and weak-access in runtime-LRB

05b2c5c

Use ShBarrierSet::is_strong_access() in CmpP optimization

3542ca5

Mask decorators in hash/cmp, not in ctor

c0734e6

Whitespace changes

ac2674f

shipilev approved these changes Nov 10, 2020

View changes

Aarch64 parts

d361e60

zhengyu123 suggested changes Nov 10, 2020

View changes

src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp Show resolved Hide resolved

zhengyu123 suggested changes Nov 10, 2020

View changes

src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp Outdated Show resolved Hide resolved

src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetAssembler_aarch64.cpp Outdated Show resolved Hide resolved

rkennke added 2 commits Nov 10, 2020

Fixes/missing parts for x86_64

7f3f14b

Remove superfluous LP64 in aarch64 part

37c9778

zhengyu123 approved these changes Nov 10, 2020

View changes

rkennke added 5 commits Nov 10, 2020

Don't make phantom-access narrow (mistake when doing 32bit parts

92a92fc

Merge branch 'master' into JDK-8256011

59a0733

Merge branch 'master' into JDK-8256011

0087aa6

Fix after merge

7628e3a

Fix/invert condition in CmpP optimization

c0ee934

shipilev approved these changes Nov 11, 2020

View changes

rkennke added 8 commits Nov 11, 2020

Merge branch 'master' into JDK-8256011

245d601

Move resurrection-barrier from JDK-8256020 into right place after merge

492cea4

Merge branch 'master' into JDK-8256011

3eeee3c

Call phantom LRB when ON_PHANTOM_OOP_REF is requested in C1 LRB stub

2934768

Call weak-LRB with ON_WEAK_OOP_REF, not ON_UNKNOWN_OOP_REF (cosmetic)

5dfd192

Avoid null-check and skip cset-check on non-strong refs in C1 LRB stub

83c07cb

Don't call compressed-oops LRB on IN_NATIVE from C1

51811e3

Remaining aarch64 changes

eef816c

openjdk bot removed ready rfr labels Nov 12, 2020

Whitespace fixes

0363359

openjdk bot added ready rfr labels Nov 12, 2020

Asserts against impossible combinations of weak/phantom vs in-native/…

c3b1b19

…in-heap

shipilev approved these changes Nov 13, 2020

View changes

openjdk bot closed this Nov 13, 2020

openjdk bot added integrated and removed ready rfr labels Nov 13, 2020

openjdk-notifier bot referenced this issue Nov 13, 2020

8256011: Shenandoah: Don't resurrect finalizably reachable objects

b0c28fa

Reviewed-by: shade, zgu

8256011: Shenandoah: Don't resurrect finalizably reachable objects #1109

8256011: Shenandoah: Don't resurrect finalizably reachable objects #1109

rkennke commented Nov 7, 2020 •

edited by openjdk[bot] bot

bridgekeeper bot commented Nov 7, 2020

openjdk bot commented Nov 7, 2020

mlbridge bot commented Nov 7, 2020 •

edited

shipilev left a comment

openjdk bot commented Nov 8, 2020 •

edited

zhengyu123 left a comment

shipilev left a comment

shipilev left a comment

shipilev left a comment

shipilev left a comment

rkennke commented Nov 13, 2020

openjdk bot commented Nov 13, 2020

8256011: Shenandoah: Don't resurrect finalizably reachable objects #1109

8256011: Shenandoah: Don't resurrect finalizably reachable objects #1109

Conversation

rkennke commented Nov 7, 2020 • edited by openjdk[bot] bot

Progress

Testing

Issue

Reviewers

Download

bridgekeeper bot commented Nov 7, 2020

openjdk bot commented Nov 7, 2020

mlbridge bot commented Nov 7, 2020 • edited

Webrevs

shipilev left a comment

openjdk bot commented Nov 8, 2020 • edited

zhengyu123 left a comment

shipilev left a comment

shipilev left a comment

shipilev left a comment

shipilev left a comment

rkennke commented Nov 13, 2020

openjdk bot commented Nov 13, 2020

rkennke commented Nov 7, 2020 •

edited by openjdk[bot] bot

mlbridge bot commented Nov 7, 2020 •

edited

openjdk bot commented Nov 8, 2020 •

edited