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: 8263561: Re-examine uses of LinkedList
On Fri, 26 Feb 2021 10:48:33 GMT, Сергей Цыпанов wrote: > The usage of `LinkedList` is senseless and can be replaced with either > `ArrayList` or `ArrayDeque` which are both more compact and effective. > > jdk:tier1 and jdk:tier2 are both ok Are linked lists worse for addition even in cases where addition to array list or deque requires resize and copying? i thought that's the advantage of linked list. - PR: https://git.openjdk.java.net/jdk/pull/2744
Re: RFR: 8263561: Re-examine uses of LinkedList
On Sun, 14 Mar 2021 14:58:11 GMT, Yi Yang wrote: >> The usage of `LinkedList` is senseless and can be replaced with either >> `ArrayList` or `ArrayDeque` which are both more compact and effective. >> >> jdk:tier1 and jdk:tier2 are both ok > > src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 220: > >> 218: return Collections.emptyList(); >> 219: } >> 220: List result = new ArrayList<>(); > > We'd better be cautious about this replacement since its > [caller](https://github.com/openjdk/jdk/blob/73029fe10a8a814a8c5f5221f2e667fd14a5b379/src/java.base/share/classes/java/net/URLClassLoader.java#L363) > will remove the first element of this array, that's one of the scenarios > where LinkedList usually has better performance than ArrayList. > > Just IMHO, I suggest replacing them only if there is a performance > improvement(e.g. benchmark reports). Changing field types will break users' > existing application code, they might reflectively modify these values. If that's the only use case, I recommend changing the return type to a deque, and replace the linked list with an array deque instead (as done elsewhere in this pr) - PR: https://git.openjdk.java.net/jdk/pull/2744
Re: RFR: 8263561: Re-examine uses of LinkedList
On Mon, 24 May 2021 07:13:29 GMT, Сергей Цыпанов wrote: >> Just for completeness, they're using the add-exports: >> https://github.com/AdoptOpenJDK/IcedTea-Web/blob/master/launchers/itw-modularjdk.args#L19 > > Should we then revert the changes to `JarIndex`? But don't people access these internal code at their own risk, as jdk may change these code at any time without notice? - PR: https://git.openjdk.java.net/jdk/pull/2744
Re: RFR: 8263561: Re-examine uses of LinkedList
On Mon, 24 May 2021 10:13:55 GMT, Сергей Цыпанов wrote: >> But don't people access these internal code at their own risk, as jdk may >> change these code at any time without notice? > > True, I just wonder whether it's OK to change internals when we know for sure > that this breaks 3rd party code I think so. There are always unexpected ways the jdk may break and third-party libraries would need a different workaround for a new java version. For instance, in Apache log4j, its library has a special guard against the broken implementation of Reflection getCallerClass during java 7. The libraries can just handle these in their version-specific components. Especially given the fact that the code linked above already has version-specific handling (8 vs 9), so it won't hurt much for them to add another piece of logic to handle jdk 17+ in case this optimization is merged. - PR: https://git.openjdk.java.net/jdk/pull/2744
