Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 22:50:49 GMT, David Holmes wrote: >> Regarding **jcmd** updates, I'm thinking maybe this would be better handled >> separately. There is the potential to update to `GC.finalizer_info` >> discussed previously. Looking at the **jcmd** tool docs, it seems like >> `GC.run_finalization` also ought to be updated. And maybe one or more of the >> other commands (maybe `VM.flags` or `VM.info`?) ought to list the >> finalization enabled or disabled status. And of course the tool's doc will >> need to be updated as well. > > @stuart-marks no issue with doing dcmd/jcmd changes separately, but I don't > think we need to go too far with this. I had considered `GC.run_finalization` > but it just says it calls `System.run_finalization` - so no change needed > there as it will be documented in System.runFinalization. And `VM.flags` only > reports `-XX` flag information. And `VM.info` doesn't seem appropriate for > mentioning this either. So no further changes needed to the other Dcmds IMO > and no need to update anything on the jcmd tool page either. @dholmes-ora OK if you're confident that it's sufficient just to add `GC.finalizer_info` and nothing else, and no docs or additional testing, then I'll just drop in the code from that branch you posted. Of course I'll do a full build & test. - PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 20:13:06 GMT, Stuart Marks wrote: >> Stuart Marks has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove Finalizer.Holder class. > > Regarding **jcmd** updates, I'm thinking maybe this would be better handled > separately. There is the potential to update to `GC.finalizer_info` discussed > previously. Looking at the **jcmd** tool docs, it seems like > `GC.run_finalization` also ought to be updated. And maybe one or more of the > other commands (maybe `VM.flags` or `VM.info`?) ought to list the > finalization enabled or disabled status. And of course the tool's doc will > need to be updated as well. @stuart-marks no issue with doing dcmd/jcmd changes separately, but I don't think we need to go too far with this. I had considered `GC.run_finalization` but it just says it calls `System.run_finalization` - so no change needed there as it will be documented in System.runFinalization. And `VM.flags` only reports `-XX` flag information. And `VM.info` doesn't seem appropriate for mentioning this either. So no further changes needed to the other Dcmds IMO and no need to update anything on the jcmd tool page either. - PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 00:14:18 GMT, Stuart Marks wrote: >> Pretty much what it says. The new option controls a static member in >> InstanceKlass that's consulted to determine whether the finalization >> machinery is activated for instances when a class is loaded. A new native >> method is added so that this state can be queried from Java. This is used to >> control whether a finalizer thread is created and to disable the `System` >> and `Runtime::runFinalization` methods. Includes tests for the above. >> >> Adding an option to disable finalization is part of [JEP >> 421](https://openjdk.java.net/jeps/421). > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Remove Finalizer.Holder class. Lib changes and tests look good - Marked as reviewed by bchristi (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 00:14:18 GMT, Stuart Marks wrote: >> Pretty much what it says. The new option controls a static member in >> InstanceKlass that's consulted to determine whether the finalization >> machinery is activated for instances when a class is loaded. A new native >> method is added so that this state can be queried from Java. This is used to >> control whether a finalizer thread is created and to disable the `System` >> and `Runtime::runFinalization` methods. Includes tests for the above. >> >> Adding an option to disable finalization is part of [JEP >> 421](https://openjdk.java.net/jeps/421). > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Remove Finalizer.Holder class. Regarding **jcmd** updates, I'm thinking maybe this would be better handled separately. There is the potential to update to `GC.finalizer_info` discussed previously. Looking at the **jcmd** tool docs, it seems like `GC.run_finalization` also ought to be updated. And maybe one or more of the other commands (maybe `VM.flags` or `VM.info`?) ought to list the finalization enabled or disabled status. And of course the tool's doc will need to be updated as well. - PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 00:14:18 GMT, Stuart Marks wrote: >> Pretty much what it says. The new option controls a static member in >> InstanceKlass that's consulted to determine whether the finalization >> machinery is activated for instances when a class is loaded. A new native >> method is added so that this state can be queried from Java. This is used to >> control whether a finalizer thread is created and to disable the `System` >> and `Runtime::runFinalization` methods. Includes tests for the above. >> >> Adding an option to disable finalization is part of [JEP >> 421](https://openjdk.java.net/jeps/421). > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Remove Finalizer.Holder class. Marked as reviewed by kbarrett (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 00:14:34 GMT, Stuart Marks wrote:
>> src/java.base/share/classes/java/lang/ref/Finalizer.java line 195:
>>
>>> 193:
>>> 194: static {
>>> 195: if (Holder.ENABLED) {
>>
>> Hello Stuart,
>> My understanding of the the lazy `Holder` is that it's there to delay the
>> static initialization of the code that's part of the `Holder`. In this case
>> here, the `Holder` is being used right within the `static` block of the
>> `Finalizer` class, that too as the first thing. In this case, is that
>> `Holder` class necessary?
>
> I pushed an update to remove the Holder class. It seems to continue to work
> fine. Thanks for pointing this out @jaikiran !
Thank you Stuart, this changed version looks fine to me.
-
PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 00:59:10 GMT, David Holmes wrote: >> Stuart Marks has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove Finalizer.Holder class. > > src/java.base/share/classes/java/lang/ref/Finalizer.java line 64: > >> 62: } >> 63: >> 64: static final boolean ENABLED = isFinalizationEnabled(); > > private? Yeah, probably should be private. Other stuff in this class is private except things that are used from outside. - PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 00:14:18 GMT, Stuart Marks wrote: >> Pretty much what it says. The new option controls a static member in >> InstanceKlass that's consulted to determine whether the finalization >> machinery is activated for instances when a class is loaded. A new native >> method is added so that this state can be queried from Java. This is used to >> control whether a finalizer thread is created and to disable the `System` >> and `Runtime::runFinalization` methods. Includes tests for the above. >> >> Adding an option to disable finalization is part of [JEP >> 421](https://openjdk.java.net/jeps/421). > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Remove Finalizer.Holder class. @stuart-marks : https://github.com/openjdk/jdk/pull/6469 (didn't intend to actually make a PR but clicked the wrong part of the button :) ) - PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Fri, 19 Nov 2021 00:14:18 GMT, Stuart Marks wrote: >> Pretty much what it says. The new option controls a static member in >> InstanceKlass that's consulted to determine whether the finalization >> machinery is activated for instances when a class is loaded. A new native >> method is added so that this state can be queried from Java. This is used to >> control whether a finalizer thread is created and to disable the `System` >> and `Runtime::runFinalization` methods. Includes tests for the above. >> >> Adding an option to disable finalization is part of [JEP >> 421](https://openjdk.java.net/jeps/421). > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Remove Finalizer.Holder class. Good simplification. src/java.base/share/classes/java/lang/ref/Finalizer.java line 64: > 62: } > 63: > 64: static final boolean ENABLED = isFinalizationEnabled(); private? - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
On Thu, 18 Nov 2021 04:13:21 GMT, Jaikiran Pai wrote:
>> Stuart Marks has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove Finalizer.Holder class.
>
> src/java.base/share/classes/java/lang/ref/Finalizer.java line 195:
>
>> 193:
>> 194: static {
>> 195: if (Holder.ENABLED) {
>
> Hello Stuart,
> My understanding of the the lazy `Holder` is that it's there to delay the
> static initialization of the code that's part of the `Holder`. In this case
> here, the `Holder` is being used right within the `static` block of the
> `Finalizer` class, that too as the first thing. In this case, is that
> `Holder` class necessary?
I pushed an update to remove the Holder class. It seems to continue to work
fine. Thanks for pointing this out @jaikiran !
-
PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization [v4]
> Pretty much what it says. The new option controls a static member in > InstanceKlass that's consulted to determine whether the finalization > machinery is activated for instances when a class is loaded. A new native > method is added so that this state can be queried from Java. This is used to > control whether a finalizer thread is created and to disable the `System` and > `Runtime::runFinalization` methods. Includes tests for the above. > > Adding an option to disable finalization is part of [JEP > 421](https://openjdk.java.net/jeps/421). Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: Remove Finalizer.Holder class. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6442/files - new: https://git.openjdk.java.net/jdk/pull/6442/files/5df8bf9f..e357eeec Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6442=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6442=02-03 Stats: 6 lines in 1 file changed: 0 ins; 2 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/6442.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6442/head:pull/6442 PR: https://git.openjdk.java.net/jdk/pull/6442
