Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v19]
On Wed, 5 Jun 2024 12:39:13 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> An assortment of potential improvements >> >> Co-authored-by: Claes Redestad > > src/java.base/share/classes/java/lang/constant/ConstantDescs.java line 356: > >> 354: ClassDesc[] fullParamTypes = new ClassDesc[paramTypes.length + >> prefixLen]; >> 355: System.arraycopy(INDY_BOOTSTRAP_ARGS, 0, fullParamTypes, 0, >> prefixLen); >> 356: System.arraycopy(paramTypes, 0, fullParamTypes, prefixLen, >> paramTypes.length); > > I'm thinking about creating a basic MethodTypeDesc like `INDY_BOOTSTRAP_TYPE > = MethodTypeDesc.ofValidated(CD_Object, INDY_BOOTSTRAP_ARGS)`, and we derive > bsm with > > INDY_BOOTSTRAP_TYPE > .insertParameterTypes(INDY_BOOTSTRAP_TYPE.parameterCount(), paramTypes) > .changeReturnType(returnType) > > which creates two MTD wrappers but they share the same parameter array That'd arguably be a bit cleaner. Might allocate an extra MTD (unless/until EA kicks in), but that might not matter really. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1630868455
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v19]
On Wed, 5 Jun 2024 12:00:25 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). >> >> The generated proxy is migrated from static initialization to CONDY >> bootstrap. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > An assortment of potential improvements > > Co-authored-by: Claes Redestad src/java.base/share/classes/java/lang/constant/ConstantDescs.java line 356: > 354: ClassDesc[] fullParamTypes = new ClassDesc[paramTypes.length + > prefixLen]; > 355: System.arraycopy(INDY_BOOTSTRAP_ARGS, 0, fullParamTypes, 0, > prefixLen); > 356: System.arraycopy(paramTypes, 0, fullParamTypes, prefixLen, > paramTypes.length); I'm thinking about creating a basic MethodTypeDesc like `INDY_BOOTSTRAP_TYPE = MethodTypeDesc.ofValidated(CD_Object, INDY_BOOTSTRAP_ARGS)`, and we derive bsm with INDY_BOOTSTRAP_TYPE .insertParameterTypes(INDY_BOOTSTRAP_TYPE.parameterCount(), paramTypes) .changeReturnType(returnType) which creates two MTD wrappers but they share the same parameter array test/micro/org/openjdk/bench/java/lang/reflect/ProxyGenBench.java line 2: > 1: /* > 2: * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights > reserved. Suggestion: * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1627683294 PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1627685503
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v19]
On Wed, 5 Jun 2024 12:00:25 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). >> >> The generated proxy is migrated from static initialization to CONDY >> bootstrap. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > An assortment of potential improvements > > Co-authored-by: Claes Redestad Looks good. Generation of proxy classes gets a nice boost this way. The condy bsm calls that may happen later takes a small hit which we can improve in a follow-up. - Marked as reviewed by redestad (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19410#pullrequestreview-2098990024
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v19]
On Wed, 5 Jun 2024 12:00:25 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). >> >> The generated proxy is migrated from static initialization to CONDY >> bootstrap. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > An assortment of potential improvements > > Co-authored-by: Claes Redestad Benchmark Mode Cnt Score Error Units ProxyGenBench.generate100Proxies(master)ss 10 15.295 ? 5.435 ms/op ProxyGenBench.generate100Proxies(#19410)ss 10 11.761 ? 3.323 ms/op Finished running test 'micro:.+ProxyGenBench.+' - PR Comment: https://git.openjdk.org/jdk/pull/19410#issuecomment-2149694078
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v19]
On Wed, 5 Jun 2024 12:00:25 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). >> >> The generated proxy is migrated from static initialization to CONDY >> bootstrap. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > An assortment of potential improvements > > Co-authored-by: Claes Redestad Name Cnt Base Error Test Error Unit Change Perfstartup-JMH 2040.000 ± 0.000 40.000 ± 0.000 ms/op 1.00x (p = 0.000 ) :.cycles 236365197.300 ± 3630668.331 224263724.700 ± 4160144.635 cycles 0.95x (p = 0.000*) :.instructions 602001346.250 ± 3134647.039 574894340.900 ± 2297441.310 instructions 0.95x (p = 0.000*) :.taskclock 60.000 ± 0.000 58.500 ± 3.181 ms 0.98x (p = 0.083 ) * = significant - PR Comment: https://git.openjdk.org/jdk/pull/19410#issuecomment-2149681885
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v19]
> [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). > > The generated proxy is migrated from static initialization to CONDY bootstrap. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: An assortment of potential improvements Co-authored-by: Claes Redestad - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/6ca164bc..54376fe8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=18 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=17-18 Stats: 25 lines in 5 files changed: 15 ins; 0 del; 10 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
