Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v7]
- Original Message -
> From: "Claes Redestad"
> To: "core-libs-dev"
> Sent: Friday, June 3, 2022 2:19:34 PM
> Subject: Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers
> in batches [v7]
> On Fri, 3 Jun 2022 11:19:04 GMT, ExE Boss wrote:
>
>>> You would think that, but javac doesn't do anything fancy once you store to
>>> a
>>> local
>>>
>>> javap output for lines 108 through 111:
>>>
>>>449: ldc #148// float 0.1f
>>>451: fstore31
>>>453: fload 31
>>>455: invokedynamic #149, 0// InvokeDynamic
>>>#4:makeConcatWithConstants:(F)Ljava/lang/String;
>>>460: astore32
>>>462: fload 31
>>>464: aload 4
>>>466: invokedynamic #152, 0// InvokeDynamic
>>>#7:makeConcatWithConstants:(FLjava/lang/String;)Ljava/lang/String;
>>>471: astore33
>>>473: aload 4
>>>475: fload 31
>>>477: invokedynamic #155, 0// InvokeDynamic
>>>#10:makeConcatWithConstants:(Ljava/lang/String;F)Ljava/lang/String;
>>
>> I guess it only happens for `final` locals.
>
> Yes, looks like javac does constant fold when the local is explicitly final:
>
> final float f = 0.1f;
> System.out.println("const folding? " + f);
>
> javap:
>
> 0: getstatic #2 // Field
> java/lang/System.out:Ljava/io/PrintStream;
> 3: ldc #3 // String const folding? 0.1
> 5: invokevirtual #4 // Method
> java/io/PrintStream.println:(Ljava/lang/String;)V
>
>
> Maybe this constant folding is something javac could be enhanced to do on
> effectively final locals, though I think we can defer adjusting
> `HelloClasslist` to break that optimization when that time comes.
The JLS defines final variables (and effectively final) here
https://docs.oracle.com/javase/specs/jls/se18/html/jls-4.html#jls-4.12.4
Constant expressions does not list effectively final variables as constant,
only final variables
https://docs.oracle.com/javase/specs/jls/se18/html/jls-15.html#jls-15.29
>
> -
>
> PR: https://git.openjdk.java.net/jdk/pull/8855
regards,
Rémi
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v7]
On Fri, 3 Jun 2022 11:19:04 GMT, ExE Boss wrote:
>> You would think that, but javac doesn't do anything fancy once you store to
>> a local
>>
>> javap output for lines 108 through 111:
>>
>>449: ldc #148// float 0.1f
>>451: fstore31
>>453: fload 31
>>455: invokedynamic #149, 0// InvokeDynamic
>> #4:makeConcatWithConstants:(F)Ljava/lang/String;
>>460: astore32
>>462: fload 31
>>464: aload 4
>>466: invokedynamic #152, 0// InvokeDynamic
>> #7:makeConcatWithConstants:(FLjava/lang/String;)Ljava/lang/String;
>>471: astore33
>>473: aload 4
>>475: fload 31
>>477: invokedynamic #155, 0// InvokeDynamic
>> #10:makeConcatWithConstants:(Ljava/lang/String;F)Ljava/lang/String;
>
> I guess it only happens for `final` locals.
Yes, looks like javac does constant fold when the local is explicitly final:
final float f = 0.1f;
System.out.println("const folding? " + f);
javap:
0: getstatic #2 // Field
java/lang/System.out:Ljava/io/PrintStream;
3: ldc #3 // String const folding? 0.1
5: invokevirtual #4 // Method
java/io/PrintStream.println:(Ljava/lang/String;)V
Maybe this constant folding is something javac could be enhanced to do on
effectively final locals, though I think we can defer adjusting
`HelloClasslist` to break that optimization when that time comes.
-
PR: https://git.openjdk.java.net/jdk/pull/8855
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v7]
On Thu, 2 Jun 2022 20:01:33 GMT, Claes Redestad wrote: >> make/jdk/src/classes/build/tools/classlist/HelloClasslist.java line 116: >> >>> 114: String CC = "string" + c; >>> 115: String CCS= "string" + c + s; >>> 116: String CSCC = "string" + s + "string" + c; >> >> I’m pretty sure that `f` and `c` will be inlined as compile‑time constants >> in the String concat recipes. > > You would think that, but javac doesn't do anything fancy once you store to a > local > > javap output for lines 108 through 111: > >449: ldc #148// float 0.1f >451: fstore31 >453: fload 31 >455: invokedynamic #149, 0// InvokeDynamic > #4:makeConcatWithConstants:(F)Ljava/lang/String; >460: astore32 >462: fload 31 >464: aload 4 >466: invokedynamic #152, 0// InvokeDynamic > #7:makeConcatWithConstants:(FLjava/lang/String;)Ljava/lang/String; >471: astore33 >473: aload 4 >475: fload 31 >477: invokedynamic #155, 0// InvokeDynamic > #10:makeConcatWithConstants:(Ljava/lang/String;F)Ljava/lang/String; I guess it only happens for `final` locals. - PR: https://git.openjdk.java.net/jdk/pull/8855
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v7]
On Thu, 2 Jun 2022 18:49:13 GMT, ExE Boss wrote: >> Claes Redestad has updated the pull request incrementally with one >> additional commit since the last revision: >> >> We now don't need big Species classes for shorter concats, so on some >> tests the improvements meant more Species class generation. Adjusting >> HelloClasslist > > make/jdk/src/classes/build/tools/classlist/HelloClasslist.java line 116: > >> 114: String CC = "string" + c; >> 115: String CCS= "string" + c + s; >> 116: String CSCC = "string" + s + "string" + c; > > I’m pretty sure that `f` and `c` will be inlined as compile‑time constants in > the String concat recipes. You would think that, but javac doesn't do anything fancy once you store to a local javap output for lines 108 through 111: 449: ldc #148// float 0.1f 451: fstore31 453: fload 31 455: invokedynamic #149, 0// InvokeDynamic #4:makeConcatWithConstants:(F)Ljava/lang/String; 460: astore32 462: fload 31 464: aload 4 466: invokedynamic #152, 0// InvokeDynamic #7:makeConcatWithConstants:(FLjava/lang/String;)Ljava/lang/String; 471: astore33 473: aload 4 475: fload 31 477: invokedynamic #155, 0// InvokeDynamic #10:makeConcatWithConstants:(Ljava/lang/String;F)Ljava/lang/String; - PR: https://git.openjdk.java.net/jdk/pull/8855
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v7]
On Thu, 2 Jun 2022 10:57:37 GMT, Claes Redestad wrote: >> When generating `MethodHandle`-based concatenation expressions in >> `StringConcatFactory` we can reduce the number of classes generated at >> runtime by creating small batches of prependers and mixers before binding >> them into the root expression tree. >> >> Improvements on one-off tests are modest, while the improvement on >> bootstrapping stress tests can be substantial >> ([MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)): >> >> | Build |# classes| Runtime | >> | --- | - | --- | >> | Baseline | 31119 | 2.942s | >> | Patch | 16208 | 1.958s | >> >> An earlier version of this patch saw a regression in the >> `StringConcatFactoryBootstraps` microbenchmark. After some refactoring along >> with the optimizations in #8881 and #8923 that is no longer the case, and >> allocation pressure is down slightly compared to the baseline on such a >> repeat-the-same-bootstrap stress test: >> >> Baseline: >> >> Benchmark Mode Cnt >> Score Error Units >> SCFB.makeConcatWithConstants avgt5 >> 2170.039 ? 117.441 ns/op >> SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 >> 3538.020 ?4.558B/op >> >> This patch: >> >> Benchmark Mode Cnt >> Score Error Units >> SCFB.makeConcatWithConstants avgt5 >> 2144.807 ? 162.685 ns/op >> SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 >> 3184.943 ?4.495B/op > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > We now don't need big Species classes for shorter concats, so on some tests > the improvements meant more Species class generation. Adjusting HelloClasslist make/jdk/src/classes/build/tools/classlist/HelloClasslist.java line 116: > 114: String CC = "string" + c; > 115: String CCS= "string" + c + s; > 116: String CSCC = "string" + s + "string" + c; I’m pretty sure that `f` and `c` will be inlined as compile‑time constants in the String concat recipes. - PR: https://git.openjdk.java.net/jdk/pull/8855
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v7]
> When generating `MethodHandle`-based concatenation expressions in > `StringConcatFactory` we can reduce the number of classes generated at > runtime by creating small batches of prependers and mixers before binding > them into the root expression tree. > > Improvements on one-off tests are modest, while the improvement on > bootstrapping stress tests can be substantial > ([MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)): > > | Build |# classes| Runtime | > | --- | - | --- | > | Baseline | 31119 | 2.942s | > | Patch | 16208 | 1.958s | > > An earlier version of this patch saw a regression in the > `StringConcatFactoryBootstraps` microbenchmark. After some refactoring along > with the optimizations in #8881 and #8923 that is no longer the case, and > allocation pressure is down slightly compared to the baseline on such a > repeat-the-same-bootstrap stress test: > > Baseline: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2170.039 ? 117.441 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3538.020 ?4.558B/op > > This patch: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2144.807 ? 162.685 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3184.943 ?4.495B/op Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: We now don't need big Species classes for shorter concats, so on some tests the improvements meant more Species class generation. Adjusting HelloClasslist - Changes: - all: https://git.openjdk.java.net/jdk/pull/8855/files - new: https://git.openjdk.java.net/jdk/pull/8855/files/444cdd39..aaa442d5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8855&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8855&range=05-06 Stats: 18 lines in 1 file changed: 17 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8855.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8855/head:pull/8855 PR: https://git.openjdk.java.net/jdk/pull/8855
