Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDK-8269651: ZGC: Optimize away gc locker in mark start #4638

Closed

Conversation

Hamlin-Li
Copy link

@Hamlin-Li Hamlin-Li commented Jun 30, 2021

Currently, ZGC involves gc locker in mark-start and relocate-start.
It seems gc locker in mark-start is not necessary anymore.
This is to remove this unnecessary(??) gc locker before further optimization is discussed.


Progress

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

Issue

  • JDK-8269651: ZGC: Optimize away gc locker in mark start ⚠️ Issue is not open.

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4638/head:pull/4638
$ git checkout pull/4638

Update a local copy of the PR:
$ git checkout pull/4638
$ git pull https://git.openjdk.java.net/jdk pull/4638/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 4638

View PR using the GUI difftool:
$ git pr show -t 4638

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4638.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Jun 30, 2021

👋 Welcome back mli! 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 openjdk bot added the rfr label Jun 30, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Jun 30, 2021

@Hamlin-Li The following label will be automatically applied to this pull request:

  • hotspot-gc

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

@openjdk openjdk bot added the hotspot-gc label Jun 30, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Jun 30, 2021

Webrevs

Copy link
Contributor

@pliden pliden left a comment

Sorry, but we can't do this. The GC locker needs to be honored any time the good/bad mask is changed, so both MarkStart and RelocateStart still needs it.

@Hamlin-Li
Copy link
Author

@Hamlin-Li Hamlin-Li commented Jul 1, 2021

Thanks for clarification.
When I invesitgated JDK-8269423, I also thought good/bad mask changing might be the reason to keep gc locker at MarkStart.

But after further investigation, it seems to me that for GetXxxCritical it's OK to remove gc locker at MarkStart.
The reasons are:

  1. GetXxxCritical returns the native address, and whether good or bad these addresses are valid address for native code (am I right here?), unless it's relocated later at Relocate.
  2. The gc locker in RelocateStart will block until ReleaseXxxCritical, so gc locker in RelocateStart ensures the native address retrieved from GetXxxCritical to be valid.

Please kindly correct my comments above.

@pliden
Copy link
Contributor

@pliden pliden commented Jul 1, 2021

GetXxxCritical returns the native address, and whether good or bad these addresses are valid address for native code (am I right here?), unless it's relocated later at Relocate.

We currently have the invariant that all accesses must go through the currently good heap view. While you can "cheat" and access objects through a bad heap view that will only work as long as you don't enable -XX:+ZVerifyViews, which will enforce this invariant. ZVerifyViews unmaps memory that isn't currently in the good heap view. It's a valuable debugging option to catch bad accesses, like when a load barrier is missing. ZVerifyViews is disabled by default because it's expensive.

@Hamlin-Li
Copy link
Author

@Hamlin-Li Hamlin-Li commented Jul 1, 2021

I see, Thanks for clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotspot-gc rfr
2 participants