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

Conversation

shipilev
Copy link
Member

@shipilev shipilev commented May 26, 2021

Current Valhalla code has the experimental marker interface java.lang.NonTearable, which is actually about access atomicity. It makes weird claims about word tearing and out-of-thin air values.

First, this is not word tearing. Word tearing, as defined by JLS 17.6 is: "This problem is sometimes known as word tearing, and on processors that cannot easily update a single byte in isolation some other approach will be required". That is, word tearing is when we cannot update the narrow member without doing a wider access, thus necessarily affecting the adjacent members. In Valhalla case, what we are dealing with is access atomicity: we sometimes cannot access the wide member without doing a set of narrower accesses. This is why JLS 17.7 says "non-atomic treatment of double and longs", not "word-tearing of double and longs".

Second, the docs for j.l.NonTearable mention "out-of-thin-air" (OOTA) values, which are not related here at all. OOTA are the beasts from the causality loops: those are values that were never written by normal execution of the program (i.e. speculative values). In Valhalla case, the writes that produce the broken hybrid are known and expected writes from the conflicting writers.

This nomenclature percolates to Valhalla VM code, so some change is needed there as well.

Additional testing:

  • runtime/valhalla tests

Progress

  • Change must not contain extraneous whitespace

Issue

  • JDK-8267763: [lworld][lw3] Change "non-tearable" nomenclature to "access atomic"

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/valhalla pull/428/head:pull/428
$ git checkout pull/428

Update a local copy of the PR:
$ git checkout pull/428
$ git pull https://git.openjdk.java.net/valhalla pull/428/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 428

View PR using the GUI difftool:
$ git pr show -t 428

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/valhalla/pull/428.diff

@bridgekeeper
Copy link

bridgekeeper bot commented May 26, 2021

👋 Welcome back shade! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented May 26, 2021

@shipilev This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

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

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 3 new commits pushed to the lworld branch:

  • 44cb67e: 8267818: [lworld] [AArch64] Shenandoah barrier set build warnings and register conflict
  • 0871590: 8266890: [lworld] [AArch64] add support for InlineTypePassFieldsAsArgs
  • 0f1c33c: 8267710: [lworld][lw3] Hook AlwaysAtomicAccesses to primitive classes atomicity rules

Please see this link for an up-to-date comparison between the source branch of this pull request and the lworld branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@TobiHartmann) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@mlbridge
Copy link

mlbridge bot commented May 26, 2021

Webrevs

@shipilev shipilev changed the title [lworld][lw3] Change "non-tearable" nomenclature to "access atomic" 8267763: [lworld][lw3] Change "non-tearable" nomenclature to "access atomic" May 26, 2021
Copy link
Member

@TobiHartmann TobiHartmann left a comment

The corresponding tests in compiler/valhalla/inlinetypes should be renamed as well (see TestBufferTearing.java and TestBufferTearingC1.java).

@shipilev
Copy link
Member Author

shipilev commented May 31, 2021

Renamed other "tearing" tests too now. compiler/valhalla passes with Linux x86_64 fastdebug builds.

@TobiHartmann
Copy link
Member

TobiHartmann commented May 31, 2021

Looks good to me but since most of this touches runtime code, someone from runtime should review.

@rose00
Copy link
Collaborator

rose00 commented Jun 14, 2021

Thank you for explaining to all of us about word-tearing, but that seems to be moot in the context of this PR. This feature (NonTearable) is about structure tearing, which is a distinct concept. Hence the different name, as you might have noticed.

There is also a reason for not using the term "atomic". Indeed it was my preference, but Brian (as he can explain himself as well) has at least two good reasons for avoiding that term, and going with the new term NonTearable (as well as the underlying exposition about the new phenomenon of structure tearing).

(The term non-tearable is in fact closely synonymous with the term atomic, which literally means non-cuttable.)

A couple of reasons not to use "atomic" here: First, the term is already taken, in the package java.util.concurrent.atomic, with classes like AtomicInteger. Users would be faced with different AtomicFoo types, leading to needless pedagogical burden. (It doesn't matter that a dozen JMM experts would be made happier by using a common term for a common underlying notion.) Second, although you nicely propose AtomicAccess as a good and correct term, the basic meaning of Atomic, at the user level, is already stronger than atomic access: It implies not only that structures won't tear, as longs and doubles can tear in Old Java, but also the word "atomic" implies some kind of state-to-state consistency, such as is provided by CAS or atomic add. Yes, I see that "access" in "atomic access" is a clever way of backing away from state-to-state consistency claims, but it's too subtle. A new term is needed for the weaker concept.

In other words, for API design, let's keep using the term Atomic for the stronger (more useful term) and NonTearable for the weaker idea of a atomic-for-single-acceses-only. That's what you are seeing with the term "non-tearable", and what I hope you can also see with the development of the not-really-so-weird discussion of "structure tearing".

* 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.

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.

@mlbridge
Copy link

mlbridge bot commented Jun 14, 2021

Mailing list message from Remi Forax on valhalla-dev:

----- Mail original -----

De: "John R Rose" <jrose at openjdk.java.net>
?: "valhalla-dev" <valhalla-dev at openjdk.java.net>
Envoy?: Lundi 14 Juin 2021 22:59:52
Objet: Re: [lworld] RFR: 8267763: [lworld][lw3] Change "non-tearable" nomenclature to "access atomic" [v2]

On Mon, 31 May 2021 09:36:40 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

Current Valhalla code has the experimental marker interface
`java.lang.NonTearable`, which is actually about access atomicity. It makes
weird claims about word tearing and out-of-thin air values.

First, this is not word tearing. Word tearing, as defined by JLS 17.6 is: _"This
problem is sometimes known as word tearing, and on processors that cannot
easily update a single byte in isolation some other approach will be
required"._ That is, word tearing is when we cannot update the _narrow_ member
without doing a _wider_ access, thus necessarily affecting the adjacent
members. In Valhalla case, what we are dealing with is access atomicity: we
sometimes cannot access the _wide_ member without doing a set of _narrower_
accesses. This is why JLS 17.7 says "non-atomic treatment of double and longs",
not "word-tearing of double and longs".

Second, the docs for `j.l.NonTearable` mention "out-of-thin-air" (OOTA) values,
which are not related here at all. OOTA are the beasts from the causality
loops: those are values that were never written by normal execution of the
program (i.e. speculative values). In Valhalla case, the writes that produce
the broken hybrid are known and expected writes from the conflicting writers.

This nomenclature percolates to Valhalla VM code, so some change is needed there
as well.

Additional testing:
- [x] `runtime/valhalla` tests

Aleksey Shipilev has updated the pull request incrementally with one additional
commit since the last revision:

Rename a few other tests

Thank you for explaining to all of us about word-tearing, but that seems to be
moot in the context of this PR. This feature (`NonTearable`) is about
*structure tearing*, which is a distinct concept. Hence the different name, as
you might have noticed.

There is also a reason for not using the term "atomic". Indeed it was my
preference, but Brian (as he can explain himself as well) has at least two good
reasons for avoiding that term, and going with the new term `NonTearable` (as
well as the underlying exposition about the *new* phenomenon of structure
tearing).

(The term non-tearable is in fact closely synonymous with the term atomic, which
literally means non-cuttable.)

Another possible term is Indivisible.

R?mi

1 similar comment
@mlbridge
Copy link

mlbridge bot commented Jun 14, 2021

Mailing list message from Remi Forax on valhalla-dev:

----- Mail original -----

De: "John R Rose" <jrose at openjdk.java.net>
?: "valhalla-dev" <valhalla-dev at openjdk.java.net>
Envoy?: Lundi 14 Juin 2021 22:59:52
Objet: Re: [lworld] RFR: 8267763: [lworld][lw3] Change "non-tearable" nomenclature to "access atomic" [v2]

On Mon, 31 May 2021 09:36:40 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

Current Valhalla code has the experimental marker interface
`java.lang.NonTearable`, which is actually about access atomicity. It makes
weird claims about word tearing and out-of-thin air values.

First, this is not word tearing. Word tearing, as defined by JLS 17.6 is: _"This
problem is sometimes known as word tearing, and on processors that cannot
easily update a single byte in isolation some other approach will be
required"._ That is, word tearing is when we cannot update the _narrow_ member
without doing a _wider_ access, thus necessarily affecting the adjacent
members. In Valhalla case, what we are dealing with is access atomicity: we
sometimes cannot access the _wide_ member without doing a set of _narrower_
accesses. This is why JLS 17.7 says "non-atomic treatment of double and longs",
not "word-tearing of double and longs".

Second, the docs for `j.l.NonTearable` mention "out-of-thin-air" (OOTA) values,
which are not related here at all. OOTA are the beasts from the causality
loops: those are values that were never written by normal execution of the
program (i.e. speculative values). In Valhalla case, the writes that produce
the broken hybrid are known and expected writes from the conflicting writers.

This nomenclature percolates to Valhalla VM code, so some change is needed there
as well.

Additional testing:
- [x] `runtime/valhalla` tests

Aleksey Shipilev has updated the pull request incrementally with one additional
commit since the last revision:

Rename a few other tests

Thank you for explaining to all of us about word-tearing, but that seems to be
moot in the context of this PR. This feature (`NonTearable`) is about
*structure tearing*, which is a distinct concept. Hence the different name, as
you might have noticed.

There is also a reason for not using the term "atomic". Indeed it was my
preference, but Brian (as he can explain himself as well) has at least two good
reasons for avoiding that term, and going with the new term `NonTearable` (as
well as the underlying exposition about the *new* phenomenon of structure
tearing).

(The term non-tearable is in fact closely synonymous with the term atomic, which
literally means non-cuttable.)

Another possible term is Indivisible.

R?mi

Copy link
Collaborator

@rose00 rose00 left a comment

Some of this PR is based on wrong-headed misunderstanding
of the term "structure tearing", and some is based on a difference
of opinion about what's best for the end-user. The part I would
prefer to keep makes more clear the connections between
the existing "compound-atomic" APIs, all of which have Atomic
in their names, and the necessary new feature for multi-component
access atomicity, which should not have Atomic in its name.

Please reformulate to be less disruptive of existing decisions,
or change the decisions by persuasion.

It looks to me like the only useful change proposed here
is a reformulation of the trailing comment in NonTearable.java,
relating it more usefully to the Atomic ™ APIs.

A better exposition of "structure tearing" would be welcome too.
The discussion of transient vs. permanent failure is illuminating.

I think "out of thin air" is a useful description of what a programmer
sees on such a failure, since the structure as a whole was never
constructed by any thread. I realize that it might distract someone
familiar with the JMM literature. Perhaps a less confusing term
would be "a composite value never created by any thread", but
"out of thin air" (in scare quotes) seems descriptive to me at least.

The material point for the end user is that structure tearing can
lead to the appearance (temporary or permanent) of primitive
values which were never created by any thread. Giving a way
to control that phenomenon ("out of thin air" structures, created
by races if not speculation) is the motivation for this feature.

Notice that this problem is new to primitive classes. There is
no corresponding notion for identity classes which are
configured like primitives, and therefore have all final fields.
The JMM guarantees safe publication for those, which means
no rogue states are ever visible in those multi-component
structures. But when multi-component primitives are inlined,
the JMM needs a new guarantee in the place of safe publication.
Non-tearability is the term we have chosen for this.

@briangoetz
Copy link
Collaborator

briangoetz commented Jun 14, 2021

I am not married to the term non-tearable, and happy to consider alternatives, but atomic is definitely a move in the wrong direction. John outlined most of the arguments, but basically: atomicity, in the context of user-visible Java APIs, is generally reserved for describing atomicity of user-level mutative operations, including read-modify-write operations (e.g., CAS, incrementAndGet, etc.) This is the A from "ACID". (Our friends in the database community might call these operations serialized, but as that also means something else in Java, we won't call it that either.)

So, happy to consider alternatives to non-tearable (it was just the best we came up with at the time), but I think Atomic* will definitely send the wrong message about what operations are atomic relative to each other.

@openjdk openjdk deleted a comment from mlbridge bot Jun 16, 2021
@openjdk openjdk deleted a comment from mlbridge bot Jun 16, 2021
@mlbridge
Copy link

mlbridge bot commented Jun 16, 2021

Mailing list message from Kirk Pepperdine on valhalla-dev:

Not that my vote counts but I didn?t find NonTearable to be all that terrible (pun intended). At first I was thinking primitive tearing but a wee bit of reading cleared that up. I did a little splunking about in the OED and nothing really stood out as being more appropriate or offered any where near the level of clarity that this name does.

Kind regards,
Kirk

1 similar comment
@mlbridge
Copy link

mlbridge bot commented Jun 16, 2021

Mailing list message from Kirk Pepperdine on valhalla-dev:

Not that my vote counts but I didn?t find NonTearable to be all that terrible (pun intended). At first I was thinking primitive tearing but a wee bit of reading cleared that up. I did a little splunking about in the OED and nothing really stood out as being more appropriate or offered any where near the level of clarity that this name does.

Kind regards,
Kirk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants