Concurrency level for ConcurrentHashMap in synchronized method

Question

I'm trying to fix a memory leak issue. Heap dump analysis shows that a ConcurrentHashMap is occupying around 98% of heap memory. Checked the code and it turns out that ConcurrentHashMap instantiation is using a constructor with no parameter. The default configuration for concurrencyLevel is 16. After this map instantiation I see a synchronized method call where data is being put in the map.

I would like to know that since data is being put only in synchronized method, is it safe to set concurrencyLevel of ConcurrentHashMap to 1?

Following is the sample code snippet:

private volatile Map<String, Integer> storeCache;

public void someMethod() {
    storeCache = new ConcurrentHashMap<String, Integer>();
    syncMethod();
}

private synchronized void syncMethod() {
    storeCache.put("Test", 1);
}

The CHM doesn't use any byte arrays, so this won't make any difference. The default CHM instance uses at least 5 orders of magnitude less space than 100 million bytes used by the byte array. — Marko Topolnik, Sep 07 '16 at 08:40
My point: the CHM isn't occupying 98% space with its internals, but with the stuff it contains. You'll get nothing by using parallelism of 1. — Marko Topolnik, Sep 07 '16 at 08:44
You need to look for a memory leak in your application code. Someone is adding too much stuff to the map - simple as that. You can always trying printing the map to a text file. — rghome, Sep 07 '16 at 08:45
I asked this after referring to this link -- https://ria101.wordpress.com/2011/12/12/concurrenthashmap-avoid-a-common-misuse/ — Vijay Nandwana, Sep 07 '16 at 08:47
Few good points about memory leak for concurenthashmap are here, on this qiestion: http://stackoverflow.com/questions/3959122/memory-fully-utilized-by-java-concurrenthashmap-under-tomcat — Hrabosch, Sep 07 '16 at 08:54
a bit off topic, if you are accessing the map only thru synchronized method, then why use ConcurrentHashMap? Simply a HashMap will work and should give you better performance — Adrian Shum, Sep 09 '16 at 02:17

score 4 · Accepted Answer · edited May 23 '17 at 12:01

I would like to know that since data is being put only in synchronized method, is it safe to set concurrencyLevel of ConcurrentHashMap to 1?

It's certainly safe, in the sense that it's not going to cause any Map corruption. However, it's not going to fix your memory leak. In fact, you probably don't want to synchronize access to the ConcurrentHashMap, which already guarantees safe reads and writes from multiple threads. Synchronizing externally is going to single-thread access to your CHM, which is going to eliminate many of the benefits of the CHM over a HashMap. If you remove the synchronized and specify a concurrencyLevel equal to the estimated number of concurrent writes, you'll probably achieve much better performance.

As for your memory leak, the keys and values in the CHM are strong references, meaning the Java garbage collector won't collect them, even if they're no longer referenced anywhere else in your code. So if you're using the CHM as a cache for temporary values, you'll need to .remove() them when your application no longer needs them.

(If you want the semantics of a ConcurrentMap without the strong keys, you can't get that out-of-the-box, but Guava provides a pretty good alternative.)

You may also want to check that the keys that you're .put()ing into the map have properly implemented .equals() and .hashCode().

It does make sense. Thank you Jason for writing an elaborative answer. — Vijay Nandwana, Sep 12 '16 at 11:09

Concurrency level for ConcurrentHashMap in synchronized method

1 Answers1