java.nio.*Buffer read/write atomicity
Hi guys, I wanted to cross-check what's the expected behavior of Buffers with respect to atomicity? Don't confuse the atomic operations (a la j.u.c.atomic.*) and the read/write atomicity. Here's the concrete example, should the assert always be true? ByteBuffer buf = ByteBuffer.allocate(100); (publish $buf to both threads properly) (start both threads) Thread 1: buf.putInt(0, 42); // at position 0 Thread 2: int i = buf.getInt(0); // at position 0 Assert.assertTrue( (i == 0) || (i == 42) ) Javadoc is silent about that, except for noting Buffers are not supposed to be used from the multiple threads without synchronization. I would anyway advocate to follow the atomicity behavior of plain fields and arrays, and make these reads/writes atomic under the race. The apparent reason for at least BB to fail to be atomic is that we read/write larger values byte-by-byte. Luckily, it appears to be easy to fix (for a given endianness, we can just throw in the Unsafe call). Before going out and submitting the RFE, I wanted to crosscheck if somebody has strong feelings about this. Thanks, Aleksey.
On 19/12/2012 17:48, Aleksey Shipilev wrote:
Hi guys,
I wanted to cross-check what's the expected behavior of Buffers with respect to atomicity? Don't confuse the atomic operations (a la j.u.c.atomic.*) and the read/write atomicity. Here's the concrete example, should the assert always be true?
ByteBuffer buf = ByteBuffer.allocate(100); (publish $buf to both threads properly) (start both threads)
Thread 1: buf.putInt(0, 42); // at position 0
Thread 2: int i = buf.getInt(0); // at position 0 Assert.assertTrue( (i == 0) || (i == 42) )
Javadoc is silent about that, except for noting Buffers are not supposed to be used from the multiple threads without synchronization. I would anyway advocate to follow the atomicity behavior of plain fields and arrays, and make these reads/writes atomic under the race.
The apparent reason for at least BB to fail to be atomic is that we read/write larger values byte-by-byte. Luckily, it appears to be easy to fix (for a given endianness, we can just throw in the Unsafe call). Before going out and submitting the RFE, I wanted to crosscheck if somebody has strong feelings about this.
On memory model rules then there is is an outstanding bug to update the buffer spec with at least minimal properties. Doug might remember the discussion with Dave Dice about this a few years ago. I've always meant to do it but it never got to the top of the list. That aside, I'm not aware of any discussion about the atomicity issue that you are concerned about now. As buffers are accessed directly in native code and by system calls then I think you would be limited to only specifying the put and get methods. -Alan.
On 12/19/2012 10:45 PM, Alan Bateman wrote:
On memory model rules then there is is an outstanding bug to update the buffer spec with at least minimal properties. Doug might remember the discussion with Dave Dice about this a few years ago. I've always meant to do it but it never got to the top of the list.
Aha, thanks, Alan. Does anyone has the CR number handy? Searching through bugtrack has a lots of false hits.
That aside, I'm not aware of any discussion about the atomicity issue that you are concerned about now. As buffers are accessed directly in native code and by system calls then I think you would be limited to only specifying the put and get methods.
I don't think there are problems with full-width ops in non-BB implementations. The problematic area seems to be ByteBuffer allocated on heap. Direct ByteBuffer seems to be atomic. -Aleksey.
On 19/12/2012 18:56, Aleksey Shipilev wrote:
On 12/19/2012 10:45 PM, Alan Bateman wrote:
On memory model rules then there is is an outstanding bug to update the buffer spec with at least minimal properties. Doug might remember the discussion with Dave Dice about this a few years ago. I've always meant to do it but it never got to the top of the list. Aha, thanks, Alan. Does anyone has the CR number handy? Searching through bugtrack has a lots of false hits. 6476827
That aside, I'm not aware of any discussion about the atomicity issue that you are concerned about now. As buffers are accessed directly in native code and by system calls then I think you would be limited to only specifying the put and get methods. I don't think there are problems with full-width ops in non-BB implementations. The problematic area seems to be ByteBuffer allocated on heap. Direct ByteBuffer seems to be atomic.
I don't think we can make assumptions about the access to direct buffers because it's it may not go through the Buffer API. -Alan.
Aleksey, If multiple threads have to synchronize access to the buffer then the reads/writes do not need to be atomic. Atomicity is only needed when data races are allowed. David On 20/12/2012 3:48 AM, Aleksey Shipilev wrote:
Hi guys,
I wanted to cross-check what's the expected behavior of Buffers with respect to atomicity? Don't confuse the atomic operations (a la j.u.c.atomic.*) and the read/write atomicity. Here's the concrete example, should the assert always be true?
ByteBuffer buf = ByteBuffer.allocate(100); (publish $buf to both threads properly) (start both threads)
Thread 1: buf.putInt(0, 42); // at position 0
Thread 2: int i = buf.getInt(0); // at position 0 Assert.assertTrue( (i == 0) || (i == 42) )
Javadoc is silent about that, except for noting Buffers are not supposed to be used from the multiple threads without synchronization. I would anyway advocate to follow the atomicity behavior of plain fields and arrays, and make these reads/writes atomic under the race.
The apparent reason for at least BB to fail to be atomic is that we read/write larger values byte-by-byte. Luckily, it appears to be easy to fix (for a given endianness, we can just throw in the Unsafe call). Before going out and submitting the RFE, I wanted to crosscheck if somebody has strong feelings about this.
Thanks, Aleksey.
2012/12/19 12:37 -0800, david.holmes@oracle.com:
If multiple threads have to synchronize access to the buffer then the reads/writes do not need to be atomic. Atomicity is only needed when data races are allowed.
Correct. Byte buffers, especially the direct variety, are all about performance. Making operations upon them atomic is not a problem that needs to be solved. - Mark
On 12/20/2012 09:11 PM, mark.reinhold@oracle.com wrote:
2012/12/19 12:37 -0800, david.holmes@oracle.com:
If multiple threads have to synchronize access to the buffer then the reads/writes do not need to be atomic. Atomicity is only needed when data races are allowed.
Correct.
Byte buffers, especially the direct variety, are all about performance. Making operations upon them atomic is not a problem that needs to be solved.
I would say that you can have the read/write atomicity for heap ByteBuffers without the compromises in performance (i.e. like direct ByteBuffer does with full-width reads/writes) -- basically prune out the Java code which breaks ints into the series of bytes, and cram in Unsafe.putInt() against backing array. This is not about making the CAS updates, it is about making reads and writes appear indivisible. Seems to be a good thing even if unspec'ed. -Aleksey.
Am 20.12.2012 18:18, schrieb Aleksey Shipilev:
On 12/20/2012 09:11 PM, mark.reinhold@oracle.com wrote:
2012/12/19 12:37 -0800, david.holmes@oracle.com:
If multiple threads have to synchronize access to the buffer then the reads/writes do not need to be atomic. Atomicity is only needed when data races are allowed. Correct.
Byte buffers, especially the direct variety, are all about performance. Making operations upon them atomic is not a problem that needs to be solved. I would say that you can have the read/write atomicity for heap ByteBuffers without the compromises in performance (i.e. like direct ByteBuffer does with full-width reads/writes) -- basically prune out the Java code which breaks ints into the series of bytes, and cram in Unsafe.putInt() against backing array.
I believe, it would enhance performance too. Also intrinsifying could be interesting. Here is a similar request: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6914113 -Ulf
Aleksey, Just curious - what's the driver for this? Suppose it did have full width writes/reads - you still shouldn't use it in a data racey way since it's not spec'd to be threadsafe and you can only observe torn reads/writes if you access it without synchronization. As others have suggested, I think the right approach would be to create a threadsafe version that's documented to be as such. Just my $.02 Sent from my phone On Dec 20, 2012 12:19 PM, "Aleksey Shipilev" <aleksey.shipilev@oracle.com> wrote:
On 12/20/2012 09:11 PM, mark.reinhold@oracle.com wrote:
2012/12/19 12:37 -0800, david.holmes@oracle.com:
If multiple threads have to synchronize access to the buffer then the reads/writes do not need to be atomic. Atomicity is only needed when data races are allowed.
Correct.
Byte buffers, especially the direct variety, are all about performance. Making operations upon them atomic is not a problem that needs to be solved.
I would say that you can have the read/write atomicity for heap ByteBuffers without the compromises in performance (i.e. like direct ByteBuffer does with full-width reads/writes) -- basically prune out the Java code which breaks ints into the series of bytes, and cram in Unsafe.putInt() against backing array.
This is not about making the CAS updates, it is about making reads and writes appear indivisible. Seems to be a good thing even if unspec'ed.
-Aleksey.
On 12/20/2012 09:49 PM, Vitaly Davidovich wrote:
Just curious - what's the driver for this? Suppose it did have full width writes/reads - you still shouldn't use it in a data racey way since it's not spec'd to be threadsafe and you can only observe torn reads/writes if you access it without synchronization.
The driver is the infamous "principle of least astonishment", aided by my purism. Java is remarkable in the way it deals with races, trying to surprise the least when something breaks. I think the change that brings in more consistency without sacrificing maintainability and/or performance is the change we endorse, right? -Aleksey.
But why would there be astonishment/surprise here if it says it's not threadsafe? I guess that's the part I'm having trouble understanding. Sent from my phone On Dec 20, 2012 12:54 PM, "Aleksey Shipilev" <aleksey.shipilev@oracle.com> wrote:
On 12/20/2012 09:49 PM, Vitaly Davidovich wrote:
Just curious - what's the driver for this? Suppose it did have full width writes/reads - you still shouldn't use it in a data racey way since it's not spec'd to be threadsafe and you can only observe torn reads/writes if you access it without synchronization.
The driver is the infamous "principle of least astonishment", aided by my purism. Java is remarkable in the way it deals with races, trying to surprise the least when something breaks. I think the change that brings in more consistency without sacrificing maintainability and/or performance is the change we endorse, right?
-Aleksey.
The astonishment comes from this: a. racy Unsafe.putInt(...) to byte[] is atomic* b. racy direct ByteBuffer.putInt(...) is atomic* c. racy heap ByteBuffer.putInt(...) is NOT! d. racy heap ByteBuffer.asIntBuffer().put(...) is NOT atomic again! So then, the hacky code over the byte arrays appears more consistent than public API, without any solid reason for that? Granted, we can always put our fingers in our ears and sing "la-la-la, you were never guaranteed that", but I would say this is surprising inconsistency. And by the way direct ByteBuffers (inadvertently) deal with that, it's obvious we can do the same for heap ByteBuffers. I feel dumb for having to explain this, I should have probably publish the CR with the change and advocate it helps performance, fixing the atomicity issue under the cover. The atomicity does not have to be spec'ed, but I would certainly vote for it for the equivalent implementation. Sorry for mudding the quiet waters. -Aleksey. (*) subject to underlying hardware memory model, alignment, etc. On 12/20/2012 10:03 PM, Vitaly Davidovich wrote:
But why would there be astonishment/surprise here if it says it's not threadsafe? I guess that's the part I'm having trouble understanding.
Sent from my phone
On Dec 20, 2012 12:54 PM, "Aleksey Shipilev" <aleksey.shipilev@oracle.com <mailto:aleksey.shipilev@oracle.com>> wrote:
On 12/20/2012 09:49 PM, Vitaly Davidovich wrote: > Just curious - what's the driver for this? Suppose it did have full > width writes/reads - you still shouldn't use it in a data racey way > since it's not spec'd to be threadsafe and you can only observe torn > reads/writes if you access it without synchronization.
The driver is the infamous "principle of least astonishment", aided by my purism. Java is remarkable in the way it deals with races, trying to surprise the least when something breaks. I think the change that brings in more consistency without sacrificing maintainability and/or performance is the change we endorse, right?
-Aleksey.
In the name of performance, go for it. :) Vitaly Sent from my phone On Dec 20, 2012 1:22 PM, "Aleksey Shipilev" <aleksey.shipilev@oracle.com> wrote:
The astonishment comes from this: a. racy Unsafe.putInt(...) to byte[] is atomic* b. racy direct ByteBuffer.putInt(...) is atomic* c. racy heap ByteBuffer.putInt(...) is NOT! d. racy heap ByteBuffer.asIntBuffer().put(...) is NOT atomic again!
So then, the hacky code over the byte arrays appears more consistent than public API, without any solid reason for that? Granted, we can always put our fingers in our ears and sing "la-la-la, you were never guaranteed that", but I would say this is surprising inconsistency. And by the way direct ByteBuffers (inadvertently) deal with that, it's obvious we can do the same for heap ByteBuffers.
I feel dumb for having to explain this, I should have probably publish the CR with the change and advocate it helps performance, fixing the atomicity issue under the cover. The atomicity does not have to be spec'ed, but I would certainly vote for it for the equivalent implementation.
Sorry for mudding the quiet waters.
-Aleksey.
(*) subject to underlying hardware memory model, alignment, etc.
On 12/20/2012 10:03 PM, Vitaly Davidovich wrote:
But why would there be astonishment/surprise here if it says it's not threadsafe? I guess that's the part I'm having trouble understanding.
Sent from my phone
On Dec 20, 2012 12:54 PM, "Aleksey Shipilev" <aleksey.shipilev@oracle.com <mailto:aleksey.shipilev@oracle.com>> wrote:
On 12/20/2012 09:49 PM, Vitaly Davidovich wrote: > Just curious - what's the driver for this? Suppose it did have full > width writes/reads - you still shouldn't use it in a data racey way > since it's not spec'd to be threadsafe and you can only observe torn > reads/writes if you access it without synchronization.
The driver is the infamous "principle of least astonishment", aided by my purism. Java is remarkable in the way it deals with races, trying to surprise the least when something breaks. I think the change that brings in more consistency without sacrificing maintainability and/or performance is the change we endorse, right?
-Aleksey.
On 21/12/2012 4:22 AM, Aleksey Shipilev wrote:
The astonishment comes from this: a. racy Unsafe.putInt(...) to byte[] is atomic* b. racy direct ByteBuffer.putInt(...) is atomic* c. racy heap ByteBuffer.putInt(...) is NOT! d. racy heap ByteBuffer.asIntBuffer().put(...) is NOT atomic again!
The only thing I find astonishing here is that anyone would spend time analysing how different non-thread-safe data structures behave in the face of data races. Seriously I think you are in a very small crowd :) Even the c-i discussion was all over the field with very little actual discussion of atomicity as such. If there are performance issues that can be addressed here, or you can generally make this "better", then fine. But atomicity is not a problem here and the last thing we want is to encourage people to use these classes in racy contexts because they now think it is "safe" because of atomic accesses. David ----
So then, the hacky code over the byte arrays appears more consistent than public API, without any solid reason for that? Granted, we can always put our fingers in our ears and sing "la-la-la, you were never guaranteed that", but I would say this is surprising inconsistency. And by the way direct ByteBuffers (inadvertently) deal with that, it's obvious we can do the same for heap ByteBuffers.
I feel dumb for having to explain this, I should have probably publish the CR with the change and advocate it helps performance, fixing the atomicity issue under the cover. The atomicity does not have to be spec'ed, but I would certainly vote for it for the equivalent implementation.
Sorry for mudding the quiet waters.
-Aleksey.
(*) subject to underlying hardware memory model, alignment, etc.
On 12/20/2012 10:03 PM, Vitaly Davidovich wrote:
But why would there be astonishment/surprise here if it says it's not threadsafe? I guess that's the part I'm having trouble understanding.
Sent from my phone
On Dec 20, 2012 12:54 PM, "Aleksey Shipilev" <aleksey.shipilev@oracle.com<mailto:aleksey.shipilev@oracle.com>> wrote:
On 12/20/2012 09:49 PM, Vitaly Davidovich wrote: > Just curious - what's the driver for this? Suppose it did have full > width writes/reads - you still shouldn't use it in a data racey way > since it's not spec'd to be threadsafe and you can only observe torn > reads/writes if you access it without synchronization.
The driver is the infamous "principle of least astonishment", aided by my purism. Java is remarkable in the way it deals with races, trying to surprise the least when something breaks. I think the change that brings in more consistency without sacrificing maintainability and/or performance is the change we endorse, right?
-Aleksey.
On 12/20/2012 06:44 PM, David Holmes wrote:
On 21/12/2012 4:22 AM, Aleksey Shipilev wrote:
The astonishment comes from this: a. racy Unsafe.putInt(...) to byte[] is atomic* b. racy direct ByteBuffer.putInt(...) is atomic* c. racy heap ByteBuffer.putInt(...) is NOT! d. racy heap ByteBuffer.asIntBuffer().put(...) is NOT atomic again!
The only thing I find astonishing here is that anyone would spend time analysing how different non-thread-safe data structures behave in the face of data races. Seriously I think you are in a very small crowd :) Even the c-i discussion was all over the field with very little actual discussion of atomicity as such.
Well I reckon that atomicity might become important in the face of buffer slices - particularly the practice of slicing up large buffers into smaller buffers, for the purposes of pooling or whatever. Especially if someone thinks it is a good idea to use weird unalignable sizes for their slices.
If there are performance issues that can be addressed here, or you can generally make this "better", then fine. But atomicity is not a problem here and the last thing we want is to encourage people to use these classes in racy contexts because they now think it is "safe" because of atomic accesses.
David ----
So then, the hacky code over the byte arrays appears more consistent than public API, without any solid reason for that? Granted, we can always put our fingers in our ears and sing "la-la-la, you were never guaranteed that", but I would say this is surprising inconsistency. And by the way direct ByteBuffers (inadvertently) deal with that, it's obvious we can do the same for heap ByteBuffers.
I feel dumb for having to explain this, I should have probably publish the CR with the change and advocate it helps performance, fixing the atomicity issue under the cover. The atomicity does not have to be spec'ed, but I would certainly vote for it for the equivalent implementation.
Sorry for mudding the quiet waters.
-Aleksey.
(*) subject to underlying hardware memory model, alignment, etc.
On 12/20/2012 10:03 PM, Vitaly Davidovich wrote:
But why would there be astonishment/surprise here if it says it's not threadsafe? I guess that's the part I'm having trouble understanding.
Sent from my phone
On Dec 20, 2012 12:54 PM, "Aleksey Shipilev" <aleksey.shipilev@oracle.com<mailto:aleksey.shipilev@oracle.com>> wrote:
On 12/20/2012 09:49 PM, Vitaly Davidovich wrote: > Just curious - what's the driver for this? Suppose it did have full > width writes/reads - you still shouldn't use it in a data racey way > since it's not spec'd to be threadsafe and you can only observe torn > reads/writes if you access it without synchronization.
The driver is the infamous "principle of least astonishment", aided by my purism. Java is remarkable in the way it deals with races, trying to surprise the least when something breaks. I think the change that brings in more consistency without sacrificing maintainability and/or performance is the change we endorse, right?
-Aleksey.
-- - DML
Users are unlikely to expect multi-byte atomicity on a ByteBuffer. However they are more likely to expect int-width atomicity on an IntBuffer view of a ByteBuffer; such atomicity is unfortunately not guaranteed either. Zhong Yu On Wed, Dec 19, 2012 at 11:48 AM, Aleksey Shipilev <aleksey.shipilev@oracle.com> wrote:
Hi guys,
I wanted to cross-check what's the expected behavior of Buffers with respect to atomicity? Don't confuse the atomic operations (a la j.u.c.atomic.*) and the read/write atomicity. Here's the concrete example, should the assert always be true?
ByteBuffer buf = ByteBuffer.allocate(100); (publish $buf to both threads properly) (start both threads)
Thread 1: buf.putInt(0, 42); // at position 0
Thread 2: int i = buf.getInt(0); // at position 0 Assert.assertTrue( (i == 0) || (i == 42) )
Javadoc is silent about that, except for noting Buffers are not supposed to be used from the multiple threads without synchronization. I would anyway advocate to follow the atomicity behavior of plain fields and arrays, and make these reads/writes atomic under the race.
The apparent reason for at least BB to fail to be atomic is that we read/write larger values byte-by-byte. Luckily, it appears to be easy to fix (for a given endianness, we can just throw in the Unsafe call). Before going out and submitting the RFE, I wanted to crosscheck if somebody has strong feelings about this.
Thanks, Aleksey.
I'm with David on this one - since BB specifically says that it's not threadsafe I don't see why there would be any expectation of atomicity. Same goes for IntBuffer or LongBuffer. Vitaly Sent from my phone On Dec 19, 2012 6:23 PM, "Zhong Yu" <zhong.j.yu@gmail.com> wrote:
Users are unlikely to expect multi-byte atomicity on a ByteBuffer.
However they are more likely to expect int-width atomicity on an IntBuffer view of a ByteBuffer; such atomicity is unfortunately not guaranteed either.
Zhong Yu
On Wed, Dec 19, 2012 at 11:48 AM, Aleksey Shipilev <aleksey.shipilev@oracle.com> wrote:
Hi guys,
I wanted to cross-check what's the expected behavior of Buffers with respect to atomicity? Don't confuse the atomic operations (a la j.u.c.atomic.*) and the read/write atomicity. Here's the concrete example, should the assert always be true?
ByteBuffer buf = ByteBuffer.allocate(100); (publish $buf to both threads properly) (start both threads)
Thread 1: buf.putInt(0, 42); // at position 0
Thread 2: int i = buf.getInt(0); // at position 0 Assert.assertTrue( (i == 0) || (i == 42) )
Javadoc is silent about that, except for noting Buffers are not supposed to be used from the multiple threads without synchronization. I would anyway advocate to follow the atomicity behavior of plain fields and arrays, and make these reads/writes atomic under the race.
The apparent reason for at least BB to fail to be atomic is that we read/write larger values byte-by-byte. Luckily, it appears to be easy to fix (for a given endianness, we can just throw in the Unsafe call). Before going out and submitting the RFE, I wanted to crosscheck if somebody has strong feelings about this.
Thanks, Aleksey.
Thanks for all the inputs. I think I need to get off to concurrency-interest to ask what the general public expectations are, to see whether we need to spec this more accurately. Even though spec is explicit now about using Buffers with synchronization, there is also a "reasonable behavior", that we would certainly like to take into the consideration. Here's the link for the discussion: http://cs.oswego.edu/pipermail/concurrency-interest/2012-December/010390.htm... -Aleksey. On 12/20/2012 04:21 AM, Vitaly Davidovich wrote:
I'm with David on this one - since BB specifically says that it's not threadsafe I don't see why there would be any expectation of atomicity. Same goes for IntBuffer or LongBuffer.
Vitaly
Sent from my phone
On Dec 19, 2012 6:23 PM, "Zhong Yu" <zhong.j.yu@gmail.com <mailto:zhong.j.yu@gmail.com>> wrote:
Users are unlikely to expect multi-byte atomicity on a ByteBuffer.
However they are more likely to expect int-width atomicity on an IntBuffer view of a ByteBuffer; such atomicity is unfortunately not guaranteed either.
Zhong Yu
On Wed, Dec 19, 2012 at 11:48 AM, Aleksey Shipilev <aleksey.shipilev@oracle.com <mailto:aleksey.shipilev@oracle.com>> wrote: > Hi guys, > > I wanted to cross-check what's the expected behavior of Buffers with > respect to atomicity? Don't confuse the atomic operations (a la > j.u.c.atomic.*) and the read/write atomicity. Here's the concrete > example, should the assert always be true? > > ByteBuffer buf = ByteBuffer.allocate(100); > (publish $buf to both threads properly) > (start both threads) > > Thread 1: > buf.putInt(0, 42); // at position 0 > > Thread 2: > int i = buf.getInt(0); // at position 0 > Assert.assertTrue( (i == 0) || (i == 42) ) > > Javadoc is silent about that, except for noting Buffers are not supposed > to be used from the multiple threads without synchronization. I would > anyway advocate to follow the atomicity behavior of plain fields and > arrays, and make these reads/writes atomic under the race. > > The apparent reason for at least BB to fail to be atomic is that we > read/write larger values byte-by-byte. Luckily, it appears to be easy to > fix (for a given endianness, we can just throw in the Unsafe call). > Before going out and submitting the RFE, I wanted to crosscheck if > somebody has strong feelings about this. > > Thanks, > Aleksey.
participants (8)
-
Alan Bateman
-
Aleksey Shipilev
-
David Holmes
-
David M. Lloyd
-
mark.reinhold＠oracle.com
-
Ulf Zibis
-
Vitaly Davidovich
-
Zhong Yu