Re: RFR: 8331114: Further improve performance of MethodTypeDesc::descriptorString [v4]

2024-04-26 Thread Claes Redestad
> When analyzing (startup) performance of the Classfile API I found this 
> opportunity to further improve `MethodTypeDescImpl::descriptorString`.
> 
> Performance improves across the board in existing microbenchmarks:
> 
> Name 
> (descString) Cnt   Base   ErrorTest   Error  Unit  Change
> MethodTypeDescFactories.descriptorString  
> (Ljava/lang/Object;Ljava/lang/String;)I   6 55,179 ± 2,027  32,920 ± 1,189 
> ns/op   1,68x (p = 0,000*)
> MethodTypeDescFactories.descriptorString  
> ()V   6 17,689 ± 1,871  11,060 ± 0,331 ns/op   1,60x (p = 0,000*)
> MethodTypeDescFactories.descriptorString 
> ([IJLjava/lang/String;Z)Ljava/util/List;   6 86,627 ± 1,646  41,035 ± 0,636 
> ns/op   2,11x (p = 0,000*)
> MethodTypeDescFactories.descriptorString
> ()[Ljava/lang/String;   6 18,305 ± 1,974  13,110 ± 0,089 ns/op   1,40x (p = 
> 0,000*)
>   * = significant
> 
> 
> The improvement is even more pronounced when running with `-Xint`, which is 
> relevant for reducing startup overheads of early ClassFile API use:
> 
> Name 
> (descString) Cnt Base  Error  Test Error  Unit  Change
> MethodTypeDescFactories.descriptorString  
> (Ljava/lang/Object;Ljava/lang/String;)I   6 5122,061 ±   81,335  2626,481 ± 
> 101,466 ns/op   1,95x (p = 0,000*)
> MethodTypeDescFactories.descriptorString  
> ()V   6 3481,316 ±  258,904  1489,267 ±  15,506 ns/op   2,34x (p = 0,000*)
> MethodTypeDescFactories.descriptorString 
> ([IJLjava/lang/String;Z)Ljava/util/List;   6 7741,081 ± 1628,244  3281,778 ±  
> 41,892 ns/op   2,36x (p = 0,000*)
> MethodTypeDescFactories.descriptorString
> ()[Ljava/lang/String;   6 3677,803 ±   63,432  1495,291 ±   8,995 ns/op   
> 2,46x (p = 0,000*)
>   * = significant
>   ```
>   
>  I also applied similar approach to `MethodTypeDesc::displayDescriptor`: 
> while not performance sensitive I think these are so inter-related that it 
> makes sense to implement them in a similar fashion.

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

  Use returnType field directly

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18945/files
  - new: https://git.openjdk.org/jdk/pull/18945/files/15c8d39c..6ff505f6

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18945&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18945&range=02-03

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

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


Re: RFR: 8331114: Further improve performance of MethodTypeDesc::descriptorString [v4]

2024-04-26 Thread Mandy Chung
On Fri, 26 Apr 2024 08:07:04 GMT, Claes Redestad  wrote:

>> When analyzing (startup) performance of the Classfile API I found this 
>> opportunity to further improve `MethodTypeDescImpl::descriptorString`.
>> 
>> Performance improves across the board in existing microbenchmarks:
>> 
>> Name 
>> (descString) Cnt   Base   ErrorTest   Error  Unit  Change
>> MethodTypeDescFactories.descriptorString  
>> (Ljava/lang/Object;Ljava/lang/String;)I   6 55,179 ± 2,027  32,920 ± 1,189 
>> ns/op   1,68x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>>  ()V   6 17,689 ± 1,871  11,060 ± 0,331 ns/op   1,60x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>> ([IJLjava/lang/String;Z)Ljava/util/List;   6 86,627 ± 1,646  41,035 ± 0,636 
>> ns/op   2,11x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString
>> ()[Ljava/lang/String;   6 18,305 ± 1,974  13,110 ± 0,089 ns/op   1,40x (p = 
>> 0,000*)
>>   * = significant
>> 
>> 
>> The improvement is even more pronounced when running with `-Xint`, which is 
>> relevant for reducing startup overheads of early ClassFile API use:
>> 
>> Name 
>> (descString) Cnt Base  Error  Test Error  Unit  Change
>> MethodTypeDescFactories.descriptorString  
>> (Ljava/lang/Object;Ljava/lang/String;)I   6 5122,061 ±   81,335  2626,481 ± 
>> 101,466 ns/op   1,95x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>>  ()V   6 3481,316 ±  258,904  1489,267 ±  15,506 ns/op   2,34x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>> ([IJLjava/lang/String;Z)Ljava/util/List;   6 7741,081 ± 1628,244  3281,778 ± 
>>  41,892 ns/op   2,36x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString
>> ()[Ljava/lang/String;   6 3677,803 ±   63,432  1495,291 ±   8,995 ns/op   
>> 2,46x (p = 0,000*)
>>   * = significant
>>   ```
>>   
>>  I also applied similar approach to `MethodTypeDesc::displayDescriptor`: 
>> while not performance sensitive I think these are so inter-related that it 
>> makes sense to implement them in a similar fashion.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use returnType field directly

I am glad that this only focuses on the descriptor string as 
`MethodTypeDesc::displayDescriptor` is typically used for debugging and 
exception message and not performance-sensitive.

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18945#pullrequestreview-2025627526


Re: RFR: 8331114: Further improve performance of MethodTypeDesc::descriptorString [v4]

2024-04-26 Thread Chen Liang
On Fri, 26 Apr 2024 08:07:04 GMT, Claes Redestad  wrote:

>> When analyzing (startup) performance of the Classfile API I found this 
>> opportunity to further improve `MethodTypeDescImpl::descriptorString`.
>> 
>> Performance improves across the board in existing microbenchmarks:
>> 
>> Name 
>> (descString) Cnt   Base   ErrorTest   Error  Unit  Change
>> MethodTypeDescFactories.descriptorString  
>> (Ljava/lang/Object;Ljava/lang/String;)I   6 55,179 ± 2,027  32,920 ± 1,189 
>> ns/op   1,68x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>>  ()V   6 17,689 ± 1,871  11,060 ± 0,331 ns/op   1,60x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>> ([IJLjava/lang/String;Z)Ljava/util/List;   6 86,627 ± 1,646  41,035 ± 0,636 
>> ns/op   2,11x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString
>> ()[Ljava/lang/String;   6 18,305 ± 1,974  13,110 ± 0,089 ns/op   1,40x (p = 
>> 0,000*)
>>   * = significant
>> 
>> 
>> The improvement is even more pronounced when running with `-Xint`, which is 
>> relevant for reducing startup overheads of early ClassFile API use:
>> 
>> Name 
>> (descString) Cnt Base  Error  Test Error  Unit  Change
>> MethodTypeDescFactories.descriptorString  
>> (Ljava/lang/Object;Ljava/lang/String;)I   6 5122,061 ±   81,335  2626,481 ± 
>> 101,466 ns/op   1,95x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>>  ()V   6 3481,316 ±  258,904  1489,267 ±  15,506 ns/op   2,34x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>> ([IJLjava/lang/String;Z)Ljava/util/List;   6 7741,081 ± 1628,244  3281,778 ± 
>>  41,892 ns/op   2,36x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString
>> ()[Ljava/lang/String;   6 3677,803 ±   63,432  1495,291 ±   8,995 ns/op   
>> 2,46x (p = 0,000*)
>>   * = significant
>>   ```
>>   
>>  I also applied similar approach to `MethodTypeDesc::displayDescriptor`: 
>> while not performance sensitive I think these are so inter-related that it 
>> makes sense to implement them in a similar fashion.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use returnType field directly

Marked as reviewed by liach (Author).

Marked as reviewed by liach (Author).

-

PR Review: https://git.openjdk.org/jdk/pull/18945#pullrequestreview-2025741971
PR Review: https://git.openjdk.org/jdk/pull/18945#pullrequestreview-2025743039


Re: RFR: 8331114: Further improve performance of MethodTypeDesc::descriptorString [v4]

2024-04-26 Thread Claes Redestad
On Fri, 26 Apr 2024 18:21:16 GMT, Mandy Chung  wrote:

> I am glad that this only focuses on the descriptor string as 
> `MethodTypeDesc::displayDescriptor` is typically used for debugging and 
> exception message and not performance-sensitive.

Yes, which is why I'd really like to desugar it: this is the method where I 
tripped on a bootstrap circularity when print-debugging an issue in the lambda 
code generator. It's no fun at all hitting bootstrapping issues when calling 
innocuous methods in `java.lang.*`

Obviously the performance of it is not important - I was just stringing it 
along for consistency.

-

PR Comment: https://git.openjdk.org/jdk/pull/18945#issuecomment-2080016093


Re: RFR: 8331114: Further improve performance of MethodTypeDesc::descriptorString [v4]

2024-04-26 Thread Chen Liang
On Fri, 26 Apr 2024 08:07:04 GMT, Claes Redestad  wrote:

>> When analyzing (startup) performance of the Classfile API I found this 
>> opportunity to further improve `MethodTypeDescImpl::descriptorString`.
>> 
>> Performance improves across the board in existing microbenchmarks:
>> 
>> Name 
>> (descString) Cnt   Base   ErrorTest   Error  Unit  Change
>> MethodTypeDescFactories.descriptorString  
>> (Ljava/lang/Object;Ljava/lang/String;)I   6 55,179 ± 2,027  32,920 ± 1,189 
>> ns/op   1,68x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>>  ()V   6 17,689 ± 1,871  11,060 ± 0,331 ns/op   1,60x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>> ([IJLjava/lang/String;Z)Ljava/util/List;   6 86,627 ± 1,646  41,035 ± 0,636 
>> ns/op   2,11x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString
>> ()[Ljava/lang/String;   6 18,305 ± 1,974  13,110 ± 0,089 ns/op   1,40x (p = 
>> 0,000*)
>>   * = significant
>> 
>> 
>> The improvement is even more pronounced when running with `-Xint`, which is 
>> relevant for reducing startup overheads of early ClassFile API use:
>> 
>> Name 
>> (descString) Cnt Base  Error  Test Error  Unit  Change
>> MethodTypeDescFactories.descriptorString  
>> (Ljava/lang/Object;Ljava/lang/String;)I   6 5122,061 ±   81,335  2626,481 ± 
>> 101,466 ns/op   1,95x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>>  ()V   6 3481,316 ±  258,904  1489,267 ±  15,506 ns/op   2,34x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString 
>> ([IJLjava/lang/String;Z)Ljava/util/List;   6 7741,081 ± 1628,244  3281,778 ± 
>>  41,892 ns/op   2,36x (p = 0,000*)
>> MethodTypeDescFactories.descriptorString
>> ()[Ljava/lang/String;   6 3677,803 ±   63,432  1495,291 ±   8,995 ns/op   
>> 2,46x (p = 0,000*)
>>   * = significant
>>   ```
>>   
>>  I also applied similar approach to `MethodTypeDesc::displayDescriptor`: 
>> while not performance sensitive I think these are so inter-related that it 
>> makes sense to implement them in a similar fashion.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use returnType field directly

test/micro/org/openjdk/bench/java/lang/constant/MethodTypeDescFactories.java 
line 1:

> 1: /*

You can update the copyright of this file if you don't mind.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18945#discussion_r1581519592