Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers [v2]

2022-06-09 Thread Mandy Chung
On Thu, 9 Jun 2022 16:04:43 GMT, Claes Redestad  wrote:

>> To take optimal advantage of the pre-existing optimization for repeated 
>> filters we could split the application of different types of stringifiers.
>> 
>> The resulting difference in order of evaluation is not observable by 
>> conventional means since all reference type share the same object 
>> stringifier, and the others are filtering primitives (floats and doubles) 
>> which have been passed by value already. 
>> 
>> This change neutral on many concatenation expression shapes, but for any 
>> complex expressions with interleaving float/double and reference parameters 
>> it brings a straightforward reduction in rebinds and underlying LFs 
>> generated. For example on the 
>> [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)
>>  test there's a modest 2% reduction in total classes loaded with this change 
>> (from 16209 to 15872)
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Improve comments based on review feedback

Looks good to me.

Like Jorne's observation, it'd benefit everyone if this could generate just a 
single new lambda form multiple filters, not just for repeated filters if 
feasible.   Maybe something to explore more in the future with a tradeoff of 
complexity.

-

Marked as reviewed by mchung (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9082


Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers [v2]

2022-06-09 Thread Claes Redestad
> To take optimal advantage of the pre-existing optimization for repeated 
> filters we could split the application of different types of stringifiers.
> 
> The resulting difference in order of evaluation is not observable by 
> conventional means since all reference type share the same object 
> stringifier, and the others are filtering primitives (floats and doubles) 
> which have been passed by value already. 
> 
> This change neutral on many concatenation expression shapes, but for any 
> complex expressions with interleaving float/double and reference parameters 
> it brings a straightforward reduction in rebinds and underlying LFs 
> generated. For example on the 
> [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)
>  test there's a modest 2% reduction in total classes loaded with this change 
> (from 16209 to 15872)

Claes Redestad has updated the pull request incrementally with one additional 
commit since the last revision:

  Improve comments based on review feedback

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9082/files
  - new: https://git.openjdk.org/jdk/pull/9082/files/51c841e8..4b7c696e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=9082=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9082=00-01

  Stats: 11 lines in 1 file changed: 7 ins; 2 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/9082.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9082/head:pull/9082

PR: https://git.openjdk.org/jdk/pull/9082