On Fri, 6 Jun 2025 07:14:21 GMT, Radim Vansa <rva...@openjdk.org> wrote:

>> This optimization is a followup to https://github.com/openjdk/jdk/pull/24290 
>> trying to reduce the performance regression in some scenarios introduced in 
>> https://bugs.openjdk.org/browse/JDK-8292818 . Based both on performance and 
>> memory consumption it is a (better) alternative to 
>> https://github.com/openjdk/jdk/pull/24713 .
>> 
>> This PR optimizes local field lookup in classes with more than 16 fields; 
>> rather than sequentially iterating through all fields during lookup we sort 
>> the fields based on the field name. The stream includes extra table after 
>> the field information: for field at position 16, 32 ... we record the 
>> (variable-length-encoded) offset of the field info in this stream. On field 
>> lookup, rather than iterating through all fields, we iterate through this 
>> table, resolve names for given fields and continue field-by-field iteration 
>> only after the last record (hence at most 16 fields).
>> 
>> In classes with <= 16 fields this PR reduces the memory consumption by 1 
>> byte that was left with value 0 at the end of stream. In classes with > 16 
>> fields we add extra 4 bytes with offset of the table, and the table contains 
>> one varint for each 16 fields. The terminal byte is not used either.
>> 
>> My measurements on the attached reproducer
>> 
>> hyperfine -w 50 -r 100 '/path/to/jdk-17/bin/java -cp /tmp CCC'
>> Benchmark 1: /path/to/jdk-17/bin/java -cp /tmp CCC
>>   Time (mean ± σ):      51.3 ms ±   2.8 ms    [User: 44.7 ms, System: 13.7 
>> ms]
>>   Range (min … max):    45.1 ms …  53.9 ms    100 runs
>> 
>> hyperfine -w 50 -r 100 '/path/to/jdk25-master/bin/java -cp /tmp CCC'
>> Benchmark 1: /path/to/jdk25-master/bin/java -cp /tmp CCC
>>   Time (mean ± σ):      78.2 ms ±   1.0 ms    [User: 74.6 ms, System: 17.3 
>> ms]
>>   Range (min … max):    73.8 ms …  79.7 ms    100 runs
>> 
>> (the jdk25-master above already contains JDK-8353175)
>> 
>> hyperfine -w 50 -r 100 '/path/to/jdk25-this-pr/bin/java -cp /tmp CCC'
>> Benchmark 1: /path/to/jdk25-this-pr/jdk/bin/java -cp /tmp CCC
>>   Time (mean ± σ):      38.5 ms ±   0.5 ms    [User: 34.4 ms, System: 17.3 
>> ms]
>>   Range (min … max):    37.7 ms …  42.1 ms    100 runs
>> 
>> While https://github.com/openjdk/jdk/pull/24713 returned the performance to 
>> previous levels, this PR improves it by 25% compared to JDK 17 (which does 
>> not contain the regression)! This time, the undisclosed production-grade 
>> reproducer shows even higher improvement:
>> 
>> JDK 17: 1.6 s
>> JDK 21 (no patches): 22 s
>> JDK25-master: 12.3 s
>> JDK25-this-pr: 0.5 s
>
> Radim Vansa has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add more comments
>  - Disable search table with dynamic CDS

To integrate hotspot changes, you need two reviewers and people 'requesting 
changes' to withdraw their requests.  Thank goodness the bots prevented this 
from being integrated.  You need to wait for all the comments to be resolved.
This is a P3 bug so you have more time to get this integrated for JDK 25.  I 
posted the schedule in the issue.  The process is that this change would be 
integrated into the main repository (destined for JDK 26 and then 
slash-backported to JDK 25 a couple days later if testing is clean).
My tier 1-7 testing passes with the dynamic CDS patch above.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/24847#issuecomment-2948894141

Reply via email to