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.

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

8267763: [lworld][lw3] Change "non-tearable" nomenclature to "access atomic" #428

Open
wants to merge 2 commits into
base: lworld
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -6603,14 +6603,14 @@ void ClassFileParser::post_process_parsed_stream(const ClassFileStream* const st
}
}

if (_class_name == vmSymbols::java_lang_NonTearable() && _loader_data->class_loader() == NULL) {
if (_class_name == vmSymbols::java_lang_AtomicAccess() && _loader_data->class_loader() == NULL) {
// This is the original source of this condition.
// It propagates by inheritance, as if testing "instanceof NonTearable".
// It propagates by inheritance, as if testing "instanceof AtomicAccess".
_is_declared_atomic = true;
} else if (*ForceNonTearable != '\0') {
} else if (*ForceAtomicAccess != '\0') {
// Allow a command line switch to force the same atomicity property:
const char* class_name_str = _class_name->as_C_string();
if (StringUtils::class_list_match(ForceNonTearable, class_name_str)) {
if (StringUtils::class_list_match(ForceAtomicAccess, class_name_str)) {
_is_declared_atomic = true;
}
}
@@ -66,7 +66,7 @@
template(java_lang_Thread, "java/lang/Thread") \
template(java_lang_ThreadGroup, "java/lang/ThreadGroup") \
template(java_lang_Cloneable, "java/lang/Cloneable") \
template(java_lang_NonTearable, "java/lang/NonTearable") \
template(java_lang_AtomicAccess, "java/lang/AtomicAccess") \
template(java_lang_Throwable, "java/lang/Throwable") \
template(java_lang_ClassLoader, "java/lang/ClassLoader") \
template(java_lang_ThreadDeath, "java/lang/ThreadDeath") \
@@ -286,7 +286,7 @@ class InstanceKlass: public Klass {
_misc_has_inline_type_fields = 1 << 16, // has inline fields and related embedded section is not empty
_misc_is_empty_inline_type = 1 << 17, // empty inline type (*)
_misc_is_naturally_atomic = 1 << 18, // loaded/stored in one instruction
_misc_is_declared_atomic = 1 << 19, // implements jl.NonTearable
_misc_is_declared_atomic = 1 << 19, // implements j.l.AtomicAccess
_misc_invalid_inline_super = 1 << 20, // invalid super type for an inline type
_misc_invalid_identity_super = 1 << 21, // invalid super type for an identity type
_misc_has_injected_identityObject = 1 << 22, // IdentityObject has been injected by the JVM
@@ -446,11 +446,11 @@ class InstanceKlass: public Klass {
_misc_flags |= _misc_is_naturally_atomic;
}

// Query if this class implements jl.NonTearable or was
// mentioned in the JVM option ForceNonTearable.
// Query if this class implements j.l.AtomicAccess or was
// mentioned in the JVM option ForceAtomicAccess.
// This bit can occur anywhere, but is only significant
// for inline classes *and* their super types.
// It inherits from supers along with NonTearable.
// It inherits from supers along with AtomicAccess.
bool is_declared_atomic() const {
return (_misc_flags & _misc_is_declared_atomic) != 0;
}
@@ -2077,8 +2077,8 @@ const intx ObjectAlignmentInBytes = 8;
product(bool, UseArrayMarkWordCheck, NOT_LP64(false) LP64_ONLY(true), \
"Use bits in the mark word to check for flat/null-free arrays") \
\
product(ccstrlist, ForceNonTearable, "", DIAGNOSTIC, \
"List of inline classes which are forced to be atomic " \
product(ccstrlist, ForceAtomicAccess, "", DIAGNOSTIC, \
"List of inline classes which are forced to be atomic access " \
"(whitespace and commas separate names, " \
"and leading and trailing stars '*' are wildcards)") \
\
Copy link
Collaborator

@rose00 rose00 Jun 14, 2021

Choose a reason for hiding this comment

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

Internally to the JVM, the term "atomic" is acceptable, as long as
in context it is correctly understood as single-access atomicity.
(Which it is.) The product flag here is at the boundary between
the JVM internals and the end-user (who sets the JVM flag on
the command line). At that point, it is better to use the external
term NonTearable, which (for pedagogical reasons, as explained
elsewhere) does not use the word "atomic" but rather a synonym.

@@ -26,38 +26,52 @@
package java.lang;

/**
* A primitive class implements the {@code NonTearable} interface to
* request that the JVM take extra care to avoid structure tearing
* A primitive class implements the {@code AtomicAccess} interface to
* request that the JVM take extra care to avoid non-atomic operations
* when loading or storing any value of the class to a field or array
* element. Normally, only fields declared {@code volatile} are
* protected against structure tearing, but a class that implements
* this marker interface will never have its values torn, even when
* element. Normally, only naturally atomic fields and fields declared
* {@code volatile} are always atomic, but a class that implements
* this marker interface will always be accessed atomically, even when
* they are stored in array elements or in non-{@code volatile}
* fields, and even when multiple threads perform racing writes.
*
* <p> An primitive instance of multiple components is said to be "torn"
* when two racing threads compete to write those components, and one
* thread writes some components while another thread writes other
* components, so a subsequent observer will read a hybrid composed,
* as if "out of thin air", of field values from both racing writes.
* Tearing can also occur when the effects of two non-racing writes
* are observed by a racing read. In general, structure tearing
* requires a read and two writes (initialization counting as a write)
* of a multi-component value, with a race between any two of the
* accesses. The effect can also be described as if the Java memory
* model break up primitive classinstance reads and writes into reads and
* <p> A primitive instance of multiple components can experience both
* transient and persistent access atomicity failures.
*
* <p> Transient failures usually arise from write-read conflicts:
* when one thread writes the components one-by-one, and another
* thread reads the components one-by-one. In doing so, reader
* observes the intermediate state of the primitive instance.
* This failure is transient, since "after" (in memory model sense)
* the writer finishes its writes, the instance is observed in full
* by any subsequent observer.
*
* <p> Permanent failures usually arise from write-write conflicts:
* when two threads compete to write the components, and one thread
* writes some components while another thread writes other
* components. This failure is permanent: as every subsequent observer
* will read a hybrid composed of field values from both racing writes.
*
* Both these effects can be described as if the Java memory model
* break up primitive class instance reads and writes into reads and
* writes of their various fields, as it does with longs and doubles
* (JLS 17.7).
*
* <p> In extreme cases, the hybrid observed after structure tearing
* <p> In extreme cases, the hybrid observed under non-atomic access
* might be a value which is impossible to construct by normal means.
* If data integrity or security depends on proper construction,
* the class should be declared as implementing {@code NonTearable}.
* the class should be declared as implementing {@code AtomicAccess}.
*
Copy link
Collaborator

@rose00 rose00 Jun 14, 2021

Choose a reason for hiding this comment

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

This comment is useful, because it distinguishes non-tearable from the
de-facto standard notion (in the JDK) of "atomic", which pertains to
"compound operations". Atomic compound operations relate multiple
states of a variable in a consistent sequence sequence. This is a
strictly stronger notion than access atomicity or non-tearability.

I support adding such a comment here, but also note that its
presence in the PR is evidence that the type name should
not be changed to AtomicAccess, precisely because
users are likely to confuse this new use of atomic with
existing old uses, all of which imply the stronger concept.

* <p> Note this atomicity guarantee only relates to the <i>individual
* accesses</i>, not the compound operations over the values. The
* read-modify-write operation over {@code AtomicAccess} would still
* be non-atomic, unless specifically written with appropriate
* synchronization.
*
* @author John Rose
* @since (valhalla)
*/
public interface NonTearable {
public interface AtomicAccess {
// TO DO: Finalize name.
// TO DO: Decide whether and how to restrict this type to
// primitive classes only, or if not, whether to document its
@@ -31,27 +31,27 @@
import jdk.internal.misc.Unsafe;

/**
* @test TestBufferTearing
* @test TestBufferAtomicAccess
* @key randomness
* @summary Detect tearing on inline type buffer writes due to missing barriers.
* @summary Detect atomic access violations on inline type buffer writes due to missing barriers.
* @library /testlibrary /test/lib /compiler/whitebox /
* @modules java.base/jdk.internal.misc
* @run main/othervm -XX:InlineFieldMaxFlatSize=0 -XX:FlatArrayElementMaxSize=0
* -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM -XX:+StressLCM
* compiler.valhalla.inlinetypes.TestBufferTearing
* compiler.valhalla.inlinetypes.TestBufferAtomicAccess
* @run main/othervm -XX:InlineFieldMaxFlatSize=0 -XX:FlatArrayElementMaxSize=0
* -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM -XX:+StressLCM
* -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
* compiler.valhalla.inlinetypes.TestBufferTearing
* compiler.valhalla.inlinetypes.TestBufferAtomicAccess
* @run main/othervm -XX:InlineFieldMaxFlatSize=0 -XX:FlatArrayElementMaxSize=0
* -XX:CompileCommand=dontinline,*::incrementAndCheck*
* -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM -XX:+StressLCM
* compiler.valhalla.inlinetypes.TestBufferTearing
* compiler.valhalla.inlinetypes.TestBufferAtomicAccess
* @run main/othervm -XX:InlineFieldMaxFlatSize=0 -XX:FlatArrayElementMaxSize=0
* -XX:CompileCommand=dontinline,*::incrementAndCheck*
* -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM -XX:+StressLCM
* -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
* compiler.valhalla.inlinetypes.TestBufferTearing
* compiler.valhalla.inlinetypes.TestBufferAtomicAccess
*/

primitive class MyValue {
@@ -91,7 +91,7 @@ MyValue incrementAndCheckUnsafe() {
}
}

public class TestBufferTearing {
public class TestBufferAtomicAccess {

static MyValue vtField1;
MyValue vtField2;
@@ -113,9 +113,9 @@ public class TestBufferTearing {
}

static class Runner extends Thread {
TestBufferTearing test;
TestBufferAtomicAccess test;

public Runner(TestBufferTearing test) {
public Runner(TestBufferAtomicAccess test) {
this.test = test;
}

@@ -142,8 +142,9 @@ public void run() {

public static void main(String[] args) throws Exception {
// Create threads that concurrently update some inline type (array) fields
// and check the fields of the inline types for consistency to detect tearing.
TestBufferTearing test = new TestBufferTearing();
// and check the fields of the inline types for consistency to detect access
// atomicity violations.
TestBufferAtomicAccess test = new TestBufferAtomicAccess();
Thread runner = null;
for (int i = 0; i < 10; ++i) {
runner = new Runner(test);
@@ -25,12 +25,12 @@
package compiler.valhalla.inlinetypes;

/**
* @test TestBufferTearingC1
* @test TestBufferAtomicAccessC1
* @key randomness
* @summary Additional tests for C1 missing barriers when buffering inline types.
* @run main/othervm -XX:InlineFieldMaxFlatSize=-1 -XX:FlatArrayElementMaxSize=-1
* -XX:TieredStopAtLevel=1
* compiler.valhalla.inlinetypes.TestBufferTearingC1
* compiler.valhalla.inlinetypes.TestBufferAtomicAccessC1
*/

primitive class Point {
@@ -51,7 +51,7 @@ public Rect(Point a, Point b) {
}
}

public class TestBufferTearingC1 {
public class TestBufferAtomicAccessC1 {

public static Point[] points = new Point[] { new Point(1, 1) };
public static Rect rect = new Rect(new Point(1, 1), new Point(2, 2));
@@ -99,7 +99,7 @@ private static void checkMissingBarrier() {
public static void main(String[] args) throws InterruptedException {
Thread[] threads = new Thread[10];
for (int i = 0; i < 10; i++) {
threads[i] = new Thread(TestBufferTearingC1::checkMissingBarrier);
threads[i] = new Thread(TestBufferAtomicAccessC1::checkMissingBarrier);
threads[i].start();
}

@@ -32,9 +32,9 @@
* @compile -XDallowWithFieldOperator Point.java JumboInline.java
* @compile -XDallowWithFieldOperator FlattenableSemanticTest.java
* @run main/othervm -Xint -XX:InlineFieldMaxFlatSize=64 runtime.valhalla.inlinetypes.FlattenableSemanticTest
* @run main/othervm -Xint -XX:+UnlockDiagnosticVMOptions -XX:ForceNonTearable=* runtime.valhalla.inlinetypes.FlattenableSemanticTest
* @run main/othervm -Xint -XX:+UnlockDiagnosticVMOptions -XX:ForceAtomicAccess=* runtime.valhalla.inlinetypes.FlattenableSemanticTest
* @run main/othervm -Xcomp -XX:InlineFieldMaxFlatSize=64 runtime.valhalla.inlinetypes.FlattenableSemanticTest
* @run main/othervm -Xcomp -XX:+UnlockDiagnosticVMOptions -XX:ForceNonTearable=* runtime.valhalla.inlinetypes.FlattenableSemanticTest
* @run main/othervm -Xcomp -XX:+UnlockDiagnosticVMOptions -XX:ForceAtomicAccess=* runtime.valhalla.inlinetypes.FlattenableSemanticTest
* // debug: -XX:+PrintInlineLayout -XX:-ShowMessageBoxOnError
*/
public class FlattenableSemanticTest {
@@ -39,7 +39,7 @@
* @run main/othervm -Xint -XX:FlatArrayElementMaxSize=0 runtime.valhalla.inlinetypes.InlineTypeArray
* @run main/othervm -Xcomp -XX:FlatArrayElementMaxSize=-1 runtime.valhalla.inlinetypes.InlineTypeArray
* @run main/othervm -Xcomp -XX:FlatArrayElementMaxSize=0 runtime.valhalla.inlinetypes.InlineTypeArray
* @run main/othervm -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:ForceNonTearable=* runtime.valhalla.inlinetypes.InlineTypeArray
* @run main/othervm -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:ForceAtomicAccess=* runtime.valhalla.inlinetypes.InlineTypeArray
*/
public class InlineTypeArray {
public static void main(String[] args) {
@@ -43,14 +43,14 @@
* -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
* -XX:+WhiteBoxAPI InlineTypeDensity
* @run main/othervm -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:FlatArrayElementMaxSize=-1
* -Xbootclasspath/a:. -XX:ForceNonTearable=*
* -Xbootclasspath/a:. -XX:ForceAtomicAccess=*
* -XX:+WhiteBoxAPI InlineTypeDensity
*/

public class InlineTypeDensity {

private static final WhiteBox WHITE_BOX = WhiteBox.getWhiteBox();
private static final boolean VM_FLAG_FORCENONTEARABLE = WHITE_BOX.getStringVMFlag("ForceNonTearable").equals("*");
private static final boolean VM_FLAG_FORCEATOMICACCESS = WHITE_BOX.getStringVMFlag("ForceAtomicAccess").equals("*");

public InlineTypeDensity() {
if (WHITE_BOX.getIntxVMFlag("FlatArrayElementMaxSize") != -1) {
@@ -306,7 +306,7 @@ void testAlignedSize() {
public void test() {
ensureArraySizeWin();
testPrimitiveArraySizesSame();
if (!VM_FLAG_FORCENONTEARABLE) {
if (!VM_FLAG_FORCEATOMICACCESS) {
testAlignedSize();
}
}
@@ -64,7 +64,7 @@
* -XX:+ExplicitGCInvokesConcurrent
* -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
* -Djava.lang.invoke.MethodHandle.DUMP_CLASS_FILES=false
* -XX:ForceNonTearable=*
* -XX:ForceAtomicAccess=*
* runtime.valhalla.inlinetypes.InlineTypesTest
*/
public class InlineTypesTest {