On Tue, 21 May 2024 13:49:18 GMT, jengebr <d...@openjdk.org> wrote: > Improve `java/lang/reflect/Method.java` by eliminating needless cloning of > Class[0] instances. This cloning is intended to prevent callers from > changing array contents, but smany Methods have zero exceptions or zero > parameters, and returning the original `Class[0]` is sufficient.
Please also enable GHA testing: https://github.com/jengebr/jdk/actions. You may need to trigger the test run manually after this, since the PR hook have already missed it. Go to "OpenJDK Sanity Checks" on the left on that page, then select your branch on in the drop-down on the right, and trigger the run. src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 249: > 247: * the original can safely be reused. > 248: */ > 249: public Class<?>[] copyClasses(Class<?>[] classes) { There is no need to make this utility method non-static. This would also obviate the need for having an instance of `ReflectionFactory` to access it. Additionally, at the risk of more bikeshedding, I think this utility method is better suited in `AccessibleObject` base class, with package-private visibility. Putting the util methods in `ReflectionFactory` erodes this comment a bit: <P> The methods in this class are extremely unsafe and can cause subversion of both the language and the verifier. For this reason, they are all instance methods, and access to the constructor of this factory is guarded by a security check, in similar style to {@link jdk.internal.misc.Unsafe}. </P> ------------- PR Comment: https://git.openjdk.org/jdk/pull/19327#issuecomment-2124338419 PR Review Comment: https://git.openjdk.org/jdk/pull/19327#discussion_r1609635793
