8386972: [lworld] Finalizer should not be registered for value objects#2589
8386972: [lworld] Finalizer should not be registered for value objects#2589fparain wants to merge 1 commit into
Conversation
|
👋 Welcome back fparain! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
| if (InstanceKlass::is_finalization_enabled() && | ||
| (m != nullptr) && !m->is_empty_method()) { | ||
| (m != nullptr) && !m->is_empty_method() && | ||
| m->method_holder()->access_flags().is_identity_class()) { |
There was a problem hiding this comment.
Could we call ClassFileParser's is_identity_class() here instead of going through the method_holder?
There was a problem hiding this comment.
Fred pointed out my proposed change actually changes the meaning as my proposal checks the current class, not the class that defines the finalize method. This is different for a finalize method from an abstract value class
There was a problem hiding this comment.
A finalizer in an abstract value class should probably still be registered for identity subclasses; as otherwise, changing an abstract identity class with a finalizer to an abstract value class with a finalizer would be a breaking change as long as finalizers continue to exist.
3 participants
Change in finalizers registration to prevent it for value classes.
Tested tier1-4.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2589/head:pull/2589$ git checkout pull/2589Update a local copy of the PR:
$ git checkout pull/2589$ git pull https://git.openjdk.org/valhalla.git pull/2589/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2589View PR using the GUI difftool:
$ git pr show -t 2589Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2589.diff