Is This correct way to use Soft References

Question

I created a cache using Soft References a while ago, but in trying to resolve a bug I'm getting concerned that actually I've done it incorrectly and it's removing objects when it shouldn't. This is how I've done it:

private static final Map<String, SoftReference<Buffered>> imageMap =
        new HashMap<String,SoftReference<Buffered>>();

public static synchronized Buffered addImage(String sum, final byte[] imageData)
    {
        SoftReference<Buffered> bufferedRef = imageMap.get(sum);
        Buffered buffered;
        if (bufferedRef!=null)
        {
            //There are no longer any hard refs but we need again so add back in
            if(bufferedRef.get()==null)
            {
                buffered = new Buffered(imageData, sum);
                imageMap.put(sum, new SoftReference(buffered));
            }
            else
            {
                buffered=bufferedRef.get();
            }
        }
        else
        {
            buffered = new Buffered(imageData, logDescriptor, sum);
            imageMap.put(sum, new SoftReference(buffered));
        }
        return buffered;
    }

    public static Buffered getImage(String sum)
{           
    SoftReference<Buffered> sr = imageMap.get(sum);
    if(sr!=null)
    {
        return sr.get();
    }
    return null;
}

So the idea is a calling process can add new Buffered objects which can be identifed/looked up by the key sum, then as long as this Buffered object is being used by at least one object it won't be removed from the map, but if it is no longer being used by any objects then it could be garbage collection if memory gets tight.

But looking at my code now is the important thing that the key field sum is always being referenced somewhere else (which isn't necessarily the case)

EDIT: So I tried Colin's solution but I'm kind of stumped because putIfAbsent() doesn't seem to return the added value. I modified my addImage method to get some debugging

public static synchronized Buffered addImage(String sum, final byte[] imageData)
    {
        Buffered buffered = new Buffered(imageData, sum);
        Buffered buffered2 =  imageMap.get(sum );
        Buffered buffered3 =  imageMap.putIfAbsent(sum,buffered );
        Buffered buffered4 =  imageMap.get(sum );
        System.out.println("Buffered AddImage1:"+buffered);
        System.out.println("Buffered AddImage2:"+buffered2);
        System.out.println("Buffered AddImage3:"+buffered3);
        System.out.println("Buffered AddImage4:"+buffered4);                
        return buffered2;
    }

returns

Buffered AddImage1:com.Buffered@6ef725a6
Buffered AddImage2:null
Buffered AddImage3:null
Buffered AddImage4:com.Buffered@6ef725a6

So it clearly show the Buffered instance wasn't there to start with and is successfully constructed and added, but surely should be returned by putIfAbsent?

Answer 1

I'd recommend using Guava's MapMaker instead of doing this yourself.

private static final ConcurrentMap<String, Buffered> imageMap = 
    new MapMaker().softValues().makeMap();

public static Buffered addImage(String sum, final byte[] imageData) {
  Buffered buffered = new Buffered(imageData, sum);
  Buffered inMap = imageMap.putIfAbsent(sum, buffered);
  return inMap != null ? inMap : buffered;
}

public static Buffered getImage(String sum) {           
  return imageMap.get(sum);
}

Since this is a ConcurrentMap and uses putIfAbsent, you don't have to synchronize addImage unless creating an instance of Buffered is expensive. This also handles actually removing entries from the map when their value is garbage collected, unlike your code.

Edit: What do you do if you call getImage and get null as a result (perhaps because the value was garbage collected)? Is there some way that you can get the image data byte[] based on the sum key? If so, you may want to encapsulate the process of creating an instance of Buffered for a given sum as a Function<String, Buffered>. This allows you to use a computing map instead of a normal one:

private static final ConcurrentMap<String, Buffered> imageMap = new MapMaker()
    .softValues()
    .createComputingMap(getBufferedForSumFunction());

Done this way, you may not even need an addImage method... if get is called on the map and it doesn't have an entry for the given sum, it'll call the function, cache the result and return it.

Answer 2

If all you want to do is allow data to get garbage collected when it's not referenced anywhere, use a WeakHashMap.

If you want your map to actually be able to recreate the data if it is no longer available, then you'll need to modify the getImage() to check if the reference is available and if not, recreate it.

It seems to me that what you want is the former.

The difference between a soft reference and a weak reference is that the garbage collector uses algorithms to decide whether or not to reclaim a softly reachable object but always reclaims a weakly reachable object. (ref)