Two Laptop Bag*
The moose likes Threads and Synchronization and the fly likes ConcurrentHapMap size not as expected when put done using Multiple Threads Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "ConcurrentHapMap size not as expected when put done using Multiple Threads" Watch "ConcurrentHapMap size not as expected when put done using Multiple Threads" New topic
Author

ConcurrentHapMap size not as expected when put done using Multiple Threads

Vivek Kr Singh
Ranch Hand

Joined: Oct 12, 2007
Posts: 56
I wrote a piece of code to use java.util.concurrent.CountDownLatch. In this code 5 threads are being created which insert 5000 values each in a java.util.concurrent.ConcurrentHashMap. Before calculating the final map size latch should be decremented to 0.

ConcurrentHashMap size is not 25000 as expected. The value is lesser than 25000 and it is different on each run, which does suggest that size() is invoked on map when it is under modification. Any help would be appreciated.



Edit : Changed first parameter in map constructor to 5000

SCJP 1.4
Ernest Friedman-Hill
author and iconoclast
Marshal

Joined: Jul 08, 2003
Posts: 24187
    
  34

I think the major issue here is the assumption that "i++" is atomic -- it is not. The i++ operation involves a fetch, an increment, and a store, three separate steps. All five threads are trying to fetch and postincrement the same int, and you're going to have some collisions, resulting in some missed increments and map elements being overwritten.


[Jess in Action][AskingGoodQuestions]
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18977
    
  40

Vivek Kr Singh wrote:ConcurrentHashMap size is not 25000 as expected. The value is lesser than 25000 and it is different on each run, which does suggest that size() is invoked on map when it is under modification.


Not sure what you mean by "size() is invoked on map when it is under modification" -- can you elaborate a bit?

Vivek Kr Singh wrote:
Any help would be appreciated.


I believe the issue (at least the one that I see) is here...



The index variable may be volatile, but that doesn't mean that all operations are thread safe. It just means that atomic operations are thread safe -- as the JVM will not cache the values.

An increment is *not* an atomic operation in Java. It is actually a load, increment in register, and store back to memory. Because of this, you may do a few parallel increments, and since a map don't allow duplicates, lose a few items.

Henry


Books: Java Threads, 3rd Edition, Jini in a Nutshell, and Java Gems (contributor)
Vivek Kr Singh
Ranch Hand

Joined: Oct 12, 2007
Posts: 56
Thanks very much. I was using index marked as volatile which only guarantees synchronized read.
I changed index type to java.util.concurrent.atomic.AtomicInteger

Also i updated put operation on ConcurrentHashMap.

This fixed the issue. Now this code is 100% Concurrency safe or is there any pitfall remaining ?
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18977
    
  40

Ernest Friedman-Hill wrote:I think the major issue here is the assumption that "i++" is atomic -- it is not.


Ernest,

I think you mean "index++", as the "i" variable is a local variable.

Henry
Vivek Kr Singh
Ranch Hand

Joined: Oct 12, 2007
Posts: 56
Henry Wong wrote:
Not sure what you mean by "size() is invoked on map when it is under modification" -- can you elaborate a bit?


Wrong Assumption. I was assuming ConcurrentHashMap operations are delayed.
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18977
    
  40

Also i updated put operation on ConcurrentHashMap.


BTW, not that its an issue, since this is only a test program, but...

If you want "index++" functionality, then you should use the atomicIndex.getAndIncrement() method. The incrementAndGet() method implements an atomic pre increment operation.

Henry
Ernest Friedman-Hill
author and iconoclast
Marshal

Joined: Jul 08, 2003
Posts: 24187
    
  34

Henry Wong wrote:
Ernest Friedman-Hill wrote:I think the major issue here is the assumption that "i++" is atomic -- it is not.


Ernest,

I think you mean "index++", as the "i" variable is a local variable.

Henry


Yeah. Do I still get points for answering first?
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18977
    
  40

Ernest Friedman-Hill wrote:
Yeah. Do I still get points for answering first?


I guess... but... since your lead is insurmountable, I don't keep track...

Henry
Vivek Kr Singh
Ranch Hand

Joined: Oct 12, 2007
Posts: 56
Henry Wong wrote:
If you want "index++" functionality, then you should use the atomicIndex.getAndIncrement() method. The incrementAndGet() method implements an atomic pre increment operation.

I already updated that
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: ConcurrentHapMap size not as expected when put done using Multiple Threads