Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    primarykey
    data
    text
    <p>Rather than trying to roll out your own solution, why not use a <a href="http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/ConcurrentHashMap.html" rel="noreferrer">ConcurrentHashMap</a> as your set and just set all the values to some standard value? (A constant like <code>Boolean.TRUE</code> would work well.)</p> <p>I think this implementation works well with the many-readers-few-writers scenario. There's even <a href="http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/ConcurrentHashMap.html#ConcurrentHashMap%28int,%20float,%20int%29" rel="noreferrer">a constructor that lets you set the expected "concurrency level"</a>.</p> <p><strong><em>Update:</em></strong> Veer has suggested using the <a href="http://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#newSetFromMap%28java.util.Map%29" rel="noreferrer">Collections.newSetFromMap</a> utility method to turn the ConcurrentHashMap into a Set. Since the method takes a <code>Map&lt;E,Boolean&gt;</code> my guess is that it does the same thing with setting all the values to <code>Boolean.TRUE</code> behind-the-scenes.</p> <hr> <p><strong><em>Update: Addressing the poster's example</em></strong></p> <blockquote> <p>That is probably what I will end up going with, but I am still curious about how my minimalist solution could fail. – MilesHampson</p> </blockquote> <p>Your minimalist solution would work just fine with a bit of tweaking. My worry is that, although it's minimal now, it might get more complicated in the future. It's hard to remember all of the conditions you assume when making something thread-safe—especially if you're coming back to the code weeks/months/years later to make a seemingly insignificant tweak. If the ConcurrentHashMap does everything you need with sufficient performance then why not use that instead? All the nasty concurrency details are encapsulated away and even 6-months-from-now you will have a hard time messing it up!</p> <p>You do need at least one tweak before your current solution will work. As has already been pointed out, you should probably add the <code>volatile</code> modifier to <code>global</code>'s declaration. I don't know if you have a C/C++ background, but I was very surprised when I learned that the semantics of <code>volatile</code> <a href="http://en.wikipedia.org/wiki/Volatile_variable#In_Java" rel="noreferrer">in Java</a> are actually much more complicated than <a href="http://en.wikipedia.org/wiki/Volatile_variable#In_C_and_C.2B.2B" rel="noreferrer">in C</a>. If you're planning on doing a lot of concurrent programming in Java then it'd be a good idea to familiarize yourself with the basics of <a href="http://en.wikipedia.org/wiki/Java_Memory_Model" rel="noreferrer">the Java memory model</a>. If you don't make the reference to <code>global</code> a <code>volatile</code> reference then it's possible that no thread will ever see any changes to the value of <code>global</code> until they try to update it, at which point entering the <code>synchronized</code> block will flush the local cache and get the updated reference value.</p> <p>However, even with the addition of <code>volatile</code> there's still a <strong>huge</strong> problem. Here's a problem scenario with two threads:</p> <ol> <li>We begin with the empty set, or <code>global={}</code>. Threads <code>A</code> and <code>B</code> both have this value in their thread-local cached memory.</li> <li>Thread <code>A</code> obtains obtains the <code>synchronized</code> lock on <code>global</code> and starts the update by making a copy of <code>global</code> and adding the new key to the set.</li> <li>While Thread <code>A</code> is still inside the <code>synchronized</code> block, Thread <code>B</code> reads its local value of <code>global</code> onto the stack and tries to enter the <code>synchronized</code> block. Since Thread <code>A</code> is currently inside the monitor Thread <code>B</code> blocks.</li> <li>Thread <code>A</code> completes the update by setting the reference and exiting the monitor, resulting in <code>global={1}</code>.</li> <li>Thread <code>B</code> is now able to enter the monitor and makes a copy of the <code>global={1}</code> set.</li> <li>Thread <code>A</code> decides to make another update, reads in its local <code>global</code> reference and tries to enter the <code>synchronized</code> block. <strong>Since Thread B currently holds the lock on <code>{}</code> there is no lock on <code>{1}</code> and Thread <code>A</code> successfully enters the monitor!</strong></li> <li>Thread <code>A</code> also makes a copy of <code>{1}</code> for purposes of updating.</li> </ol> <p>Now Threads <code>A</code> and <code>B</code> are both inside the <code>synchronized</code> block and they have identical copies of the <code>global={1}</code> set. <strong>This means that one of their updates will be lost!</strong> This situation is caused by the fact that you're synchronizing on an object stored in a reference that you're updating inside your <code>synchronized</code> block. You should always be very careful which objects you use to synchronize. You can fix this problem by adding a new variable to act as the lock:</p> <pre><code>private volatile Collection global = new HashSet(); // start threading after this private final Object globalLock = new Object(); // final reference used for synchronization void allUpdatesGoThroughHere(Object exampleOperand) { // My hypothesis is that this prevents operations in the block being re-ordered synchronized(globalLock) { Collection copy = new HashSet(global); copy.remove(exampleOperand); // Given my hypothesis, we should have a fully constructed object here. So a // reader will either get the old or the new Collection, but never an // inconsistent one. global = copy; } } </code></pre> <p>This bug was insidious enough that none of the other answers have addressed it yet. <em>It's these kinds of crazy concurrency details that cause me to recommend using something from the already-debugged java.util.concurrent library rather than trying to put something together yourself.</em> I think the above solution would work—but how easy would it be to screw it up again? This would be so much easier:</p> <pre><code>private final Set&lt;Object&gt; global = Collections.newSetFromMap(new ConcurrentHashMap&lt;Object,Boolean&gt;()); </code></pre> <p>Since the reference is <code>final</code> you don't need to worry about threads using stale references, and since the <code>ConcurrentHashMap</code> handles all the nasty memory model issues internally you don't have to worry about all the nasty details of monitors and memory barriers!</p>
    singulars
    1. This table or related slice is empty.
    plurals
    1. This table or related slice is empty.
    1. This table or related slice is empty.
    1. This table or related slice is empty.
    1. VO
      singulars
      1. This table or related slice is empty.
    2. VO
      singulars
      1. This table or related slice is empty.
    3. VO
      singulars
      1. This table or related slice is empty.
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload