Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов wrote: >> Non-static classes hold a link to their parent classes, which in many cases >> can be avoided. > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8261880: Remove static from declarations of Holder nested classes Just for information there is similar issues in `javax.imageio.metadata.IIOMetadataFormatImpl` class in the `java.desktop` module. - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Thu, 20 May 2021 10:42:49 GMT, Claes Redestad wrote: >> Сергей Цыпанов has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8261880: Remove static from declarations of Holder nested classes > > Marked as reviewed by redestad (Reviewer). @cl4es now you can sponsor :) - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов wrote: >> Non-static classes hold a link to their parent classes, which in many cases >> can be avoided. > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8261880: Remove static from declarations of Holder nested classes Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов wrote: >> Non-static classes hold a link to their parent classes, which in many cases >> can be avoided. > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8261880: Remove static from declarations of Holder nested classes Any more comments? - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 24 Feb 2021 08:50:36 GMT, Alan Bateman wrote: >> Сергей Цыпанов has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8261880: Remove static from declarations of Holder nested classes > > src/java.base/windows/classes/sun/nio/ch/PipeImpl.java line 67: > >> 65: private final SinkChannel sink; >> 66: >> 67: private static class Initializer > > This one is okay to do. Thanks for review! Could you review the rest of the code and approve this PR, if it's fine? - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов
wrote:
>> Non-static classes hold a link to their parent classes, which in many cases
>> can be avoided.
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8261880: Remove static from declarations of Holder nested classes
src/java.base/windows/classes/sun/nio/ch/PipeImpl.java line 67:
> 65: private final SinkChannel sink;
> 66:
> 67: private static class Initializer
This one is okay to do.
src/java.base/share/classes/jdk/internal/module/ServicesCatalog.java line 51:
> 49: * Represents a service provider in the services catalog.
> 50: */
> 51: public static final class ServiceProvider {
This one is okay to do.
-
PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов wrote: >> Non-static classes hold a link to their parent classes, which in many cases >> can be avoided. > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8261880: Remove static from declarations of Holder nested classes The changes in java/net look ok to me. - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 17:24:50 GMT, Claes Redestad wrote: >> For static methods, since in java language you cannot declare static method >> in instance inner classes, I'd say making them static makes more sense >> language-wise. Also making them static reduces compiler synthetic instance >> field and constructors. > > Incidentally, Java-the-language allows static methods in inner instance > classes since JDK 16. And I'm not sure this was ever a restriction at the > JVMS level since we've been generating static methods (using ASM) into these > inner instance classes since at least JDK 9. Inner classes doesn't really exist for the JVM, it's just an attribute (in fact, a pair of attributes) that is read/write by javac (it's very similar to the way generics work). So it is Ok to have static methods in inner classes since Java 1.1 from the JVM POV with the caveat that you may not be all to call them from Java-the-language. Also since Java 11, inner classes are also nestmate and those attributes (NestHost/NestMembers) change the behavior of the VM. - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:35:02 GMT, liach wrote: >> I'll just revert them > > For static methods, since in java language you cannot declare static method > in instance inner classes, I'd say making them static makes more sense > language-wise. Also making them static reduces compiler synthetic instance > field and constructors. Incidentally, Java-the-language allows static methods in inner instance classes since JDK 16. And I'm not sure this was ever a restriction at the JVMS level since we've been generating static methods (using ASM) into these inner instance classes since at least JDK 9. - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
> Non-static classes hold a link to their parent classes, which in many cases > can be avoided. Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision: 8261880: Remove static from declarations of Holder nested classes - Changes: - all: https://git.openjdk.java.net/jdk/pull/2589/files - new: https://git.openjdk.java.net/jdk/pull/2589/files/5650cce4..c6f9cf6b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2589=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2589=00-01 Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2589.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2589/head:pull/2589 PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:32:39 GMT, Сергей Цыпанов
wrote:
>> src/java.base/share/classes/java/lang/invoke/DelegatingMethodHandle.java
>> line 192:
>>
>>> 190:
>>> 191: /* Placeholder class for DelegatingMethodHandles generated ahead
>>> of time */
>>> 192: static final class Holder {}
>>
>> For the four `Holder` classes in `java.lang.invoke`, the class is generated
>> when running jlink via `java.lang.invoke.GenerateJLIClassesHelper`. To stay
>> in sync with the definition you'd have to update that code. Also, since
>> these `Holder` classes will only contain static methods and are never
>> instantiated then I'm not sure it matters whether the classes are static or
>> not.
>
> I'll just revert them
For static methods, since in java language you cannot declare static method in
instance inner classes, I'd say making them static makes more sense
language-wise. Also making them static reduces compiler synthetic instance
field and constructors.
-
PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:24:46 GMT, Claes Redestad wrote:
>> Сергей Цыпанов has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8261880: Remove static from declarations of Holder nested classes
>
> src/java.base/share/classes/java/lang/invoke/DelegatingMethodHandle.java line
> 192:
>
>> 190:
>> 191: /* Placeholder class for DelegatingMethodHandles generated ahead of
>> time */
>> 192: static final class Holder {}
>
> For the four `Holder` classes in `java.lang.invoke`, the class is generated
> when running jlink via `java.lang.invoke.GenerateJLIClassesHelper`. To stay
> in sync with the definition you'd have to update that code. Also, since these
> `Holder` classes will only contain static methods and are never instantiated
> then I'm not sure it matters whether the classes are static or not.
I'll just revert them
-
PR: https://git.openjdk.java.net/jdk/pull/2589
