Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 12:20:20 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> performance improvements > > src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 76: > >> 74: >> 75: private static final MethodTypeDesc >> 76: MTD_boolean = MethodTypeDescImpl.ofValidated(CD_boolean, >> ConstantUtils.EMPTY_CLASSDESC), > > Maybe we can change the `MethodTypeDescImpl.ofValidated` to varargs so we > don't need explicit array initializations. Yes, I like that idea. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624576109
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 16:08:04 GMT, Adam Sotona wrote: >> src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 822: >> >>> 820:.iconst_0() // false >>> 821:.aload(0)// classLoader >>> 822:.invokestatic(CD_Class, "forName", >>> MTD_Class_String_boolean_ClassLoader); >> >> We can probably replace this `forName(name, false, thisClassLoader)` with >> loading a class constant to reduce load on symbols. > > I'm wondering why all the `Class.forName(... .getClassLoader())` > is necessary? > Simple `ldc ` seems to work well. Indeed, I initially added it because a JBS ticket recommended so. ldc instruction has the same behavior as such as forName call. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1616241702
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 12:24:31 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> performance improvements > > src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 822: > >> 820:.iconst_0() // false >> 821:.aload(0)// classLoader >> 822:.invokestatic(CD_Class, "forName", >> MTD_Class_String_boolean_ClassLoader); > > We can probably replace this `forName(name, false, thisClassLoader)` with > loading a class constant to reduce load on symbols. I'm wondering why all the `Class.forName(... .getClassLoader())` is necessary? Simple `ldc ` seems to work well. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1616232095
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 11:47:15 GMT, Adam Sotona wrote: >> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use >> classfile API for reflection proxy-generation. Actual implementation of >> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap >> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and >> each proxy class is transformed from the template. >> >> This patch is intended to examine plain proxy generation impact on >> performance and JDK bootstrap (vs proxy transformation from template). >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > performance improvements src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 76: > 74: > 75: private static final MethodTypeDesc > 76: MTD_boolean = MethodTypeDescImpl.ofValidated(CD_boolean, > ConstantUtils.EMPTY_CLASSDESC), Maybe we can change the `MethodTypeDescImpl.ofValidated` to varargs so we don't need explicit array initializations. src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 822: > 820:.iconst_0() // false > 821:.aload(0)// classLoader > 822:.invokestatic(CD_Class, "forName", > MTD_Class_String_boolean_ClassLoader); We can probably replace this `forName(name, false, thisClassLoader)` with loading a class constant to reduce load on symbols. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1615978269 PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1615982718
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: performance improvements - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/1d4edea5..29ca6a4e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=01-02 Stats: 77 lines in 2 files changed: 39 ins; 9 del; 29 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
