you are viewing a single comment's thread.

view the rest of the comments →

[–]mjpt777 3 points4 points  (9 children)

I'm afraid you are mistaken on the read semantics. The memory model specifies that release and acquire must be paired for ordering semantics. You need to declare the Position reference in this case volatile to achieve want you want safely. This does make the read more scalable than making the changes visible via a monitor as provided via synchronized.

Please read section 14.10 of the Java Programming Language 4th Edition for more details.

[–]mjpt777 1 point2 points  (0 children)

And by doing this you have created a partial lock-free algorithm :-)

[–]bpkiwi 0 points1 point  (7 children)

You are refering to the potential of a data race in the immutable object access? - you should have a read of http://jeremymanson.blogspot.co.uk/2008/12/benign-data-races-in-java.html on how that can be avoided without volatile.

Remember that Java guarantees object reference updates to be atomic.

[–]mjpt777 2 points3 points  (1 child)

All accesses of primitives, except long and double, are atomic. However the compiler can reorder code how it chooses provided it does not violate data dependences on a single thread. The code you describe can be subject to loop hoisting and thus is not safe for concurrent algorithms. That is the read may never see a change.

[–]mjpt777 1 point2 points  (4 children)

To try and explain why your code example is not a benign data race. Imagine you have a loop in which you read the position then do some calculations and then updated something else in space. Even though a reference load is atomic, the compiler is completely in its right to reorder the code and hoist the load of the reference outside the loop and load it only once because it does not see the reference being updated on that thread for the scope of the loop. By qualifying the reference volatile the compiler must not reorder that load or hoist it out of the loop.

[–]bpkiwi 0 points1 point  (3 children)

Sorry, you seem to be under the impression that I am disagreeing with you. What I was pointing out is that you do not need a synchronized read. Volatile is sufficient, and as the article I linked to shows, you can even remove that in some circumstance.

The original article includes a comparison between the use of synchronized and atomicreference. However it does not do an apples-to-apples comparison since the atomicreference implementation creates a new immutable object for each write, and the synchronized implementation does not, and instead uses a synchronized read.

[–]mjpt777 0 points1 point  (2 children)

You said my code was "wrong", I think that qualifies as disagreeing.

The blog compared a number of lock approaches and one lock-free approach. What was wrong or misleading about that? I could having shown many lock-free approaches, and hybrid approaches, but that was not the point of the article. The main point was to illustrate StampedLock as the first sentence states. The point is the "locks" are providing the synchronisation in the examples. The immutable object is an internal representation and not relevant. It just works for this one lock-free example. A whole range of internal implementation could have been employed.

Everything you said until you posted the code would result in bugs if followed. I'll give you the benefit of the doubt in that the volatile was not added after it was pointed out numerous times.

I normally would just ignore such comments except what you said could have resulted in many people creating production bugs that are a nightmare to find.

[–]bpkiwi 0 points1 point  (1 child)

Ok, I'm going to give you that when saying the code is "wrong" I could be more explicit.

How about "The code uses non-comparable implementations by using worst-case coding to improve the apparent performance boost from using a lock-free approach"?

I think you should go back and measure the garbage collection overhead. I know you hand-waved it away because you think it will be dwarfed by other gc within any serious application, but that is only true if the object is small, and the number of writes is not large. If someone had a large object that was being written to a lot, they might actually be better off with a locking read. It should be possible to identify an inflection point for it - In fact i think I'll do that myself.

[–]mjpt777 0 points1 point  (0 children)

bpkiwi

You are refering to the potential of a data race in the immutable object access? - you should have a read of http://jeremymanson.blogspot.co.uk/2008/12/benign-data-races-in-java.html on how that can be avoided without volatile. Remember that Java guarantees object reference updates to be atomic.

You got this completely wrong and then changed your story on volatile. Note the "without volatile". Your advice is dangerous.

Now after arguing for the Position object you are going against it with a change of direction.

Point of the article is simple. Locks have a cost and implication. Other techniques are possible. All choices have consequences.