10

Java Gurus,

Currently we have a HashMap<String,SomeApplicationObject> which is being read frequently and modified occasionally and we are having issues that during the modification/reloading, Read operation returns null which is not acceptable.

To fix this I have following options:

A. Use ConcurrentHashMap

Which looks like the first choice but the operation which we are talking about is reload() - means clear() followed by replaceAll(). So if the Map is read post clear() and pre replaceAll() it returns null which is not desirable. Even if I synchronize this doesn't resolves the issue.

B. Create another implementation based upon ReentrantReadWriteLock

Where I would create acquire Write Lock before reload() operation. This seems more appropriate but I feel there must be something already available for this and I need not to reinvent the wheel.

What is the best way out?

EDIT Is any Collection already available with such feature?

Bharat Sinha
  • 13,973
  • 6
  • 39
  • 63

3 Answers3

8

Since you are reloading the map, I would replace it on a reload.

You can do this by using a volatile Map, which you replace in full when it is updated.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • could you please explain in more details? It is somehow hard for me to reason why this will work. I do understand that volatile will guarantee visibility, but how a compound operation like clear than replaceAll will work? Thank You. – Eugene Aug 24 '12 at 12:28
  • To replace the entire map, all you have to do is assign the new map to the old volatile reference in your field when you have built the new copy. i.e. you really are replacing all, even the map itself not just its contents. – Peter Lawrey Aug 24 '12 at 13:40
6

It seems you are not sure as to how what Peter Lawrey suggests can be implemented. It could look like this:

class YourClass {
    private volatile Map<String, SomeApplicationObject> map;

    //constructors etc.

    public void reload() {
        Map<String,SomeApplicationObject> newMap = getNewValues();
        map = Collections.unmodifiableMap(newMap);
    }
}

There are no concurrency issues because:

  • The new map is created via a local variable, which by definition is not shared - getNewValues does not need to be synchronized or atomic
  • The assignement to map is atomic
  • map is volatile, which guarantees that other threads will see the change
assylias
  • 321,522
  • 82
  • 660
  • 783
5

This sounds a lot like Guava's Cache, though it really depends how you're populating the map, and how you compute the values. (Disclosure: I contribute to Guava.)

The real question is whether or not you can specify how to compute your SomeApplicationObject given the input String. Just based on what you've told us so far, it might look something like this...

LoadingCache<String, SomeApplicationObject> cache = CacheBuilder.newBuilder()
   .build(
       new CacheLoader<String, SomeApplicationObject>() {
         public SomeApplicationObject load(String key) throws AnyException {
           return computeSomeApplicationObject(key);
         }
       });

Then, whenever you wanted to rebuild the cache, you just call cache.invalidateAll(). With a LoadingCache, you can then call cache.get(key) and if it hasn't computed the value already, it'll get recomputed. Or maybe after calling cache.invalidateAll(), you can call cache.loadAll(allKeys), though you'd still need to be able to load single elements at a time in case any queries come in between the invalidateAll and loadAll.

If this isn't acceptable -- if you can't load one value individually, you have to load them all at once -- then I'd go ahead with Peter Lawrey's approach -- keep a volatile reference to a map (ideally an ImmutableMap), recompute the whole map and assign the new map to the reference when you're done.

Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
  • SomeApplicationObject is an Entity which is preloaded from DB when application starts and read from Map all the time... Occasionally System Admin can reload the values. – Bharat Sinha Aug 24 '12 at 16:58