Skip to content
This repository has been archived by the owner on Oct 10, 2019. It is now read-only.

Interaction with the memory model & atomic operations #78

Closed
lars-t-hansen opened this issue Sep 5, 2017 · 12 comments
Closed

Interaction with the memory model & atomic operations #78

lars-t-hansen opened this issue Sep 5, 2017 · 12 comments

Comments

@lars-t-hansen
Copy link

(Surely you saw this coming? :-)

I assume that BigInt64 and BigUint64 TypedArrays can also be applied to SharedArrayBuffers. This raises the issue of how valus of these views are loaded and stored in shared memory.

(1) Currently the memory model guarantees that certain racy accesses are executed as-if atomic, specifically, if you have racy writes of the same size to the same location they are written one after another, parts of values are not interleaved (they do not "tear"). Reads that are performed during such a race will observe the value initially in memory, or one of the values written. Values will not fluctuate; once a value is written, the cell does not revert spontaneously to a previous value, at least not when viewed from a single observer.

Ideally we would simply extend this scheme to 64-bit integers. Doing so has some costs, however. On many 32-bit systems there are no simple non-tearing 64-bit access instructions, though one can emulate them expensively with full atomic operations; this is the case on x86 and on many ARMv7 models. On some 32-bit systems there are no non-tearing 64-bit access instructions at all; this is the case on MIPS32 and many ARMv6 models.

IMO it is possible to get away with allowing 64-bit integers to be loaded and stored racily in parts, ie, we guarantee non-tearing behavior up to 4 bytes, as JS has now, but after that nothing. (Anyone needing to store an int64 atomically would then use a lock.) Accessing int64 values atomically is nice, but in that case one should use an atomic operation. Which leads me to:

(2) Should the Atomics object be extended to loading and storing 64-bit values? (WebAssembly will provide 64-bit atomics, as well as a 64-bit version of Atomics.wait.) These are not constrained in the same way as the racy accesses, because they can use spinlocks behind the scenes on the few platforms that can't accomodate them directly (such as MIPS32).

(3) If we provide 64-bit atomic operations we also want to upgrade Atomics.isLockFree(8) to have a useful value, not always false as now.

@littledan
Copy link
Member

Thanks for raising this issue; not sure how it fell off my radar.

FWIW memory model/atomics behavior is currently "handled" because ValidateSharedIntegerTypedArray would throw on a BigInt64Array/BigUint64Array, and all atomic operations are prohibited. For the memory model, because BigUint64 and BigInt64 are not on the noTear list in SetValueInBuffer, which means that the semantics you're suggesting in (1) is already the specification.

Seems like supporting most of the atomics would be just a matter of removing the prohibition on BigUint64 and BigInt64 and making sure BigInts are handled separately from Numbers. I don't see any other changes that would be needed, but I don't understand all the memory model details, if any changes are needed there.

For Atomics.wait/Atomics.wake, I'm not sure what the semantics are supposed to be. If you do a 64-bit wait, and wake one or both of the 32-bit segments contained in it, what happens? Does it wait for both of them? Does a 64-bit wake wake all 32-bit waiters on each half?

Does Wasm have consensus on whether it will introduce an i64 wait? In WebAssembly/threads#7 , it sounded like there wasn't consensus one way or the other.

For (3), presumably by a sort of AR.[[IsLockFree8]], right? This would return false on MIPS32 then and true on x86, for example.

@lars-t-hansen
Copy link
Author

I believe, like you, that it would be easy to fix the atomics. (Also I think it's desirable to do so.) SetValueInBuffer might need a little more work but I don't think it'll be bad. I mostly want to make sure we don't spec something we can't afford to implement.

For Atomics.wait/Atomics.wake, I'm not sure what the semantics are supposed to be. If you do a 64-bit wait, and wake one or both of the 32-bit segments contained in it, what happens? Does it wait for both of them? Does a 64-bit wake wake all 32-bit waiters on each half?

Since wasm operates on byte addresses it's the address that you wait on that matters, and that's the address the two different-sized waits and the wake must agree on. If one agent performs a 64-bit wait on address A and the other performs a wake on A+4 (or A+3 for that matter, as it can) then there is no wakeup. Indeed after the cell value has been checked the size of the wait is irrelevant, only the byte address matters.

In JS we could expose that behavior to explain the corner cases, since, by and large, everyone will use the same element size to wake on as they used to wait on and won't have to worry about it. It would be legal to wake on either an int32 array or int64 array but if you don't use the same element size for waking as you did for waiting you have to take care.

Does Wasm have consensus on whether it will introduce an i64 wait?

I believe so.

In WebAssembly/threads#7 , it sounded like there wasn't consensus one way or the other.

Indeed, but there was an in-person meeting in July, and the issue was settled IIRC, anyway the instruction for int64 wait is in the wasm threads proposal and I'm seeing no pushback. It's a useful feature.

For (3), presumably by a sort of AR.[[IsLockFree8]], right? This would return false on MIPS32 then and true on x86, for example.

I think so.

littledan added a commit that referenced this issue Sep 6, 2017
Previously, it was left up for future integration details to allow
BigInt64Array and BigUint64Array to use atomic operations. This patch
spells out the details of how these should work.

The semantics of Atomics.wait/wake are based on doing the wait/wake
on the relevant index of the underlying memory. For example, a
64-bit wait would be woken up by a 32-bit wake at twice the index.

Non-atomic writes to BigInt64Arrays can be torn, following the memory
model for Float64Array.

In some places in internal spec names, `Int64` is replaced by `BigInt64`
for consistency.

Addresses #78
littledan added a commit that referenced this issue Sep 6, 2017
Previously, it was left up for future integration details to allow
BigInt64Array and BigUint64Array to use atomic operations. This patch
spells out the details of how these should work.

The semantics of Atomics.wait/wake are based on doing the wait/wake
on the relevant index of the underlying memory. For example, a
64-bit wait would be woken up by a 32-bit wake at twice the index.

Non-atomic writes to BigInt64Arrays can be torn, following the memory
model for Float64Array.

In some places in internal spec names, `Int64` is replaced by `BigInt64`
for consistency.

Addresses #78
littledan added a commit that referenced this issue Oct 12, 2017
Previously, it was left up for future integration details to allow
BigInt64Array and BigUint64Array to use atomic operations. This patch
spells out the details of how these should work.

The semantics of Atomics.wait/wake are based on doing the wait/wake
on the relevant index of the underlying memory. For example, a
64-bit wait would be woken up by a 32-bit wake at twice the index.

Non-atomic writes to BigInt64Arrays can be torn, following the memory
model for Float64Array.

In some places in internal spec names, `Int64` is replaced by `BigInt64`
for consistency.

Addresses #78
@littledan
Copy link
Member

This should be fixed now; please reopen if you see any missing details. The semantics were reviewed at the September 2017 TC39 meeting, reached consensus, and the patch was merged as a result.

@jakobkummerow
Copy link
Collaborator

It seems the Atomics.isLockFree(size) update to return a meaningful value for size == 8 (similar to what it already does for 1 and 2) has not happened.

Also, Atomics.store currently uses SetValueInBuffer, which explicitly specifies 64-bit accesses as tearing, contrary to what you would expect from an atomic operation.

@lars-t-hansen
Copy link
Author

Good point, we need an [[IsLockFree8]] property and one more step in Atomics.isLockFree.

As for the store (and presumably the same problem in Atomics.load?) one could fix this by passing another parameter atomic to Atomics.load and Atomics.store s.t. the latter compute noTear as true when the current conditions are true or atomic is true and the array is an int64 array.

@lars-t-hansen
Copy link
Author

@littledan, I'm not able to reopen this for some reason, can you perhaps do it?

@littledan littledan reopened this Feb 22, 2018
@littledan
Copy link
Member

@jakobkummerow Thanks for these reports. I'll work on patches for these today.

@ajklein
Copy link

ajklein commented Feb 22, 2018

One more note about this: it took @jakobkummerow and myself quite a bit of digging through the spec, this issue, the meeting notes, etc., to get to the point where we understood the intent of the spec. I think it would have been useful to have a non-normative note somewhere explaining the intent is to have no guarantees about tearing for non-atomic operations, but support for Int64 and Uint64 atomic ops. I know it can be tricky to figure out where to put such notes, but thought it was worth a mention.

@littledan
Copy link
Member

@ajklein Feel free to contact me any time if you're wondering about the intent of this specification. As for a note in the specification, there already was the following note:

BigInt64 and BigUint64, like Float64, are excluded from the list of types which experience noTear writes. That is, non-atomic writes may be observed in a partially completed state.

I'm not sure what else to write besides this. Would it be meaningful to write atomic writes should be atomic, and that any deviation from that is a bug in the specification?

@ajklein
Copy link

ajklein commented Feb 22, 2018

@littledan Thanks for the pointer to the existing text. That indeed seems like it covers the non-atomic side pretty well. For the atomic side, I think additions to the existing non-normative text in the "Agents" section around [[LockFree8]] as well as in the isLockFree method would take care of the other bit of this, I think: the lack of those made it unclear what the spec expected implementations to do about 64-bit atomic ops on 32-bit platforms.

@littledan
Copy link
Member

@ajklein Thanks for the suggestion. Added a note under isLockFree. It feels a little out of place, though, as isLockFree does not describe anything about the atomic operations' semantics; if anyone has an idea for another location, I'd be interested in hearing about it.

@littledan
Copy link
Member

The patch with the specification fixes have landed, and I haven't heard any more concerns about this issue, so closing. Please feel free to post any more concerns here and we can reopen it if they come up.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants