On Wed, 17 Jun 2026 11:10:43 GMT, Viktor Klang <[email protected]> wrote:

>> David Simms has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 2822 commits:
>> 
>>  - Merge branch '8317277' into jep401_sub_review_8317279
>>  - Merge remote-tracking branch 'valhalla/lworld' into 8317277
>>  - 8386690: [lworld] cherry-pick JDK-8386124 Test 
>> serviceability/sa/TestG1HeapRegion.java failed: Address of G1HeapRegion does 
>> not match
>>    
>>    Reviewed-by: liach
>>  - 8385743: [lworld] investigate and address build related comments in the 
>> Valhalla PR
>>    
>>    Reviewed-by: liach, erikj
>>  - 8376346: [lworld] Basic Shenandoah support
>>    
>>    Reviewed-by: qamai
>>  - 8386598: [lworld] C1 acmp profiling fix and minor cleanup
>>    
>>    Reviewed-by: mchevalier
>>  - 8386618: [lworld] Remove unused entry_guard_Relocation
>>    
>>    Reviewed-by: chagedorn, jsjolen, jsikstro
>>  - Merge
>>    
>>    Merge jdk-28+2
>>  - 8386623: [lworld] Cleanup module support for system modules in JDK 
>> exploded build with preview resources
>>    
>>    Reviewed-by: liach
>>  - 8386316: [lworld] Replace Valhalla since version with 28
>>    
>>    Reviewed-by: darcy
>>  - ... and 2812 more: https://git.openjdk.org/jdk/compare/6def7d55...426cc065
>
> src/java.base/share/classes/java/io/Externalizable.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 1996, 2020, Oracle and/or its affiliates. All rights 
>> reserved.
> 
> It has likely been stated before, but unless it hasn't, this file (and 
> others) needs a copyright year update (go through the changed files and 
> ensure that their copyright year reflects 2026).

We decided that the copyright year reflects the last year a file is updated in 
the lworld branch instead of the year of this PR. @MrSimms can you confirm?

> src/java.base/share/classes/java/lang/Class.java line 354:
> 
>> 352:                         }
>> 353:                         if (isRecord()) {
>> 354:                             sb.append("record");
> 
> What about the scenario where it is a value record?

You see these are chains, so this can print "value record". Did you mistake 
this as an else-if?

> src/java.base/share/classes/java/lang/invoke/ArrayVarHandle.java line 36:
> 
>> 34: import static java.lang.invoke.MethodHandleStatics.UNSAFE;
>> 35: 
>> 36: /// The var handle for polymorphic arrays.
> 
> Just a thought—this class might benefit from a @TrustFinalFields (needs to be 
> measured of course)

java.lang.invoke is a trusted package, so not necessary so far.

> src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java
>  line 33:
> 
>> 31: import java.lang.classfile.Label;
>> 32: import java.lang.classfile.attribute.StackMapTableAttribute;
>> 33: import java.lang.classfile.constantpool.*;
> 
> Is the *-import intentional?

Yes, I think we are okay with star imports. We can introduce an all-module 
formatter and fix all the imports/spacing later if necessary.

> src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 1743:
> 
>> 1741:                 }
>> 1742:             }
>> 1743:         } else {
> 
> Is this conditional verified to be dead-code-eliminated if the program isn't 
> in preview mode? Otherwise I could see this causing performance regressions.

Code has been updated to check the class in 
https://github.com/openjdk/valhalla/pull/2576; in non-preview the class check 
can probably eliminate the true branch.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31123#discussion_r3467172096
PR Review Comment: https://git.openjdk.org/jdk/pull/31123#discussion_r3467193367
PR Review Comment: https://git.openjdk.org/jdk/pull/31123#discussion_r3467179053
PR Review Comment: https://git.openjdk.org/jdk/pull/31123#discussion_r3467201590
PR Review Comment: https://git.openjdk.org/jdk/pull/31123#discussion_r3467208484

Reply via email to