On Wed, 24 Jun 2026 02:26:29 GMT, Chen Liang <[email protected]> wrote:

> 1. `weakCompareAndSetReferenceXxx` in `Unsafe` were delegating to wrong 
> methods
> 2. Replace use of `0` for reference layout with `Unsafe.NON_FLAT_LAYOUT`
> 3. Replace use of `31 - Integer.numberOfLeadingZeros(ascale)` with 
> `numberOfTrailingZeros`
> 4. Remove null check for VarHandle getting from NR fields (parity with 
> ArrayVarHandle and DirectMethodHandle), add missing null check for flat value 
> var handles
> 5. Remove the weird directives and old flags of RecursiveValueClass
> 
> Thanks to @JornVernee and @AlanBateman for spotting these issues.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

LGTM (after comments being handled)

src/java.base/share/classes/java/io/ObjectStreamClass.java line 1936:

> 1934:                     key : Unsafe.INVALID_FIELD_OFFSET;
> 1935:                 offsets[i] = f.getOffset();
> 1936:                 layouts[i] = rf != null && !f.isPrimitive() ? 
> UNSAFE.fieldLayout(rf) : Unsafe.NON_FLAT_LAYOUT;

nice

src/java.base/share/classes/java/lang/invoke/ArrayVarHandle.java line 44:

> 42: /// This var handle must check each incoming array instance to determine 
> the
> 43: /// layout and the correct operation for that array.
> 44: final class ArrayVarHandle extends VarHandle {

Given the repetition of logic, and the risk of missing some change down the 
line, at what point do we consider using a template to generate this file?

src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 1723:

> 1721:     private final boolean isValueObject(Class<?> valueType, Object o) {
> 1722:         return ValueClass.isValueObjectCompatible(valueType)
> 1723:                 && o != null && o.getClass().isValue();

I suspect having the null-check first is going to be cheaper?

src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 1854:

> 1852:                                                              V x) {
> 1853:         if (isValueObject(valueType, expected)) {
> 1854:             return compareAndSetReference(o, offset, valueType, 
> expected, x);

My preference would be that all of the accesses that are stronger than 
necessary have a small comment stating that they are stronger than necessary 
until an appropriately-weaker implementation is added. This is to make it clear 
that the code is intentional, and under which circumstances it could/should 
change.

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

PR Review: 
https://git.openjdk.org/valhalla/pull/2576#pullrequestreview-4562500912
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2576#discussion_r3467311769
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2576#discussion_r3467318876
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2576#discussion_r3467328381
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2576#discussion_r3467343224

Reply via email to