Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8351569: [lworld] Revisit atomic access modes in flat var handles #1402

Closed
wants to merge 17 commits into from
Closed

8351569: [lworld] Revisit atomic access modes in flat var handles #1402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
31fb09c
Initial push
mcimadamore Mar 11, 2025
7bd00e1
Reintroduce assertion that static fields can't be flat
mcimadamore Mar 11, 2025
e845193
Refactor flat value var handle implementation
mcimadamore Mar 18, 2025
874e4cb
Make atomic var handle support depend on user-visible properties
mcimadamore Mar 19, 2025
d4af8c1
Fix missing fence in `putFlatValueVolatile`
mcimadamore Mar 20, 2025
856cb9a
Remove redundant import
mcimadamore Mar 20, 2025
8ca6bef
Enhance compareAndSetFlatValueAsBytes to work on different kinds of f…
mcimadamore Mar 20, 2025
1cd8176
Fix test header
mcimadamore Mar 20, 2025
70af9a2
Avoid dynamic var handle creation in the middle of array element access
mcimadamore Mar 20, 2025
0e2eff2
Tighten the signature of VarHandles::checkAtomicFlatArray
mcimadamore Mar 20, 2025
b87221a
Update src/java.base/share/classes/jdk/internal/misc/Unsafe.java
mcimadamore Mar 21, 2025
7ddd549
Update src/java.base/share/classes/jdk/internal/misc/Unsafe.java
mcimadamore Mar 21, 2025
6b1b379
Add missing `@ForceInline`
mcimadamore Mar 21, 2025
be6f00d
Merge branch 'lworld' into varhandle_flat
mcimadamore Mar 21, 2025
f8c7d9b
Simplify Unsafe::compareAndSetFlatValueAsBytes to use new unsafe flat…
mcimadamore Mar 21, 2025
000917a
Address review commments
mcimadamore Mar 21, 2025
a823ba4
Update src/java.base/share/classes/jdk/internal/misc/Unsafe.java
mcimadamore Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 46 additions & 12 deletions make/modules/java.base/gensrc/GensrcVarHandles.gmk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,43 +39,77 @@ VARHANDLES_SRC_DIR := $(MODULE_SRC)/share/classes/java/lang/invoke
# Param 2 - Type with first letter capitalized
define GenerateVarHandle

$1_Type := $2
$1_InputType := $2
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to do a bit of trickery here. Basically we have two pair of input values, which should use the same unsafe access primitives:

  • Reference, NonAtomicReference -> get/putReference
  • FlatValue, NonAtomicFlatValue -> get/putFlatValue

So, we now use InputType to model the input type to the template, and from there we derive Type (which is really the Unsafe access type).

There's also some other changes:

  • Not all generated classes need non-plain operations (e.g. NonAtomicReference, NonAtomicFlatValue do not)
  • Not all generated classes need to handle static fields (e.g. FlatValues and NonAtomicFlatValues do not, as static fields are never flattened)
  • Not all generated classes need to deal with arrays (e.g. FlatValues and NonAtomicFlatValues do not -- access to value arrays is always handled in the Reference impl class)


$1_FILENAME := $(VARHANDLES_GENSRC_DIR)/VarHandle$$($1_Type)s.java
$1_FILENAME := $(VARHANDLES_GENSRC_DIR)/VarHandle$$($1_InputType)s.java

$1_ARGS += -KCAS

ifneq ($$(findstring $$($1_Type), Byte Short Char Int Long Float Double), )
ifneq ($$(findstring $$($1_InputType), Byte Short Char Int Long Float Double), )
$1_ARGS += -KAtomicAdd
$1_ARGS += -KNonPlainAccess
$1_ARGS += -KStatic
$1_ARGS += -KArray
endif

ifneq ($$(findstring $$($1_InputType), Byte Short Char Int Long), )
$1_ARGS += -KBitwise
endif

ifneq ($$(findstring $$($1_Type), Boolean Byte Short Char Int Long), )
ifeq ($$($1_InputType), Boolean)
$1_ARGS += -KBitwise
$1_ARGS += -KNonPlainAccess
$1_ARGS += -KStatic
$1_ARGS += -KArray
endif

ifneq ($$(findstring $$($1_Type), Byte Short Char), )
ifneq ($$(findstring $$($1_InputType), Byte Short Char), )
$1_ARGS += -KShorterThanInt
endif

ifeq ($$($1_Type), Reference)
ifeq ($$($1_InputType), Reference)
$1_ARGS += -KReference
$1_ARGS += -KNonPlainAccess
$1_ARGS += -KStatic
$1_ARGS += -KArray
endif

ifeq ($$($1_InputType), NonAtomicReference)
$1_ARGS += -KReference
$1_ARGS += -KStatic
$1_Type := Reference
$1_ARGS += -KArray
endif

ifeq ($$($1_Type), FlatValue)
ifeq ($$($1_InputType), FlatValue)
$1_ARGS += -KFlatValue
$1_ARGS += -KNonPlainAccess
endif

ifeq ($$($1_InputType), NonAtomicFlatValue)
$1_ARGS += -KFlatValue
endif

$$($1_FILENAME): $(VARHANDLES_SRC_DIR)/X-VarHandle.java.template $(BUILD_TOOLS_JDK)
ifeq ($$($1_Type), Reference)
ifeq ($$($1_InputType), Reference)
$$(eval $1_type := Object)
$$(eval $1_Type := Reference)
else ifeq ($$($1_InputType), NonAtomicReference)
$$(eval $1_type := Object)
$$(eval $1_Type := Reference)
else ifeq ($$($1_InputType), FlatValue)
$$(eval $1_type := Object)
else ifeq ($$($1_Type), FlatValue)
$$(eval $1_Type := FlatValue)
else ifeq ($$($1_InputType), NonAtomicFlatValue)
$$(eval $1_type := Object)
$$(eval $1_Type := FlatValue)
else
$$(eval $1_type := $$$$(shell $(TR) '[:upper:]' '[:lower:]' <<< $$$$($1_Type)))
$$(eval $1_type := $$$$(shell $(TR) '[:upper:]' '[:lower:]' <<< $$$$($1_InputType)))
$$(eval $1_Type := $$$$($1_InputType))
endif
$$(call MakeDir, $$(@D))
$(RM) $$@
$(TOOL_SPP) -nel -K$$($1_type) -Dtype=$$($1_type) -DType=$$($1_Type) \
$(TOOL_SPP) -nel -K$$($1_type) -Dtype=$$($1_type) -DType=$$($1_Type) -DInputType=$$($1_InputType) \
$$($1_ARGS) -i$$< -o$$@

GENSRC_VARHANDLES += $$($1_FILENAME)
Expand Down Expand Up @@ -294,7 +328,7 @@ endef
################################################################################

# List the types to generate source for, with capitalized first letter
VARHANDLES_TYPES := Boolean Byte Short Char Int Long Float Double Reference FlatValue
VARHANDLES_TYPES := Boolean Byte Short Char Int Long Float Double Reference FlatValue NonAtomicReference NonAtomicFlatValue
$(foreach t, $(VARHANDLES_TYPES), \
$(eval $(call GenerateVarHandle,VAR_HANDLE_$t,$t)))

Expand Down
6 changes: 4 additions & 2 deletions src/java.base/share/classes/java/lang/invoke/VarHandle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,9 +506,11 @@ public abstract sealed class VarHandle implements Constable
VarHandleShorts.Array,
VarHandleShorts.FieldInstanceReadOnly,
VarHandleShorts.FieldStaticReadOnly,
VarHandleFlatValues.Array,
VarHandleFlatValues.FieldInstanceReadOnly,
VarHandleFlatValues.FieldStaticReadOnly {
VarHandleNonAtomicReferences.Array,
VarHandleNonAtomicReferences.FieldInstanceReadOnly,
VarHandleNonAtomicReferences.FieldStaticReadOnly,
VarHandleNonAtomicFlatValues.FieldInstanceReadOnly {
final VarForm vform;
final boolean exact;

Expand Down
98 changes: 80 additions & 18 deletions src/java.base/share/classes/java/lang/invoke/VarHandles.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@

package java.lang.invoke;

import jdk.internal.value.NullRestrictedCheckedType;
import jdk.internal.value.ValueClass;
import jdk.internal.vm.annotation.LooselyConsistentValue;
import sun.invoke.util.Wrapper;

import java.lang.foreign.MemoryLayout;
Expand All @@ -50,11 +53,31 @@ static VarHandle makeFieldHandle(MemberName f, Class<?> refc, boolean isWriteAll
long foffset = MethodHandleNatives.objectFieldOffset(f);
Class<?> type = f.getFieldType();
if (!type.isPrimitive()) {
if (f.isFlat()) {
if (type.isValue()) {
int layout = f.getLayout();
return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleFlatValues.FieldInstanceReadOnly(refc, foffset, type, f.getCheckedFieldType(), layout)
: new VarHandleFlatValues.FieldInstanceReadWrite(refc, foffset, type, f.getCheckedFieldType(), layout));
boolean isAtomic = isAtomicFlat(f);
boolean isFlat = f.isFlat();
if (isFlat) {
if (isAtomic) {
return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleFlatValues.FieldInstanceReadOnly(refc, foffset, type, f.getCheckedFieldType(), layout)
: new VarHandleFlatValues.FieldInstanceReadWrite(refc, foffset, type, f.getCheckedFieldType(), layout));
} else {
return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleNonAtomicFlatValues.FieldInstanceReadOnly(refc, foffset, type, f.getCheckedFieldType(), layout)
: new VarHandleNonAtomicFlatValues.FieldInstanceReadWrite(refc, foffset, type, f.getCheckedFieldType(), layout));
}
} else {
if (isAtomic) {
return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleReferences.FieldInstanceReadOnly(refc, foffset, type, f.getCheckedFieldType())
: new VarHandleReferences.FieldInstanceReadWrite(refc, foffset, type, f.getCheckedFieldType()));
} else {
return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this case is not terribly important (typically, if something is non-flat, then it will be "atomic" from a programming model perspective), there are various JVM flags that can affect whether something is flattened or not. It's important here that we don't return a var handles that "overpromises" and support more access modes (just because we know that, underneath, we can use reference Unsafe primitives) as such behavior might not be stable,

? new VarHandleNonAtomicReferences.FieldInstanceReadOnly(refc, foffset, type, f.getCheckedFieldType())
: new VarHandleNonAtomicReferences.FieldInstanceReadWrite(refc, foffset, type, f.getCheckedFieldType()));
}
}
} else {
return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleReferences.FieldInstanceReadOnly(refc, foffset, type, f.getCheckedFieldType())
Expand Down Expand Up @@ -119,12 +142,17 @@ static VarHandle makeStaticFieldVarHandle(Class<?> decl, MemberName f, boolean i
long foffset = MethodHandleNatives.staticFieldOffset(f);
Class<?> type = f.getFieldType();
if (!type.isPrimitive()) {
if (f.isFlat()) {
assert false : ("static field is flat in " + decl + "." + f.getName());
int layout = f.getLayout();
return f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleFlatValues.FieldStaticReadOnly(decl, base, foffset, type, f.getCheckedFieldType(), layout)
: new VarHandleFlatValues.FieldStaticReadWrite(decl, base, foffset, type, f.getCheckedFieldType(), layout);
assert !f.isFlat() : ("static field is flat in " + decl + "." + f.getName());
if (type.isValue()) {
if (isAtomicFlat(f)) {
return f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleReferences.FieldStaticReadOnly(decl, base, foffset, type, f.getCheckedFieldType())
: new VarHandleReferences.FieldStaticReadWrite(decl, base, foffset, type, f.getCheckedFieldType());
} else {
return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleNonAtomicReferences.FieldStaticReadOnly(decl, base, foffset, type, f.getCheckedFieldType())
: new VarHandleNonAtomicReferences.FieldStaticReadWrite(decl, base, foffset, type, f.getCheckedFieldType()));
}
} else {
return f.isFinal() && !isWriteAllowedOnFinalFields
? new VarHandleReferences.FieldStaticReadOnly(decl, base, foffset, type, f.getCheckedFieldType())
Expand Down Expand Up @@ -176,6 +204,36 @@ else if (type == double.class) {
}
}

static boolean isAtomicFlat(MemberName field) {
boolean hasAtomicAccess = (field.getModifiers() & Modifier.VOLATILE) != 0 ||
!(field.getCheckedFieldType() instanceof NullRestrictedCheckedType) ||
!field.getFieldType().isAnnotationPresent(LooselyConsistentValue.class);
return hasAtomicAccess && !HAS_OOPS.get(field.getFieldType());
}

static boolean isAtomicFlat(Object[] array) {
Class<?> componentType = array.getClass().componentType();
boolean hasAtomicAccess = ValueClass.isAtomicArray(array) ||
!ValueClass.isNullRestrictedArray(array) ||
!componentType.isAnnotationPresent(LooselyConsistentValue.class);
return hasAtomicAccess && !HAS_OOPS.get(componentType);
}

static final ClassValue<Boolean> HAS_OOPS = new ClassValue<>() {
@Override
protected Boolean computeValue(Class<?> c) {
for (Field f : c.getDeclaredFields()) {
Class<?> ftype = f.getType();
if (UNSAFE.isFlatField(f) && HAS_OOPS.get(ftype)) {
return true;
} else if (!ftype.isPrimitive()) {
return true;
}
}
return false;
}
};

// Required by instance field handles
static Field getFieldFromReceiverAndOffset(Class<?> receiverType,
long offset,
Expand Down Expand Up @@ -206,6 +264,13 @@ static Field getStaticFieldFromBaseAndOffset(Class<?> declaringClass,
throw new InternalError("Static field not found at offset");
}

// This is invoked by non-flat array var handle code when attempting to access a flat array
public static void checkAtomicFlatArray(Object[] array) {
if (!isAtomicFlat(array)) {
throw new IllegalArgumentException("Attempt to perform a non-plain access on a non-atomic array");
}
}

static VarHandle makeArrayElementHandle(Class<?> arrayClass) {
if (!arrayClass.isArray())
throw new IllegalArgumentException("not an array: " + arrayClass);
Expand All @@ -217,14 +282,11 @@ static VarHandle makeArrayElementHandle(Class<?> arrayClass) {
int ashift = 31 - Integer.numberOfLeadingZeros(ascale);

if (!componentType.isPrimitive()) {
VarHandle vh;
if (UNSAFE.isFlatArray(arrayClass)) {
int layout = UNSAFE.arrayLayout(arrayClass);
vh = new VarHandleFlatValues.Array(aoffset, ashift, arrayClass, layout);
} else {
vh = new VarHandleReferences.Array(aoffset, ashift, arrayClass);
}
return maybeAdapt(vh);
// Here we always return a reference array element var handle. This is because
// the access semantics is determined at runtime, when an actual array object is passed
// to the var handle. The var handle implementation will switch to use flat access
// primitives if it sees a flat array.
return maybeAdapt(new VarHandleReferences.Array(aoffset, ashift, arrayClass));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this works because:

  • the reference Array plain access uses array syntax (which works no matter the array passed in)
  • non-plain access modes always delegate to sharper var handles based on the type of the accessed array

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another note: at the language level there's no mechanism to obtain a class mirror for a null-restricted, or atomic array. This means we can't really create an array var handle that works only on one array shape (which would allow us to eliminate the dynamic dispatch completely, at least for some array var handles).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, now it is crucial for us to benchmark these var handles - I suspect they are very sensitive to profile pollutions.

}
else if (componentType == boolean.class) {
return maybeAdapt(new VarHandleBooleans.Array(aoffset, ashift));
Expand Down
Loading