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

Changes from 7 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
54 changes: 42 additions & 12 deletions make/modules/java.base/gensrc/GensrcVarHandles.gmk
View file
Original file line number Diff line number Diff line change
@@ -39,43 +39,73 @@ 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
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
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
endif

ifeq ($$($1_InputType), NonAtomicReference)
$1_ARGS += -KReference
$1_ARGS += -KStatic
$1_Type := Reference
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)
@@ -294,7 +324,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)))

6 changes: 5 additions & 1 deletion src/java.base/share/classes/java/lang/invoke/VarHandle.java
View file
Original file line number Diff line number Diff line change
@@ -508,7 +508,11 @@ public abstract sealed class VarHandle implements Constable
VarHandleShorts.FieldStaticReadOnly,
VarHandleFlatValues.Array,
VarHandleFlatValues.FieldInstanceReadOnly,
VarHandleFlatValues.FieldStaticReadOnly {
VarHandleNonAtomicReferences.Array,
VarHandleNonAtomicReferences.FieldInstanceReadOnly,
VarHandleNonAtomicReferences.FieldStaticReadOnly,
VarHandleNonAtomicFlatValues.Array,
VarHandleNonAtomicFlatValues.FieldInstanceReadOnly {
final VarForm vform;
final boolean exact;

123 changes: 105 additions & 18 deletions src/java.base/share/classes/java/lang/invoke/VarHandles.java
View file
Original file line number Diff line number Diff line change
@@ -25,9 +25,13 @@

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;
import java.lang.reflect.AccessFlag;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
@@ -50,11 +54,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())
@@ -119,12 +143,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())
@@ -176,6 +205,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,
@@ -206,6 +265,39 @@ 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 VarHandle flatArrayElementHandleFor(Object array) {
assert ValueClass.isFlatArray(array);
boolean isAtomic = isAtomicFlat(array);
return isAtomic ?
ATOMIC_FLAT_ARRAY_VAR_HANDLE.get(array.getClass()) :
NON_ATOMIC_FLAT_ARRAY_VAR_HANDLE.get(array.getClass());
}

private static final ClassValue<VarHandle> ATOMIC_FLAT_ARRAY_VAR_HANDLE = new ClassValue<>() {
@Override
protected VarHandle computeValue(Class<?> arrayClass) {
// Todo: eventually, this code should not be class-based
int aoffset = (int) UNSAFE.arrayBaseOffset(arrayClass);
int ascale = UNSAFE.arrayIndexScale(arrayClass);
int ashift = 31 - Integer.numberOfLeadingZeros(ascale);
int layout = UNSAFE.arrayLayout(arrayClass);
return new VarHandleFlatValues.Array(aoffset, ashift, arrayClass, layout);
}
};

private static final ClassValue<VarHandle> NON_ATOMIC_FLAT_ARRAY_VAR_HANDLE = new ClassValue<>() {
@Override
protected VarHandle computeValue(Class<?> arrayClass) {
// Todo: eventually, this code should not be class-based
int aoffset = (int) UNSAFE.arrayBaseOffset(arrayClass);
int ascale = UNSAFE.arrayIndexScale(arrayClass);
int ashift = 31 - Integer.numberOfLeadingZeros(ascale);
int layout = UNSAFE.arrayLayout(arrayClass);
return new VarHandleNonAtomicFlatValues.Array(aoffset, ashift, arrayClass, layout);
}
};

static VarHandle makeArrayElementHandle(Class<?> arrayClass) {
if (!arrayClass.isArray())
throw new IllegalArgumentException("not an array: " + arrayClass);
@@ -217,14 +309,9 @@ 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);
// do not check for atomicity now -- atomicity will be checked on the accessed array object
// a sharper var handle will be obtained (dynamically) from flatArrayElementHandleFor(Object)
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));
Loading