Win a copy of Modern JavaScript for the Impatient this week in the Server-Side JavaScript and NodeJS forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Ron McLeod
  • Paul Clapham
  • Bear Bibeault
  • Junilu Lacar
Sheriffs:
  • Jeanne Boyarsky
  • Tim Cooke
  • Henry Wong
Saloon Keepers:
  • Tim Moores
  • Stephan van Hulst
  • Tim Holloway
  • salvin francis
  • Frits Walraven
Bartenders:
  • Scott Selikoff
  • Piet Souris
  • Carey Brown

ConcurrentHapMap size not as expected when put done using Multiple Threads

 
Ranch Hand
Posts: 56
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
 
author and iconoclast
Posts: 24203
43
Mac OS X Eclipse IDE Chrome
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
author
Posts: 23882
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

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
 
Vivek Kr Singh
Ranch Hand
Posts: 56
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 23882
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

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
Posts: 56
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

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
Posts: 23882
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

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
Posts: 24203
43
Mac OS X Eclipse IDE Chrome
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

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
Posts: 23882
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

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
Posts: 56
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

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
 
Politics n. Poly "many" + ticks "blood sucking insects". Tiny ad:
Building a Better World in your Backyard by Paul Wheaton and Shawn Klassen-Koop
https://coderanch.com/wiki/718759/books/Building-World-Backyard-Paul-Wheaton
    Bookmark Topic Watch Topic
  • New Topic