Re: RFR: 8341548: More concise use of classfile API [v2]
> java.base should provide best practices for Class File API > > 1. Use fluent coding style > 2. Use aconst_null instead of oadConstant(null) > 3. use astore intead of 'storeLocal(REFERENCE' > 4. use aload instead of 'loadLocal(REFERENCE' > 5. 'lload/lstore' instead of 'loadLocal(LONG)/storeLocal(LONG)' Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: more fluent coding style - Changes: - all: https://git.openjdk.org/jdk/pull/21355/files - new: https://git.openjdk.org/jdk/pull/21355/files/8b2fa4d0..4d895f45 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21355&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21355&range=00-01 Stats: 21 lines in 2 files changed: 5 ins; 4 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/21355.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21355/head:pull/21355 PR: https://git.openjdk.org/jdk/pull/21355
Integrated: 8339699: Optimize DataOutputStream writeUTF
On Fri, 6 Sep 2024 09:58:58 GMT, Shaojin Wen wrote: > PR #20772 introduced an optimization for writeUTF, which can also be used in > DataOutputStream::writeUTF. This pull request has now been integrated. Changeset: b42fbf43 Author:Shaojin Wen URL: https://git.openjdk.org/jdk/commit/b42fbf43dfd62ae64973ff0e406b6609cd8e1aa6 Stats: 409 lines in 5 files changed: 227 ins; 157 del; 25 mod 8339699: Optimize DataOutputStream writeUTF Reviewed-by: liach, bpb - PR: https://git.openjdk.org/jdk/pull/20886
Re: RFR: 8341548: More concise use of classfile API
On Fri, 4 Oct 2024 12:05:02 GMT, Shaojin Wen wrote:
> java.base should provide best practices for Class File API
>
> 1. Use fluent coding style
> 2. Use aconst_null instead of oadConstant(null)
> 3. use astore intead of 'storeLocal(REFERENCE'
> 4. use aload instead of 'loadLocal(REFERENCE'
> 5. 'lload/lstore' instead of 'loadLocal(LONG)/storeLocal(LONG)'
Using a fluent coding style can reduce the code size, such as the following code
import java.lang.classfile.CodeBuilder;
public class Demo {
public static void f0(CodeBuilder cb) {
cb.aload(0);
cb.aload(1);
}
public static void f1(CodeBuilder cb) {
cb.aload(0)
.aload(1);
}
}
Compile Java code and view bytecode
javac --enable-preview --release 24 Demo.java
javap -c Demo
The bytecode is as follows
public class Demo {
public Demo();
Code:
0: aload_0
1: invokespecial #1 // Method
java/lang/Object."":()V
4: return
public static void f0(java.lang.classfile.CodeBuilder);
Code:
0: aload_0
1: iconst_0
2: invokeinterface #7, 2// InterfaceMethod
java/lang/classfile/CodeBuilder.aload:(I)Ljava/lang/classfile/CodeBuilder;
7: pop
8: aload_0
9: iconst_1
10: invokeinterface #7, 2// InterfaceMethod
java/lang/classfile/CodeBuilder.aload:(I)Ljava/lang/classfile/CodeBuilder;
15: pop
16: return
public static void f1(java.lang.classfile.CodeBuilder);
Code:
0: aload_0
1: iconst_0
2: invokeinterface #7, 2// InterfaceMethod
java/lang/classfile/CodeBuilder.aload:(I)Ljava/lang/classfile/CodeBuilder;
7: iconst_1
8: invokeinterface #7, 2// InterfaceMethod
java/lang/classfile/CodeBuilder.aload:(I)Ljava/lang/classfile/CodeBuilder;
13: pop
14: return
}
We can see that method `f0` does not use the fluent code style, and the
generated bytecode has redundant pop and aload_0 operations.
-
PR Comment: https://git.openjdk.org/jdk/pull/21355#issuecomment-2393963891
RFR: 8341548: More concise use of classfile API
java.base should provide best practices for Class File API 1. Use fluent coding style 2. Use aconst_null instead of oadConstant(null) 3. use astore intead of 'storeLocal(REFERENCE' 4. use aload instead of 'loadLocal(REFERENCE' 5. 'lload/lstore' instead of 'loadLocal(LONG)/storeLocal(LONG)' - Commit messages: - Update src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java - remove empty line - 'lload/lstore' instead of 'loadLocal(LONG)/storeLocal(LONG)'' - MethodHandleProxies More concise use of classfile API - InnerClassLambdaMetafactory More concise use of classfile API - BindingSpecializer More concise use of classfile API - SwitchBootstraps more concise use of classfile API - More concise use of classfile API Changes: https://git.openjdk.org/jdk/pull/21355/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21355&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8341548 Stats: 326 lines in 5 files changed: 1 ins; 21 del; 304 mod Patch: https://git.openjdk.org/jdk/pull/21355.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21355/head:pull/21355 PR: https://git.openjdk.org/jdk/pull/21355
Re: RFR: 8339320: Optimize ClassFile Utf8EntryImpl#inflate [v4]
> A very small optimization, split the large method inflate, split the > infrequently used paths into a method inflateCHAR Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java Co-authored-by: Chen Liang - Changes: - all: https://git.openjdk.org/jdk/pull/20767/files - new: https://git.openjdk.org/jdk/pull/20767/files/8b3c67da..f8cdfa77 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20767&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20767&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20767.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20767/head:pull/20767 PR: https://git.openjdk.org/jdk/pull/20767
Re: RFR: 8341470: BigDecimal.stripTrailingZeros() optimization [v18]
On Fri, 4 Oct 2024 14:12:16 GMT, fabioromano1 wrote:
>> An optimized algorithm for `BigDecimal.stripTrailingZeros()` that uses
>> repeated squares trick.
>
> fabioromano1 has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Merge branch 'patchStripTrailingZeros' of
> https://github.com/fabioromano1/jdk into patchStripTrailingZeros
> - Added benchmark tests
Expand the length of FIVE_TO_2_TO to 32 and use `& 0x1F` when accessing the
array to eliminate bounds checks.
private static final BigInteger[] FIVE_TO_2_TO = new BigInteger[32];
private static BigInteger fiveToTwoToThe(int n) {
if (n >= FIVE_TO_2_TO_LEN) {
BigInteger pow = FIVE_TO_2_TO[(FIVE_TO_2_TO_LEN - 1) & 0x1F];
for (int i = FIVE_TO_2_TO_LEN; i <= n; i++)
FIVE_TO_2_TO[i & 0x1F] = pow = pow.multiply(pow);
FIVE_TO_2_TO_LEN = n + 1;
}
return FIVE_TO_2_TO[n & 0x1F];
}
-
PR Comment: https://git.openjdk.org/jdk/pull/21323#issuecomment-2394034730
Re: RFR: 8339320: Optimize ClassFile Utf8EntryImpl#inflate [v3]
On Wed, 25 Sep 2024 01:10:04 GMT, Shaojin Wen wrote: >> A very small optimization, split the large method inflate, split the >> infrequently used paths into a method inflateCHAR > > Shaojin Wen has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - inflateNonAscii > - Merge remote-tracking branch 'upstream/master' into > optim_class_file_pool_inflat_202408 > ># Conflicts: ># > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java > - Merge remote-tracking branch 'origin/optim_class_file_pool_inflat_202408' > into optim_class_file_pool_inflat_202408 > - Update > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java > >Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> > - Update > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java > >Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> > - optimize Utf8EntryImpl inflate > - Merge remote-tracking branch 'upstream/master' into > optim_class_file_pool_inflat_202408 > ># Conflicts: ># > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java > - Suggestions from @liach > - fix build error > - optimize Utf8EntryImpl inflate javap -c -p -v jdk.internal.classfile.impl.AbstractPoolEntry.Utf8EntryImpl >From the output of javap, we can see that the code size of inflateNonAscii is >408 - PR Comment: https://git.openjdk.org/jdk/pull/20767#issuecomment-2394008065
Re: RFR: 8341512: Optimize StackMapGenerator::processInvokeInstructions [v2]
> A small optimization for StackMapGenerator::processInvokeInstructions. > > 1. Use local currentFrame to avoid multiple getfields > 2. remove Util.methodTypeSymbol(NameAndTypeEntry) > 3. Use decStack instead of popStack to reduce array access in popStack > 4. codeSize reduced from 277 to 262 Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/lang/classfile/attribute/EnclosingMethodAttribute.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/21344/files - new: https://git.openjdk.org/jdk/pull/21344/files/ebf280ec..eea92640 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21344&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21344&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/21344.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21344/head:pull/21344 PR: https://git.openjdk.org/jdk/pull/21344
Re: RFR: 8339205: Optimize StackMapGenerator [v2]
> Reduce code size by combining calls and defining local variables Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits: - reduce codeSize - reduce codeSize - decStack & stackUnderflow - fix merge error - Merge branch 'master' into optim_stack_map_gen_202408 - Merge remote-tracking branch 'upstream/master' into optim_stack_map_gen_202408 # Conflicts: # src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java - Merge remote-tracking branch 'upstream/master' into optim_stack_map_gen_202408 # Conflicts: # src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java - fix merge error - Merge remote-tracking branch 'origin/optim_stack_map_gen_202408' into optim_stack_map_gen_202408 - optimize for codeSize - ... and 3 more: https://git.openjdk.org/jdk/compare/3f420fac...b647a4b5 - Changes: https://git.openjdk.org/jdk/pull/20756/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20756&range=01 Stats: 363 lines in 2 files changed: 225 ins; 39 del; 99 mod Patch: https://git.openjdk.org/jdk/pull/20756.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20756/head:pull/20756 PR: https://git.openjdk.org/jdk/pull/20756
Re: RFR: 8341510: Optimize StackMapGenerator::processFieldInstructions
On Fri, 4 Oct 2024 04:59:04 GMT, Chen Liang wrote:
>> A small optimization to reduce CodeSize, codeSize reduced from 164 to 140.
>>
>> 1. Use local currentFrame to avoid multiple getfields
>> 2. Use decStack instead of popStack to reduce array access in popStack
>> 3. Call Util.fieldTypeSymbol to pass in type instead of nameAndType
>
> src/java.base/share/classes/jdk/internal/classfile/impl/Util.java line 234:
>
>> 232: }
>> 233:
>> 234: public static MethodTypeDesc methodTypeSymbol(NameAndTypeEntry nat)
>> {
>
> Can you remove this method too, since we have a Utf8Entry version?
`methodTypeSymbol(NameAndTypeEntry nat) ` This method will be removed in PR
#21344
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21345#discussion_r1787255216
RFR: 8341512: Optimize StackMapGenerator::processInvokeInstructions
A small optimization for StackMapGenerator::processInvokeInstructions. 1. Use local currentFrame to avoid multiple getfields 2. remove Util.methodTypeSymbol(NameAndTypeEntry) 3. Use decStack instead of popStack to reduce array access in popStack 4. codeSize reduced from 277 to 262 - Commit messages: - use decStack instead of popStack - remove Util.methodTypeSymbol(NameAndTypeEntry) - optimize processInvokeInstructions Changes: https://git.openjdk.org/jdk/pull/21344/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21344&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8341512 Stats: 20 lines in 10 files changed: 1 ins; 7 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/21344.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21344/head:pull/21344 PR: https://git.openjdk.org/jdk/pull/21344
RFR: 8341510: Optimize StackMapGenerator::processFieldInstructions
A small optimization to reduce CodeSize, codeSize reduced from 164 to 140. 1. Use local currentFrame to avoid multiple getfields 2. Use decStack instead of popStack to reduce array access in popStack 3. Call Util.fieldTypeSymbol to pass in type instead of nameAndType - Commit messages: - optimize processFieldInstructions Changes: https://git.openjdk.org/jdk/pull/21345/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21345&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8341510 Stats: 14 lines in 4 files changed: 1 ins; 7 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/21345.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21345/head:pull/21345 PR: https://git.openjdk.org/jdk/pull/21345
Re: RFR: 8341141: Optimize DirectCodeBuilder [v20]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/jdk/internal/classfile/impl/AttributeHolder.java Co-authored-by: Chen Liang - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/8b092270..356761ab Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=19 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=18-19 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Integrated: 8341006: Optimize StackMapGenerator detect frames
On Wed, 25 Sep 2024 12:05:02 GMT, Shaojin Wen wrote: > 1. Construct Frames directly without BitSet > 2. Use Frame[] instead of ArrayList > 3. Use EMPTY_FRAME_ARRAY for initialization. No need to allocate objects when > there is no frame. This pull request has now been integrated. Changeset: 12028000 Author: Shaojin Wen URL: https://git.openjdk.org/jdk/commit/12028000db2ef3b1c784af119c495aa3ef9590cf Stats: 128 lines in 1 file changed: 60 ins; 37 del; 31 mod 8341006: Optimize StackMapGenerator detect frames Reviewed-by: liach - PR: https://git.openjdk.org/jdk/pull/21183
Re: RFR: 8341006: Optimize StackMapGenerator detect frames [v3]
> 1. Construct Frames directly without BitSet > 2. Use Frame[] instead of ArrayList > 3. Use EMPTY_FRAME_ARRAY for initialization. No need to allocate objects when > there is no frame. Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: reduce codeSize - Changes: - all: https://git.openjdk.org/jdk/pull/21183/files - new: https://git.openjdk.org/jdk/pull/21183/files/905acdda..aa390010 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21183&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21183&range=01-02 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/21183.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21183/head:pull/21183 PR: https://git.openjdk.org/jdk/pull/21183
Integrated: 8341415: Optimize RawBytecodeHelper::next
On Wed, 2 Oct 2024 07:53:44 GMT, Shaojin Wen wrote: > A small optimization to the RawBytecodeHelper::next method > * Remove `len <= 0` once > * merge store opcode and isWide This pull request has now been integrated. Changeset: d7f32d89 Author: Shaojin Wen URL: https://git.openjdk.org/jdk/commit/d7f32d891cde2278fe7158fb28d29235db5c818c Stats: 40 lines in 1 file changed: 9 ins; 14 del; 17 mod 8341415: Optimize RawBytecodeHelper::next Reviewed-by: liach - PR: https://git.openjdk.org/jdk/pull/21300
Re: RFR: 8341415: Optimize RawBytecodeHelper::next
On Wed, 2 Oct 2024 13:28:24 GMT, Shaojin Wen wrote:
>> src/java.base/share/classes/jdk/internal/classfile/impl/RawBytecodeHelper.java
>> line 350:
>>
>>> 348: */
>>> 349: public boolean isWide() {
>>> 350: return (opcode & (WIDE << 8)) != 0;
>>
>> Suggestion:
>>
>> return (opcode >>> 8) == WIDE;
>
> WIDE is a constant, WIDE << 8 will do constant folding, your suggestion will
> be longer in bytecode.
public isWide()Z
L0
LINENUMBER 8 L0
ALOAD 0
GETFIELD internal/classfile/impl/RawBytecodeHelper.opcode : I
LDC 50176
IAND
IFEQ L1
ICONST_1
GOTO L2
L1
FRAME SAME
ICONST_0
L2
FRAME SAME1 I
IRETURN
L3
LOCALVARIABLE this Linternal/classfile/impl/RawBytecodeHelper; L0 L3 0
MAXSTACK = 2
MAXLOCALS = 1
}
public isWide1()Z
L0
LINENUMBER 12 L0
ALOAD 0
GETFIELD internal/classfile/impl/RawBytecodeHelper.opcode : I
BIPUSH 8
IUSHR
SIPUSH 196
IF_ICMPNE L1
ICONST_1
GOTO L2
L1
FRAME SAME
ICONST_0
L2
FRAME SAME1 I
IRETURN
L3
LOCALVARIABLE this Linternal/classfile/impl/RawBytecodeHelper; L0 L3 0
MAXSTACK = 2
MAXLOCALS = 1
}
The above are the bytecodes of the two implementations. It can be seen that the
original implementation is more concise.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21300#discussion_r1784575918
Re: RFR: 8341415: Optimize RawBytecodeHelper::next
On Wed, 2 Oct 2024 11:09:52 GMT, ExE Boss wrote:
>> A small optimization to the RawBytecodeHelper::next method
>> * Remove `len <= 0` once
>> * merge store opcode and isWide
>
> src/java.base/share/classes/jdk/internal/classfile/impl/RawBytecodeHelper.java
> line 350:
>
>> 348: */
>> 349: public boolean isWide() {
>> 350: return (opcode & (WIDE << 8)) != 0;
>
> Suggestion:
>
> return (opcode >>> 8) == WIDE;
WIDE is a constant, WIDE << 8 will do constant folding, your suggestion will be
longer in bytecode.
> src/java.base/share/classes/jdk/internal/classfile/impl/RawBytecodeHelper.java
> line 449:
>
>> 447: }
>> 448:
>> 449: if ((nextBci += len) > end) {
>
> This change makes it that `nextBci` will no longer monotonically increase in
> case of a malformed special instruction.
>
> Suggestion:
>
> if (len <= 0 || (nextBci += len) > end) {
The checkSpecialInstruction has already added the 'len <= 0' process, so there
is no need to check 'len <= '0' again.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21300#discussion_r1784531869
PR Review Comment: https://git.openjdk.org/jdk/pull/21300#discussion_r1784533279
RFR: 8341415: Optimize RawBytecodeHelper::next
A small optimization to the RawBytecodeHelper::next method * Remove `len <= 0` once * merge store opcode and isWide - Commit messages: - merge store opcode & isWide - optimize RawBytecodeHelper::next Changes: https://git.openjdk.org/jdk/pull/21300/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21300&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8341415 Stats: 40 lines in 1 file changed: 9 ins; 14 del; 17 mod Patch: https://git.openjdk.org/jdk/pull/21300.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21300/head:pull/21300 PR: https://git.openjdk.org/jdk/pull/21300
Re: RFR: 8341141: Optimize DirectCodeBuilder [v19]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: fix merge error - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/6dfa07ed..8b092270 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=18 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=17-18 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v18]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 77 commits: - Merge branch 'master' into optim_direct_code_builder_202409 - optimize DirectClassBuilder::build - writeU2U2U2 - optimization for powerOctal - label write clean - optimizer for codeSize - optimize MethodTypeDescImpl::descriptorString - Remove redundant requireNonNull - optimize buildContent - optimize setLabelTarget - ... and 67 more: https://git.openjdk.org/jdk/compare/39c17b39...6dfa07ed - Changes: https://git.openjdk.org/jdk/pull/21243/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=17 Stats: 967 lines in 16 files changed: 585 ins; 119 del; 263 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v17]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with two additional commits since the last revision: - optimize DirectClassBuilder::build - writeU2U2U2 - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/e7b32d43..7029dfe7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=16 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=15-16 Stats: 59 lines in 5 files changed: 37 ins; 8 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v16]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: optimization for powerOctal - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/36c81ec1..e7b32d43 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=15 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=14-15 Stats: 3 lines in 2 files changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v15]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with two additional commits since the last revision: - label write clean - optimizer for codeSize - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/77174a2b..36c81ec1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=14 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=13-14 Stats: 138 lines in 3 files changed: 66 ins; 25 del; 47 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v14]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with two additional commits since the last revision: - optimize MethodTypeDescImpl::descriptorString - Remove redundant requireNonNull - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/4e1e5daf..77174a2b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=13 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=12-13 Stats: 30 lines in 3 files changed: 17 ins; 5 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v13]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: optimize buildContent - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/d1059c58..4e1e5daf Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=12 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=11-12 Stats: 13 lines in 1 file changed: 3 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Integrated: 8341199: Use ClassFile's new API loadConstant(int)
On Mon, 30 Sep 2024 10:03:57 GMT, Shaojin Wen wrote: > The new API loadConstant(int) can shorten the calling path and can replace > ldc when the parameter is of int/Integer type. This pull request has now been integrated. Changeset: f1bf469b Author:Shaojin Wen URL: https://git.openjdk.org/jdk/commit/f1bf469b4ee07b48b629a126111e307d3cab7fd7 Stats: 9 lines in 4 files changed: 0 ins; 0 del; 9 mod 8341199: Use ClassFile's new API loadConstant(int) Reviewed-by: liach, redestad - PR: https://git.openjdk.org/jdk/pull/21259
Re: RFR: 8341141: Optimize DirectCodeBuilder [v12]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with four additional commits since the last revision: - optimize setLabelTarget - optimize checkType - optimize labelBinding - optimize AttributeHolder::withAttribute - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/8f678b3f..d1059c58 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=11 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=10-11 Stats: 45 lines in 3 files changed: 18 ins; 17 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v10]
On Mon, 30 Sep 2024 06:48:07 GMT, Shaojin Wen wrote: >> Some DirectCodeBuilder related optimizations to improve startup and running >> performance: >> 1. Merge calls, merge writeU1 and writeU2 into writeU3 >> 2. Merge calls, merge writeU1 and writeIndex operations >> 3. Directly use writeU1 instead of writeBytecode >> 4. Rewrite the implementation of load and store > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > optimize writeBranch The use of Lambda in the remove method in AttributeHolder was incorrect, and I fixed it by using an array instead of List for the attributes field. - PR Comment: https://git.openjdk.org/jdk/pull/21243#issuecomment-2383098204
Re: RFR: 8341141: Optimize DirectCodeBuilder [v11]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: use array instead of ArrayList - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/3396e379..8f678b3f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=10 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=09-10 Stats: 36 lines in 2 files changed: 24 ins; 1 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341199: Use ClassFile's new API loadConstant(int)
On Mon, 30 Sep 2024 10:03:57 GMT, Shaojin Wen wrote: > The new API loadConstant(int) can shorten the calling path and can replace > ldc when the parameter is of int/Integer type. I think loadConstant should be renamed to ldc - PR Comment: https://git.openjdk.org/jdk/pull/21259#issuecomment-2383074136
RFR: 8341199: Use ClassFile's new API loadConstant(int)
The new API loadConstant(int) can shorten the calling path and can replace ldc when the parameter is of int/Integer type. - Commit messages: - use loadConstant instead of ldc Changes: https://git.openjdk.org/jdk/pull/21259/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21259&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8341199 Stats: 9 lines in 4 files changed: 0 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/21259.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21259/head:pull/21259 PR: https://git.openjdk.org/jdk/pull/21259
Re: RFR: 8341141: Optimize DirectCodeBuilder [v10]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: optimize writeBranch - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/0b52b641..3396e379 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=09 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=08-09 Stats: 25 lines in 1 file changed: 3 ins; 3 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v9]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: optimize writeBranch - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/7b840194..0b52b641 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=08 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=07-08 Stats: 156 lines in 2 files changed: 133 ins; 13 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v8]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: remove jdkTreePrimitive & use new api - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/b69800dc..7b840194 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=07 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=06-07 Stats: 51 lines in 1 file changed: 0 ins; 48 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v7]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 60 additional commits since the last revision: - Merge branch 'master' into optim_direct_code_builder_202409 - optimize writeBranch - Local var clean - 8340621: Open source several AWT List tests Reviewed-by: prr - 8340639: Open source few more AWT List tests Reviewed-by: prr - 8340560: Open Source several AWT/2D font and rendering tests Reviewed-by: kizune - 8340721: Clarify special case handling of unboxedType and getWildcardType Reviewed-by: prappo, mcimadamore - 8341101: [ARM32] Error: ShouldNotReachHere() in TemplateInterpreterGenerator::generate_math_entry after 8338694 Reviewed-by: shade - 8340404: CharsetProvider specification updates Reviewed-by: alanb, naoto - 8337679: Memset warning in src/hotspot/share/adlc/adlArena.cpp Reviewed-by: stefank, thartmann, jwaters - ... and 50 more: https://git.openjdk.org/jdk/compare/8ac2b68e...b69800dc - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/d6c34593..b69800dc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=05-06 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v6]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with 53 additional commits since the last revision: - optimize writeBranch - Local var clean - 8340621: Open source several AWT List tests Reviewed-by: prr - 8340639: Open source few more AWT List tests Reviewed-by: prr - 8340560: Open Source several AWT/2D font and rendering tests Reviewed-by: kizune - 8340721: Clarify special case handling of unboxedType and getWildcardType Reviewed-by: prappo, mcimadamore - 8341101: [ARM32] Error: ShouldNotReachHere() in TemplateInterpreterGenerator::generate_math_entry after 8338694 Reviewed-by: shade - 8340404: CharsetProvider specification updates Reviewed-by: alanb, naoto - 8337679: Memset warning in src/hotspot/share/adlc/adlArena.cpp Reviewed-by: stefank, thartmann, jwaters - 8341059: Change Entrust TLS distrust date to November 12, 2024 Reviewed-by: mullan - ... and 43 more: https://git.openjdk.org/jdk/compare/2ca81a37...d6c34593 - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/2ca81a37..d6c34593 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=04-05 Stats: 7940 lines in 183 files changed: 4934 ins; 1662 del; 1344 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v5]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: suggestion from @cl4es and @liach - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/dab40af9..2ca81a37 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=03-04 Stats: 24 lines in 3 files changed: 0 ins; 0 del; 24 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v4]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: use array instead of ArrayList - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/9e2e4db4..dab40af9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=02-03 Stats: 27 lines in 1 file changed: 10 ins; 5 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v3]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: bug fix - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/3cb16578..9e2e4db4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=01-02 Stats: 10 lines in 1 file changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341141: Optimize DirectCodeBuilder [v2]
> Some DirectCodeBuilder related optimizations to improve startup and running > performance: > 1. Merge calls, merge writeU1 and writeU2 into writeU3 > 2. Merge calls, merge writeU1 and writeIndex operations > 3. Directly use writeU1 instead of writeBytecode > 4. Rewrite the implementation of load and store Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: use array instead of ArrayList - Changes: - all: https://git.openjdk.org/jdk/pull/21243/files - new: https://git.openjdk.org/jdk/pull/21243/files/2c3d4f12..3cb16578 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=00-01 Stats: 44 lines in 1 file changed: 25 ins; 0 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
Re: RFR: 8341136: Optimize StackMapGenerator::trimAndCompress
On Sat, 28 Sep 2024 17:15:43 GMT, Abdelhak Zaaim wrote:
> We can improve performance by avoiding repeated array access in the loop.
> Instead of accessing types[i] multiple times, we cache it in a local
> variable. Here's the optimized version `for (int i = 0; i < count; i++) {
> Type current = types[i]; if (!current.isCategory2_2nd()) { if (compressed !=
> i) { types[compressed] = current; } compressed++; } } ` This reduces overhead
> from multiple array lookups.
This will add one more store and one more load operation in the loop when there
is no isCategory2_2nd. The classfile is optimized to be faster at startup and
work better in interpreted mode or C1.
-
PR Comment: https://git.openjdk.org/jdk/pull/21227#issuecomment-2381076305
Re: RFR: 8341136: Optimize StackMapGenerator::trimAndCompress
On Fri, 27 Sep 2024 17:05:25 GMT, Shaojin Wen wrote: > A small optimization to reduce the write operations of trimAndCompress I saw trimAndCompress in the flame graph of the profile, so I made this optimization. Avoiding write operations here is a very small optimization and will not have a significant improvement. I don’t have Benchmark performance numbers. - PR Comment: https://git.openjdk.org/jdk/pull/21227#issuecomment-2381072287
RFR: 8341141: Optimize DirectCodeBuilder
Some DirectCodeBuilder related optimizations to improve startup and running performance: 1. Merge calls, merge writeU1 and writeU2 into writeU3 2. Merge calls, merge writeU1 and writeIndex operations 3. Directly use writeU1 instead of writeBytecode 4. Rewrite the implementation of load and store - Commit messages: - remove DirectCodeBuilder::writeLocalVar - optimize DirectCodeBuilder Changes: https://git.openjdk.org/jdk/pull/21243/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21243&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8341141 Stats: 489 lines in 6 files changed: 297 ins; 35 del; 157 mod Patch: https://git.openjdk.org/jdk/pull/21243.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21243/head:pull/21243 PR: https://git.openjdk.org/jdk/pull/21243
RFR: 8341136: Optimize StackMapGenerator::trimAndCompress
A small optimization to reduce the write operations of trimAndCompress - Commit messages: - optimize trimAndCompress Changes: https://git.openjdk.org/jdk/pull/21227/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21227&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8341136 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/21227.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21227/head:pull/21227 PR: https://git.openjdk.org/jdk/pull/21227
Re: RFR: 8339320: Optimize ClassFile Utf8EntryImpl#inflate [v3]
On Wed, 25 Sep 2024 01:10:04 GMT, Shaojin Wen wrote: >> A very small optimization, split the large method inflate, split the >> infrequently used paths into a method inflateCHAR > > Shaojin Wen has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - inflateNonAscii > - Merge remote-tracking branch 'upstream/master' into > optim_class_file_pool_inflat_202408 > ># Conflicts: ># > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java > - Merge remote-tracking branch 'origin/optim_class_file_pool_inflat_202408' > into optim_class_file_pool_inflat_202408 > - Update > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java > >Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> > - Update > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java > >Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> > - optimize Utf8EntryImpl inflate > - Merge remote-tracking branch 'upstream/master' into > optim_class_file_pool_inflat_202408 > ># Conflicts: ># > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java > - Suggestions from @liach > - fix build error > - optimize Utf8EntryImpl inflate I don't have a test scenario for inflateNonAscii, and I can't see the codeSize of inflateNonAscii printed in the compiler log. - PR Comment: https://git.openjdk.org/jdk/pull/20767#issuecomment-2378238661
Re: RFR: 8341006: Optimize StackMapGenerator detect frames [v2]
> 1. Construct Frames directly without BitSet > 2. Use Frame[] instead of ArrayList > 3. Use EMPTY_FRAME_ARRAY for initialization. No need to allocate objects when > there is no frame. Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: copyright - Changes: - all: https://git.openjdk.org/jdk/pull/21183/files - new: https://git.openjdk.org/jdk/pull/21183/files/0ea67959..905acdda Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21183&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21183&range=00-01 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/21183.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21183/head:pull/21183 PR: https://git.openjdk.org/jdk/pull/21183
RFR: 8341006: Optimize StackMapGenerator detect frames
1. Construct Frames directly without BitSet 2. Use Frame[] instead of ArrayList 3. Use EMPTY_FRAME_ARRAY for initialization. No need to allocate objects when there is no frame. - Commit messages: - bug fix - Use Frame[] instead of List - frame out of BytecodeRange - optimize StackMapGenerator::generate Changes: https://git.openjdk.org/jdk/pull/21183/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21183&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8341006 Stats: 127 lines in 1 file changed: 59 ins; 37 del; 31 mod Patch: https://git.openjdk.org/jdk/pull/21183.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21183/head:pull/21183 PR: https://git.openjdk.org/jdk/pull/21183
Integrated: 8340710: Optimize DirectClassBuilder::build
On Sun, 22 Sep 2024 05:30:43 GMT, Shaojin Wen wrote: > Do some refactoring so that the code can be inlined by the C1/C2 optimizer. > > 1. DirectClassBuilder::build codeSize 361 -> 319 > 2. DirectCodeBuilder::writeExceptionHandlers codeSize 183 -> 31 > 3. BufWriterImpl::writeIndex codeSize 62 -> 37 (forceinline) > 4. BufWriterImpl::writeU2 (forceinline) > 5. Util::writeAttributes codSize 45 -> 40 (forceinline) > 6. Util::writeList codSize 47 -> 42 (forceinline) This pull request has now been integrated. Changeset: 2e0554a6 Author:Shaojin Wen URL: https://git.openjdk.org/jdk/commit/2e0554a69548dae6e8ce9eec48c82e08dd3c1ffa Stats: 50 lines in 4 files changed: 31 ins; 4 del; 15 mod 8340710: Optimize DirectClassBuilder::build Reviewed-by: liach - PR: https://git.openjdk.org/jdk/pull/21118
Integrated: 8340587: Optimize StackMapGenerator$Frame::checkAssignableTo
On Sun, 22 Sep 2024 13:20:01 GMT, Shaojin Wen wrote: > Optimize checkAssignableTo to avoid clone when stackSize is 0, and use clone > instead of Array.copyOf to avoid compression and then expansion This pull request has now been integrated. Changeset: 2d38af61 Author:Shaojin Wen URL: https://git.openjdk.org/jdk/commit/2d38af61e4133ca98d5a98b3cfb6a6dde2877026 Stats: 7 lines in 1 file changed: 4 ins; 0 del; 3 mod 8340587: Optimize StackMapGenerator$Frame::checkAssignableTo Reviewed-by: liach - PR: https://git.openjdk.org/jdk/pull/21121
Integrated: 8340708: Optimize StackMapGenerator::processMethod
On Mon, 23 Sep 2024 13:53:13 GMT, Shaojin Wen wrote: > A small optimization of processMethod, by using local variables and > extracting exception methods, reduces codeSize from 326 to 283 This pull request has now been integrated. Changeset: 9bcc7b66 Author:Shaojin Wen URL: https://git.openjdk.org/jdk/commit/9bcc7b66de6495d3da8fc7f30a2a88187dbe847d Stats: 6 lines in 1 file changed: 3 ins; 1 del; 2 mod 8340708: Optimize StackMapGenerator::processMethod Reviewed-by: liach - PR: https://git.openjdk.org/jdk/pull/21137
Re: RFR: 8339320: Optimize ClassFile Utf8EntryImpl#inflate [v3]
> A very small optimization, split the large method inflate, split the > infrequently used paths into a method inflateCHAR Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits: - inflateNonAscii - Merge remote-tracking branch 'upstream/master' into optim_class_file_pool_inflat_202408 # Conflicts: # src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java - Merge remote-tracking branch 'origin/optim_class_file_pool_inflat_202408' into optim_class_file_pool_inflat_202408 - Update src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Update src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - optimize Utf8EntryImpl inflate - Merge remote-tracking branch 'upstream/master' into optim_class_file_pool_inflat_202408 # Conflicts: # src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java - Suggestions from @liach - fix build error - optimize Utf8EntryImpl inflate - Changes: https://git.openjdk.org/jdk/pull/20767/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20767&range=02 Stats: 75 lines in 1 file changed: 22 ins; 19 del; 34 mod Patch: https://git.openjdk.org/jdk/pull/20767.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20767/head:pull/20767 PR: https://git.openjdk.org/jdk/pull/20767
Withdrawn: 8339358: Optimize TypeKind#from
On Thu, 29 Aug 2024 05:34:37 GMT, Shaojin Wen wrote: > TypeKind.from(Class) is a frequently called method, which provides a > specialized method to improve performance. > > The following Compiler log shows that the call stack level is reduced and two > reference accesses (descriptorString() -> String.value) are reduced, which > can reduce the performance degradation caused by cache misses. > > * baseline > > @ 48 java.lang.classfile.TypeKind::from (25 bytes) inline > @ 1 java.lang.Class::isPrimitive (0 bytes) intrinsic > @ 10 java.lang.Class::descriptorString (170 bytes) failed to inline: > callee is too large > @ 15 java.lang.classfile.TypeKind::fromDescriptor (232 bytes) failed to > inline: callee is too large > > > * current > > @ 52 java.lang.classfile.TypeKind::from (103 bytes) failed to inline: > callee is too large This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/20762
Re: RFR: 8340710: Optimize DirectClassBuilder::build [v2]
> Do some refactoring so that the code can be inlined by the C1/C2 optimizer. > > 1. DirectClassBuilder::build codeSize 361 -> 319 > 2. DirectCodeBuilder::writeExceptionHandlers codeSize 183 -> 31 > 3. BufWriterImpl::writeIndex codeSize 62 -> 37 (forceinline) > 4. BufWriterImpl::writeU2 (forceinline) > 5. Util::writeAttributes codSize 45 -> 40 (forceinline) > 6. Util::writeList codSize 47 -> 42 (forceinline) Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits: - Merge remote-tracking branch 'upstream/master' into optim_classfile_build_202409 # Conflicts: #src/java.base/share/classes/jdk/internal/classfile/impl/Util.java - suggestion from @liach - inline writeLong & local constantPool - fix write header - suggestion from @liach - force inline Util::writeAttributes & Util::writeList - force inline writeU2 - force inline invalidIndex - optimize for writeExceptionHandlers for C1 - optimize DirectClassBuilder::build - Changes: https://git.openjdk.org/jdk/pull/21118/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21118&range=01 Stats: 50 lines in 4 files changed: 31 ins; 4 del; 15 mod Patch: https://git.openjdk.org/jdk/pull/21118.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21118/head:pull/21118 PR: https://git.openjdk.org/jdk/pull/21118
Re: RFR: 8340708: Optimize StackMapGenerator::processMethod [v3]
> A small optimization of processMethod, by using local variables and > extracting exception methods, reduces codeSize from 326 to 283 Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: reduce getFrame - Changes: - all: https://git.openjdk.org/jdk/pull/21137/files - new: https://git.openjdk.org/jdk/pull/21137/files/ba35444b..ba1f9aa4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21137&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21137&range=01-02 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/21137.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21137/head:pull/21137 PR: https://git.openjdk.org/jdk/pull/21137
Re: RFR: 8340708: Optimize StackMapGenerator::processMethod [v2]
> A small optimization of processMethod, by using local variables and > extracting exception methods, reduces codeSize from 326 to 291 Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: suggestion from @liach - Changes: - all: https://git.openjdk.org/jdk/pull/21137/files - new: https://git.openjdk.org/jdk/pull/21137/files/e0d4e0ad..ba35444b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21137&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21137&range=00-01 Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/21137.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21137/head:pull/21137 PR: https://git.openjdk.org/jdk/pull/21137
Re: RFR: 8340710: Optimize DirectClassBuilder::build
On Mon, 23 Sep 2024 00:33:26 GMT, Chen Liang wrote:
>> Do some refactoring so that the code can be inlined by the C1/C2 optimizer.
>>
>> 1. DirectClassBuilder::build codeSize 361 -> 315
>> 2. DirectCodeBuilder::writeExceptionHandlers codeSize 183 -> 31
>> 3. BufWriterImpl::writeIndex codeSize 62 -> 37 (forceinline)
>> 4. BufWriterImpl::writeU2 (forceinline)
>> 5. Util::writeAttributes codSize 45 -> 40 (forceinline)
>> 6. Util::writeList codSize 47 -> 42 (forceinline)
>
> src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java
> line 99:
>
>> 97: }
>> 98:
>> 99: void writeMagic(int minorVersion, int majorVersion) {
>
> Can we call this `writeHeader` as this writes both the magic and the version?
Now, through the local variable constantPool, codeSize < 325 can be achieved
without extracting a separate method.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21118#discussion_r1771513668
Re: RFR: 8340710: Optimize DirectClassBuilder::build
On Sun, 22 Sep 2024 20:48:08 GMT, Chen Liang wrote: >> src/java.base/share/classes/jdk/internal/classfile/impl/DirectClassBuilder.java >> line 207: >> >>> 205: >>> 206: // Now we can make the head >>> 207: head.writeLong(((long) ClassFile.MAGIC_NUMBER) << 32 | >>> minorVersion << 16 | majorVersion); >> >> `minorVersion` needs to be cast to a `long` first, as otherwise when the MSB >> is set after the shift, then it’ll overwrite the magic number with all 1s. >> >> Suggestion: >> >> head.writeLong(((long) ClassFile.MAGIC_NUMBER) << 32 | ((long) >> minorVersion) << 16 | majorVersion); > > For clarity, I think using Integer.toUnsignedLong on the shift result is > better If minorVersion > 0x, the result of using Integer.toUnsignedLong will be wrong. I guess this is the reason why the previous version build failed - PR Review Comment: https://git.openjdk.org/jdk/pull/21118#discussion_r1770667423
RFR: 8340710: Optimize DirectClassBuilder::build
Do some refactoring so that the code can be inlined by the C1/C2 optimizer. 1. DirectClassBuilder::build codeSize 361 -> 315 2. DirectCodeBuilder::writeExceptionHandlers codeSize 183 -> 31 3. BufWriterImpl::writeIndex codeSize 62 -> 37 (forceinline) 4. BufWriterImpl::writeU2 (forceinline) 5. Util::writeAttributes codSize 45 -> 40 (forceinline) 6. Util::writeList codSize 47 -> 42 (forceinline) - Commit messages: - inline writeLong & local constantPool - fix write header - suggestion from @liach - force inline Util::writeAttributes & Util::writeList - force inline writeU2 - force inline invalidIndex - optimize for writeExceptionHandlers for C1 - optimize DirectClassBuilder::build Changes: https://git.openjdk.org/jdk/pull/21118/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21118&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8340710 Stats: 50 lines in 4 files changed: 31 ins; 6 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/21118.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21118/head:pull/21118 PR: https://git.openjdk.org/jdk/pull/21118
RFR: 8340708: Optimize StackMapGenerator::processMethod
A small optimization of processMethod, by using local variables and extracting exception methods, reduces codeSize from 326 to 286 - Commit messages: - Update src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java - optimize processMethod Changes: https://git.openjdk.org/jdk/pull/21137/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21137&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8340708 Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/21137.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21137/head:pull/21137 PR: https://git.openjdk.org/jdk/pull/21137
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v19]
> After PR https://github.com/openjdk/jdk/pull/16245, C2 optimizes stores into > primitive arrays by combining values into larger stores. > > This PR rewrites the code of appendNull and append(boolean) methods so that > these two methods can be optimized by C2. Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: fix build error - Changes: - all: https://git.openjdk.org/jdk/pull/19626/files - new: https://git.openjdk.org/jdk/pull/19626/files/399c8ef5..ae054771 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19626&range=18 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19626&range=17-18 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19626.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19626/head:pull/19626 PR: https://git.openjdk.org/jdk/pull/19626
Re: RFR: 8337279: Optimize format instant [v7]
On Tue, 3 Sep 2024 17:32:43 GMT, Naoto Sato wrote: > I don't think using SharedSecret just for performance improvement and code > size reduction is the right way, as the class is the last resort as the > header says: > > ``` > * > * Usage of these APIs often means bad encapsulation designs, > * increased complexity and lack of sustainability. > * Use this only as a last resort! > * > ``` If SharedSecret is not recommended, do you recommend extracting these methods to jdk.internal? - PR Comment: https://git.openjdk.org/jdk/pull/20353#issuecomment-2367270116
Re: RFR: 8337279: Optimize format instant [v9]
> By removing the redundant code logic in > DateTimeFormatterBuilder$InstantPrinterParser#formatTo, the codeSize can be > reduced and the performance can be improved. Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into optim_instant_fmt_202407 - suggestion from @jensli - Speed up Instant.toString using JavaTimeAccess - add JavaTimeAccess SharedSecrets - Merge remote-tracking branch 'upstream/master' into optim_instant_fmt_202407 - breaking out the printNano methods - copyright - Update src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java Co-authored-by: Stephen Colebourne - 1. fix handle fraction == -1 2. Split two methods to make codeSize less than 325 - add comment - ... and 1 more: https://git.openjdk.org/jdk/compare/32c9de0b...1c2482e7 - Changes: - all: https://git.openjdk.org/jdk/pull/20353/files - new: https://git.openjdk.org/jdk/pull/20353/files/2342276e..1c2482e7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20353&range=08 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20353&range=07-08 Stats: 179340 lines in 1462 files changed: 162395 ins; 8883 del; 8062 mod Patch: https://git.openjdk.org/jdk/pull/20353.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20353/head:pull/20353 PR: https://git.openjdk.org/jdk/pull/20353
Re: RFR: 8337279: Optimize format instant [v8]
> By removing the redundant code logic in > DateTimeFormatterBuilder$InstantPrinterParser#formatTo, the codeSize can be > reduced and the performance can be improved. Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: suggestion from @jensli - Changes: - all: https://git.openjdk.org/jdk/pull/20353/files - new: https://git.openjdk.org/jdk/pull/20353/files/df2a13d4..2342276e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20353&range=07 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20353&range=06-07 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/20353.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20353/head:pull/20353 PR: https://git.openjdk.org/jdk/pull/20353
RFR: 8340587: Optimize StackMapGenerator$Frame::checkAssignableTo
Optimize checkAssignableTo to avoid clone when stackSize is 0, and use clone instead of Array.copyOf to avoid compression and then expansion - Commit messages: - suggestion from @liach - optimize checkAssignableTo Changes: https://git.openjdk.org/jdk/pull/21121/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21121&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8340587 Stats: 7 lines in 1 file changed: 4 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/21121.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21121/head:pull/21121 PR: https://git.openjdk.org/jdk/pull/21121
Re: RFR: 8340587: Optimize StackMapGenerator$Frame::checkAssignableTo
On Sun, 22 Sep 2024 14:08:38 GMT, Shaojin Wen wrote:
>> src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java
>> line 1102:
>>
>>> 1100: target.localsSize = localsSize;
>>> 1101: if (stackSize > 0) {
>>> 1102: target.stack = Arrays.copyOf(stack, stackSize);
>>
>> Do you think we should use `.clone()` to avoid `getClass` checks and to
>> avoid extra array growing in case the new stack/local immediately grows due
>> to instructions after target?
>>
>> Also I think you can do `localsSize > 0` check for locals assignment too.
>
> stack.length may be larger than stackSize. In this case, the length of the
> array copied by clone will be longer than Arrays.copyOf. However, I have no
> evidence that Arrays.copyOf is faster than clone.
I found through debugging that in the `target.flags == -1` branch of
checkAssignableTo, `localsSize > 0 and stackSize = 0` are always, so I made
this optimization.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21121#discussion_r1770564307
Re: RFR: 8340587: Optimize StackMapGenerator$Frame::checkAssignableTo
On Sun, 22 Sep 2024 13:37:38 GMT, Chen Liang wrote:
>> Optimize checkAssignableTo to avoid clone when stackSize is 0, and use clone
>> instead of Array.copyOf to avoid compression and then expansion
>
> src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java
> line 1102:
>
>> 1100: target.localsSize = localsSize;
>> 1101: if (stackSize > 0) {
>> 1102: target.stack = Arrays.copyOf(stack, stackSize);
>
> Do you think we should use `.clone()` to avoid `getClass` checks and to avoid
> extra array growing in case the new stack/local immediately grows due to
> instructions after target?
>
> Also I think you can do `localsSize > 0` check for locals assignment too.
stack.length may be larger than stackSize. In this case, the length of the
array copied by clone will be longer than Arrays.copyOf. However, I have no
evidence that Arrays.copyOf is faster than clone.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21121#discussion_r1770563199
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v17]
On Sun, 22 Sep 2024 02:03:02 GMT, Chen Liang wrote: > We should declare `count` before `ensureCapacitiyInternal`. Same for append > boolean. Declaring count before ensureCapacityInternal will cause performance regression under x64. It took a lot of time to find this, but the underlying reason is still unclear. - PR Review Comment: https://git.openjdk.org/jdk/pull/19626#discussion_r1770587530
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v18]
On Sun, 22 Sep 2024 02:01:36 GMT, Chen Liang wrote:
>> Shaojin Wen has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 23 additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'upstream/master' into
>> optim_str_builder_append_202406
>> - Merge remote-tracking branch 'origin/optim_str_builder_append_202406'
>> into optim_str_builder_append_202406
>> - Merge remote-tracking branch 'upstream/master' into
>> optim_str_builder_append_202406
>> - Merge remote-tracking branch 'upstream/master' into
>> optim_str_builder_append_202406
>> - revert test
>> - Merge remote-tracking branch 'upstream/master' into
>> optim_str_builder_append_202406
>> - Merge remote-tracking branch 'upstream/master' into
>> optim_str_builder_append_202406
>> - replace unsafe with putChar
>> - Merge remote-tracking branch 'upstream/master' into
>> optim_str_builder_append_202406
>> - private static final field `UNSAFE`
>> - ... and 13 more: https://git.openjdk.org/jdk/compare/7ba4356c...399c8ef5
>
> test/hotspot/jtreg/compiler/patches/java.base/java/lang/Helper.java line 136:
>
>> 134:
>> 135: public static int putCharsAt(byte[] value, int i, char c1, char c2,
>> char c3, char c4) {
>> 136: return StringUTF16.putCharsAt(value, i, c1, c2, c3, c4);
>
> Why do we remove the tests for UTF16? And should we add another set of test
> for LATIN1 too?
An early version removed putCharsAt, so it was also removed from Helpers. I
have added it back.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19626#discussion_r1770586891
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v18]
> After PR https://github.com/openjdk/jdk/pull/16245, C2 optimizes stores into > primitive arrays by combining values into larger stores. > > This PR rewrites the code of appendNull and append(boolean) methods so that > these two methods can be optimized by C2. Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 23 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406 - Merge remote-tracking branch 'origin/optim_str_builder_append_202406' into optim_str_builder_append_202406 - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406 - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406 - revert test - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406 - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406 - replace unsafe with putChar - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406 - private static final field `UNSAFE` - ... and 13 more: https://git.openjdk.org/jdk/compare/7ba4356c...399c8ef5 - Changes: - all: https://git.openjdk.org/jdk/pull/19626/files - new: https://git.openjdk.org/jdk/pull/19626/files/61196ecd..399c8ef5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19626&range=17 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19626&range=16-17 Stats: 180711 lines in 1606 files changed: 163525 ins; 8881 del; 8305 mod Patch: https://git.openjdk.org/jdk/pull/19626.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19626/head:pull/19626 PR: https://git.openjdk.org/jdk/pull/19626
Integrated: 8340544: Optimize setLocalsFromArg
On Fri, 20 Sep 2024 09:18:32 GMT, Shaojin Wen wrote: > CheckLocal once, reduce redundant checkLocal, rewrite switch, reduce method > size, codeSize is reduced from 367 to 263. This pull request has now been integrated. Changeset: ab06a878 Author:Shaojin Wen URL: https://git.openjdk.org/jdk/commit/ab06a878f27026424530781f0af414a8a611 Stats: 27 lines in 1 file changed: 8 ins; 5 del; 14 mod 8340544: Optimize setLocalsFromArg Reviewed-by: redestad, liach - PR: https://git.openjdk.org/jdk/pull/21106
Re: RFR: 8339320: Optimize ClassFile Utf8EntryImpl#inflate [v2]
> A very small optimization, split the large method inflate, split the > infrequently used paths into a method inflateCHAR Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits: - Merge remote-tracking branch 'origin/optim_class_file_pool_inflat_202408' into optim_class_file_pool_inflat_202408 - Update src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Update src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - optimize Utf8EntryImpl inflate - Merge remote-tracking branch 'upstream/master' into optim_class_file_pool_inflat_202408 # Conflicts: # src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java - Suggestions from @liach - fix build error - optimize Utf8EntryImpl inflate - Changes: https://git.openjdk.org/jdk/pull/20767/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20767&range=01 Stats: 76 lines in 1 file changed: 23 ins; 20 del; 33 mod Patch: https://git.openjdk.org/jdk/pull/20767.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20767/head:pull/20767 PR: https://git.openjdk.org/jdk/pull/20767
Integrated: 8339217: Optimize ClassFile API loadConstant
On Thu, 29 Aug 2024 05:01:52 GMT, Shaojin Wen wrote:
> This is a large method. By splitting it into multiple methods with the same
> name, the caller can automatically select based on the different types of
> parameters, avoiding this large call that cannot be inlined, which can also
> improve startup performance.
>
> * current
>
> CodeBuilder {
> default CodeBuilder loadConstant(ConstantDesc value) { ... }
> }
>
> java.lang.classfile.CodeBuilder::loadConstant (465 bytes) failed to inline:
> callee is too large
This pull request has now been integrated.
Changeset: 2461263a
Author:Shaojin Wen
URL:
https://git.openjdk.org/jdk/commit/2461263aac35b25e2a48b6fc84da49e4b553dbc3
Stats: 90 lines in 2 files changed: 63 ins; 22 del; 5 mod
8339217: Optimize ClassFile API loadConstant
Reviewed-by: liach, redestad, asotona
-
PR: https://git.openjdk.org/jdk/pull/20761
Integrated: 8340232: Optimize DataInputStream::readUTF
On Sun, 8 Sep 2024 07:52:26 GMT, Shaojin Wen wrote: > Similar to ObjectInputStream, use JLA.countPositives and > JLA.inflateBytesToChars to speed up readUTF This pull request has now been integrated. Changeset: 40fba148 Author:Shaojin Wen URL: https://git.openjdk.org/jdk/commit/40fba148125b9e0d35755b6e6fd701e69d22f7da Stats: 42 lines in 1 file changed: 28 ins; 2 del; 12 mod 8340232: Optimize DataInputStream::readUTF Reviewed-by: liach, bpb - PR: https://git.openjdk.org/jdk/pull/20903
Re: RFR: 8340544: Optimize setLocalsFromArg [v2]
On Fri, 20 Sep 2024 16:13:06 GMT, Claes Redestad wrote:
>> Shaojin Wen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> more compact
>
> src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java
> line 1069:
>
>> 1067: localsSize += 2;
>> 1068: } else {
>> 1069: if (desc == CD_int || desc == CD_boolean || desc
>> == CD_byte || desc == CD_char || desc == CD_short) {
>
> An alternative would be `if (!desc.isPrimitive()) { .. } else if (desc ==
> CD_float) { .. } else { /* INTEGER_TYPE */ }` - might be more compact at
> least.
because ClassDesc permits PrimitiveClassDescImpl & ReferenceClassDescImpl,
based on @cl4es 's suggestion, I used `instanceof ReferenceClassDescImpl`
instead of `!isPrimitive()`, and now codeSize becomes 239
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21106#discussion_r1768900873
Re: RFR: 8340544: Optimize setLocalsFromArg [v2]
> CheckLocal once, reduce redundant checkLocal, rewrite switch, reduce method > size, codeSize is reduced from 367 to 263. Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: more compact - Changes: - all: https://git.openjdk.org/jdk/pull/21106/files - new: https://git.openjdk.org/jdk/pull/21106/files/9d554f11..7d71290b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21106&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21106&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/21106.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21106/head:pull/21106 PR: https://git.openjdk.org/jdk/pull/21106
RFR: 8340544: Optimize setLocalsFromArg
CheckLocal once, reduce redundant checkLocal, rewrite switch, reduce method size, codeSize is reduced from 367 to 263. - Commit messages: - optimize setLocalsFromArg Changes: https://git.openjdk.org/jdk/pull/21106/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21106&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8340544 Stats: 27 lines in 1 file changed: 8 ins; 5 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/21106.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21106/head:pull/21106 PR: https://git.openjdk.org/jdk/pull/21106
Re: RFR: 8339699: Optimize DataOutputStream writeUTF [v5]
On Thu, 19 Sep 2024 19:00:45 GMT, Chen Liang wrote: >> Shaojin Wen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> suggestion from @liach > > src/java.base/share/classes/java/io/ObjectOutputStream.java line 2047: > >> 2045: writeByte(TC_LONGSTRING); >> 2046: } >> 2047: writeLong(utflen); > > The old plain `writeUTF` does not write a long utf if the length is too long, > but rather throws an `UTFDataFormatException`. There should be code paths > that no longer throw this exception, because a few tests, including: > - `java/io/Serializable/longString/LongString.java` > - `java/io/Serializable/sanityCheck/SanityCheck.java` > - `java/text/Format/DateFormat/DateFormatRegression.java` > are failing. I think this might be the root cause. The reason for the error is that when the `drain` method is called, the local variable pos is not updated to the field `pos` - PR Review Comment: https://git.openjdk.org/jdk/pull/20886#discussion_r1767990262
Re: RFR: 8339699: Optimize DataOutputStream writeUTF [v6]
> PR #20772 introduced an optimization for writeUTF, which can also be used in > DataOutputStream::writeUTF. Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: bug fix for write long utf - Changes: - all: https://git.openjdk.org/jdk/pull/20886/files - new: https://git.openjdk.org/jdk/pull/20886/files/83fc4685..8d004285 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20886&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20886&range=04-05 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/20886.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20886/head:pull/20886 PR: https://git.openjdk.org/jdk/pull/20886
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v6]
On Tue, 10 Sep 2024 13:24:33 GMT, Shaojin Wen wrote: >> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line >> 529: >> >>> 527: mh = simpleConcat3(paramType0); >>> 528: mh = MethodHandles.insertArguments(mh, 0, prefix); >>> 529: return MethodHandles.filterArguments(mh, 1, >>> objectStringifier()); >> >> While this is a fun trick it seems like there's a non-trivial cost here? >> We'd go down different paths and generate different classes for `"foo" + bar >> + baz` and `"foo" + bar + " .. " + baz` with this. Special casing when we >> get the added shapes for more or less free (plain `simpleConcat()`) is a >> different matter but here you need to construct a new couple of shapes with >> `insert-` and `filterArguments`. >> >> (Check on paramType1 could be `!paramType1.isPrimitive()`) > > I'm also not sure how much cost the simpleConcat handling of the 2 parameters > would bring, I've removed that, and this is more appropriately implemented by > the InlineHiddenClassStrategy. If we handle the null values of prefix and suffix, the cost of supporting the 2-parameter scenario will be much lower. - PR Review Comment: https://git.openjdk.org/jdk/pull/20726#discussion_r1763226369
Re: RFR: 8340232: Optimize DataInputStream::readUTF
On Mon, 16 Sep 2024 13:15:02 GMT, Chen Liang wrote:
>> Similar to ObjectInputStream, use JLA.countPositives and
>> JLA.inflateBytesToChars to speed up readUTF
>
> src/java.base/share/classes/java/io/DataInputStream.java line 590:
>
>> 588: if (bytearr == null) {
>> 589: bytearr = new byte[utflen];
>> 590: allocate = true;
>
> Can we rename this boolean to `trusted` and set it to `false` when we assign
> it back to `dis.bytearr`? Even though that assignment will be redundant,
> calling it `trusted` is helpful to avoid causing security problems if we
> reorganize this code in the future.
When ascii != utflen, bytearr will be reused, and the name of `trusted` is not
clear here.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20903#discussion_r1761486696
Re: RFR: 8340232: Optimize DataInputStream::readUTF
On Sun, 8 Sep 2024 07:52:26 GMT, Shaojin Wen wrote: > Similar to ObjectInputStream, use JLA.countPositives and > JLA.inflateBytesToChars to speed up readUTF In the readUTFAscii scenario, the performance is significantly improved, but in the readUTFSmall scenario, the performance is regressed. Here are the performance numbers on the MacBook M1 Pro: ## Script # baseline git checkout fbe2629303bcee5855673b7e37d8c49f19dc9849 make test TEST="micro:java.io.DataInputStreamTest.readUTF" # current git checkout 5c4bd6db977ca8729e078d26cb7ca284ab42203c make test TEST="micro:java.io.DataInputStreamTest.readUTF" ## Performance Numbers -# baseline -Benchmark Mode Cnt Score Error Units -DataInputStreamTest.readUTFAsciiLarge avgt 20 1.401 ? 0.005 us/op -DataInputStreamTest.readUTFAsciiMixed avgt 20 1.337 ? 0.092 us/op -DataInputStreamTest.readUTFAsciiSmall avgt 20 0.872 ? 0.004 us/op -DataInputStreamTest.readUTFLarge avgt 20 2.178 ? 0.050 us/op -DataInputStreamTest.readUTFMixed avgt 20 1.700 ? 0.109 us/op -DataInputStreamTest.readUTFSmall avgt 20 0.879 ? 0.012 us/op +# current +Benchmark Mode Cnt Score Error Units +DataInputStreamTest.readUTFAsciiLarge avgt 20 0.833 ? 0.004 us/op +DataInputStreamTest.readUTFAsciiMixed avgt 20 0.804 ? 0.001 us/op +DataInputStreamTest.readUTFAsciiSmall avgt 20 0.828 ? 0.005 us/op +DataInputStreamTest.readUTFLarge avgt 20 2.053 ? 0.063 us/op +DataInputStreamTest.readUTFMixed avgt 20 1.675 ? 0.056 us/op +DataInputStreamTest.readUTFSmall avgt 20 1.834 ? 0.026 us/op | | baseline | current | delta | | --- | --- | --- | --- | | DataInputStreamTest.readUTFAsciiLarge | 1.401 | 0.833 | 68.19% | | DataInputStreamTest.readUTFAsciiMixed | 1.337 | 0.804 | 66.29% | | DataInputStreamTest.readUTFAsciiSmall | 0.872 | 0.828 | 5.31% | | DataInputStreamTest.readUTFLarge | 2.178 | 2.053 | 6.09% | | DataInputStreamTest.readUTFMixed | 1.700 | 1.675 | 1.49% | | DataInputStreamTest.readUTFSmall | 0.879 | 1.834 | -52.07% | - PR Comment: https://git.openjdk.org/jdk/pull/20903#issuecomment-2347702045
RFR: 8340232: Optimize DataInputStream::readUTF
Similar to ObjectInputStream, use JLA.countPositives and JLA.inflateBytesToChars to speed up readUTF - Commit messages: - suggestion from @liach - optimize for ascii - only allocate the char array if we have non-ascii - optimize DataOutputStream::readUTF Changes: https://git.openjdk.org/jdk/pull/20903/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20903&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8340232 Stats: 42 lines in 1 file changed: 28 ins; 2 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/20903.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20903/head:pull/20903 PR: https://git.openjdk.org/jdk/pull/20903
Re: RFR: 8339699: Optimize DataOutputStream writeUTF [v5]
> PR #20772 introduced an optimization for writeUTF, which can also be used in > DataOutputStream::writeUTF. Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: suggestion from @liach - Changes: - all: https://git.openjdk.org/jdk/pull/20886/files - new: https://git.openjdk.org/jdk/pull/20886/files/58a23d4f..83fc4685 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20886&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20886&range=03-04 Stats: 10 lines in 4 files changed: 3 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/20886.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20886/head:pull/20886 PR: https://git.openjdk.org/jdk/pull/20886
Re: RFR: 8339217: Optimize ClassFile API loadConstant [v6]
> This is a large method. By splitting it into multiple methods with the same
> name, the caller can automatically select based on the different types of
> parameters, avoiding this large call that cannot be inlined, which can also
> improve startup performance.
>
> * current
>
> CodeBuilder {
> default CodeBuilder loadConstant(ConstantDesc value) { ... }
> }
>
> java.lang.classfile.CodeBuilder::loadConstant (465 bytes) failed to inline:
> callee is too large
Shaojin Wen has updated the pull request incrementally with one additional
commit since the last revision:
fix comments
-
Changes:
- all: https://git.openjdk.org/jdk/pull/20761/files
- new: https://git.openjdk.org/jdk/pull/20761/files/9a365337..a1e6e3c5
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=20761&range=05
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=20761&range=04-05
Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/20761.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/20761/head:pull/20761
PR: https://git.openjdk.org/jdk/pull/20761
Re: RFR: 8339699: Optimize DataOutputStream writeUTF [v3]
On Fri, 13 Sep 2024 17:05:11 GMT, Chen Liang wrote:
>> Shaojin Wen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> reduce JDKUTF#utflen codeSize
>
> src/java.base/share/classes/jdk/internal/util/JDKUTF.java line 37:
>
>> 35: public abstract class JDKUTF {
>> 36: @ForceInline
>> 37: public static int putChar(byte[] buf, int offset, char c) {
>
> Do you think we can just move the loop here, like:
>
> public static int putChars(byte[] buf, int offset, String str, int strStart,
> int strEnd) {
> for (int i = strStart; i < strEnd; i++) {
> offset = putChar(buf, offset, str.charAt(i);
> }
> }
>
> We can even unroll putChar in the loop.
This does not work in ObjectOutputStream#writeMoreUTF
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20886#discussion_r1759243420
Re: RFR: 8339699: Optimize DataOutputStream writeUTF [v4]
> PR #20772 introduced an optimization for writeUTF, which can also be used in > DataOutputStream::writeUTF. Shaojin Wen has updated the pull request incrementally with two additional commits since the last revision: - rename JDKUTF to ModifiedUtf - suggestion from @liach - Changes: - all: https://git.openjdk.org/jdk/pull/20886/files - new: https://git.openjdk.org/jdk/pull/20886/files/9744e8a5..58a23d4f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20886&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20886&range=02-03 Stats: 47 lines in 4 files changed: 21 ins; 15 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/20886.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20886/head:pull/20886 PR: https://git.openjdk.org/jdk/pull/20886
Re: RFR: 8339217: Optimize ClassFile API loadConstant [v5]
> This is a large method. By splitting it into multiple methods with the same
> name, the caller can automatically select based on the different types of
> parameters, avoiding this large call that cannot be inlined, which can also
> improve startup performance.
>
> * current
>
> CodeBuilder {
> default CodeBuilder loadConstant(ConstantDesc value) { ... }
> }
>
> java.lang.classfile.CodeBuilder::loadConstant (465 bytes) failed to inline:
> callee is too large
Shaojin Wen has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains six commits:
- Merge remote-tracking branch 'upstream/master' into
optim_classfile_loadconstant_2020408
# Conflicts:
#
src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
- Merge remote-tracking branch 'upstream/master' into
optim_classfile_loadconstant_2020408
# Conflicts:
#src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
- comments
- fix comment
- since 24
- optimize loadConstant
-
Changes: https://git.openjdk.org/jdk/pull/20761/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20761&range=04
Stats: 86 lines in 2 files changed: 59 ins; 22 del; 5 mod
Patch: https://git.openjdk.org/jdk/pull/20761.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/20761/head:pull/20761
PR: https://git.openjdk.org/jdk/pull/20761
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v6]
On Tue, 10 Sep 2024 10:28:56 GMT, Claes Redestad wrote: >> Shaojin Wen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> more 2 arguments simple concat > > src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line > 529: > >> 527: mh = simpleConcat3(paramType0); >> 528: mh = MethodHandles.insertArguments(mh, 0, prefix); >> 529: return MethodHandles.filterArguments(mh, 1, >> objectStringifier()); > > While this is a fun trick it seems like there's a non-trivial cost here? We'd > go down different paths and generate different classes for `"foo" + bar + > baz` and `"foo" + bar + " .. " + baz` with this. Special casing when we get > the added shapes for more or less free (plain `simpleConcat()`) is a > different matter but here you need to construct a new couple of shapes with > `insert-` and `filterArguments`. > > (Check on paramType1 could be `!paramType1.isPrimitive()`) I'm also not sure how much cost the simpleConcat handling of the 2 parameters would bring, I've removed that, and this is more appropriately implemented by the InlineHiddenClassStrategy. - PR Review Comment: https://git.openjdk.org/jdk/pull/20726#discussion_r1751961145
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v7]
> The string concatenation of the java.base module is implemented using > StringBuilder. By providing a series of concat methods in StringConcatHelper, > it is used in the java.lang package to replace string concatenation. > > These concat methods can also be exposed through JLA for use by other > packages, such as java.lang.constant. > > These concat methods can replace Concat1 and become part of > StringConcatFactory#simpleConcat Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: remove 2 arguments simple concat - Changes: - all: https://git.openjdk.org/jdk/pull/20726/files - new: https://git.openjdk.org/jdk/pull/20726/files/ed070af2..de3c1d70 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=05-06 Stats: 19 lines in 1 file changed: 0 ins; 16 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/20726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20726/head:pull/20726 PR: https://git.openjdk.org/jdk/pull/20726
Integrated: 8338930: StringConcatFactory hardCoded string concatenation strategy
On Thu, 22 Aug 2024 11:50:02 GMT, Shaojin Wen wrote: > This is a follow-up to PR #20273, which improves performance when the number > of parameters exceeds 20. > > When the number of parameters is large, the possibility of reuse will be > lower, so we can use the static concat method and write the length and coder > directly into the bytecode to solve the performance regression problem. This pull request has now been integrated. Changeset: 4d597de8 Author:Shaojin Wen Committer: Claes Redestad URL: https://git.openjdk.org/jdk/commit/4d597de893dad79e74a280f3f9e82f0a14f9045d Stats: 132 lines in 3 files changed: 83 ins; 2 del; 47 mod 8338930: StringConcatFactory hardCoded string concatenation strategy Reviewed-by: redestad, liach - PR: https://git.openjdk.org/jdk/pull/20675
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v6]
On Sun, 8 Sep 2024 16:37:08 GMT, Claes Redestad wrote: >> String concatenation is required in many places in java.lang. These static >> concat methods will be used instead of "+", so null value processing is >> added. This is also the motivation for using static concat methods instead >> of Concat1. > > I don't think replacing a lot of concatenations in java.base with > `SCH.concat` is very appealing and needs to be motivated by a substantial > performance advantage. And for the places where it's motivated we can make > sure to sanitize and handle `null` arguments. Yes, we shouldn't pay for things we don't need. I added null value handling to the parameters in SCF and removed it in SCH. - PR Review Comment: https://git.openjdk.org/jdk/pull/20726#discussion_r1751099693
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v6]
> The string concatenation of the java.base module is implemented using > StringBuilder. By providing a series of concat methods in StringConcatHelper, > it is used in the java.lang package to replace string concatenation. > > These concat methods can also be exposed through JLA for use by other > packages, such as java.lang.constant. > > These concat methods can replace Concat1 and become part of > StringConcatFactory#simpleConcat Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: more 2 arguments simple concat - Changes: - all: https://git.openjdk.org/jdk/pull/20726/files - new: https://git.openjdk.org/jdk/pull/20726/files/f63e6a7f..ed070af2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=04-05 Stats: 19 lines in 1 file changed: 16 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/20726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20726/head:pull/20726 PR: https://git.openjdk.org/jdk/pull/20726
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v5]
> The string concatenation of the java.base module is implemented using > StringBuilder. By providing a series of concat methods in StringConcatHelper, > it is used in the java.lang package to replace string concatenation. > > These concat methods can also be exposed through JLA for use by other > packages, such as java.lang.constant. > > These concat methods can replace Concat1 and become part of > StringConcatFactory#simpleConcat Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: reduce changes - Changes: - all: https://git.openjdk.org/jdk/pull/20726/files - new: https://git.openjdk.org/jdk/pull/20726/files/202934ad..f63e6a7f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=03-04 Stats: 5 lines in 1 file changed: 0 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/20726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20726/head:pull/20726 PR: https://git.openjdk.org/jdk/pull/20726
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v4]
> The string concatenation of the java.base module is implemented using > StringBuilder. By providing a series of concat methods in StringConcatHelper, > it is used in the java.lang package to replace string concatenation. > > These concat methods can also be exposed through JLA for use by other > packages, such as java.lang.constant. > > These concat methods can replace Concat1 and become part of > StringConcatFactory#simpleConcat Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: remove 2 arguments simple concat - Changes: - all: https://git.openjdk.org/jdk/pull/20726/files - new: https://git.openjdk.org/jdk/pull/20726/files/0cacd090..202934ad Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=02-03 Stats: 12 lines in 1 file changed: 0 ins; 12 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/20726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20726/head:pull/20726 PR: https://git.openjdk.org/jdk/pull/20726
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v3]
> The string concatenation of the java.base module is implemented using > StringBuilder. By providing a series of concat methods in StringConcatHelper, > it is used in the java.lang package to replace string concatenation. > > These concat methods can also be exposed through JLA for use by other > packages, such as java.lang.constant. > > These concat methods can replace Concat1 and become part of > StringConcatFactory#simpleConcat Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: Let the caller ensure that prefix and suffix are not null - Changes: - all: https://git.openjdk.org/jdk/pull/20726/files - new: https://git.openjdk.org/jdk/pull/20726/files/5025729d..0cacd090 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=01-02 Stats: 18 lines in 1 file changed: 0 ins; 18 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/20726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20726/head:pull/20726 PR: https://git.openjdk.org/jdk/pull/20726
Re: RFR: 8338930: StringConcatFactory hardCoded string concatenation strategy [v11]
> This is a follow-up to PR #20273, which improves performance when the number > of parameters exceeds 20. > > When the number of parameters is large, the possibility of reuse will be > lower, so we can use the static concat method and write the length and coder > directly into the bytecode to solve the performance regression problem. Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: suggestion from @cl4es - Changes: - all: https://git.openjdk.org/jdk/pull/20675/files - new: https://git.openjdk.org/jdk/pull/20675/files/711e96d6..6c69ab34 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20675&range=10 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20675&range=09-10 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20675.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20675/head:pull/20675 PR: https://git.openjdk.org/jdk/pull/20675
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v2]
On Sun, 8 Sep 2024 13:31:53 GMT, Claes Redestad wrote:
>> Shaojin Wen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update
>> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java
>>
>> Co-authored-by: Claes Redestad
>
> src/java.base/share/classes/java/lang/StringConcatHelper.java line 731:
>
>> 729: @ForceInline
>> 730: static String concat(String prefix, float value, String suffix) {
>> 731: if (prefix == null) prefix = "null";
>
> Since we'll never bind in `null` values all these `prefix == null` are likely
> redundant unless we expose them to users. Which we probably shouldn't. It's a
> good thing this PR actually removes some shared secrets rather than adding
> new ones.
String concatenation is required in many places in java.lang. These static
concat methods will be used instead of "+", so null value processing is added.
This is also the motivation for using static concat methods instead of Concat1.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20726#discussion_r1749256205
Re: RFR: 8339704: Refactor StringConcatHelper simpleConcat [v2]
> The string concatenation of the java.base module is implemented using > StringBuilder. By providing a series of concat methods in StringConcatHelper, > it is used in the java.lang package to replace string concatenation. > > These concat methods can also be exposed through JLA for use by other > packages, such as java.lang.constant. > > These concat methods can replace Concat1 and become part of > StringConcatFactory#simpleConcat Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java Co-authored-by: Claes Redestad - Changes: - all: https://git.openjdk.org/jdk/pull/20726/files - new: https://git.openjdk.org/jdk/pull/20726/files/a0bf70fa..5025729d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20726/head:pull/20726 PR: https://git.openjdk.org/jdk/pull/20726
RFR: 8339704: Refactor StringConcatHelper simpleConcat
The string concatenation of the java.base module is implemented using StringBuilder. By providing a series of concat methods in StringConcatHelper, it is used in the java.lang package to replace string concatenation. These concat methods can also be exposed through JLA for use by other packages, such as java.lang.constant. These concat methods can replace Concat1 and become part of StringConcatFactory#simpleConcat - Commit messages: - Merge remote-tracking branch 'upstream/master' into str_factory_more_simple_concat_202408 - Merge remote-tracking branch 'upstream/master' into str_factory_more_simple_concat_202408 - refactor simple concat Changes: https://git.openjdk.org/jdk/pull/20726/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20726&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8339704 Stats: 330 lines in 4 files changed: 229 ins; 88 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/20726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20726/head:pull/20726 PR: https://git.openjdk.org/jdk/pull/20726
