Collections.synchronizedMap - worth using?

[Ian Pilcher]

I've got a map that will be access by multiple threads, so I'm
considering using Collections.synchronizedMap to "wrap" it. There will
be occasions, though, where I will need to manually synchronize.

synchronized (syncMap)
{
if (syncMap.containsKey(key))
throw new AlreadyGotOneException();
syncMap.put(key, value);
}

This "double synchronization" seems inelegant, and possibly wasteful.

Is there a consensus on the best practice in this situation?

Thanks!

 

[Andreas Leitgeb]

I've got a map that will be access by multiple threads, so I'm
considering using Collections.synchronizedMap to "wrap" it. There will
be occasions, though, where I will need to manually synchronize.

The synchronizedMap will just prevent two actions on the map
to happen at same time:
e.g. when you do a .get(...), at exactly the same time when
another thread tries to do a .put(...,...).

If you need more actions in one go (conglomerate action), then

synchronized (syncMap)
{
if (syncMap.containsKey(key))
throw new AlreadyGotOneException();
syncMap.put(key, value);
}

This "double synchronization" seems inelegant, and possibly wasteful.

No, it isn't. It's necessary, but with a synchronizedMap you
at least only need to explicitly protect conglomerate actions,
not every single .get() and .put() as you would need for normal
Maps when concurrently accessed in multiple threads.

Re-requesting a lock that the thread already has, shouldn't really
be that expensive.

 

[Daniel Pitts]

I've got a map that will be access by multiple threads, so I'm
considering using Collections.synchronizedMap to "wrap" it. There will
be occasions, though, where I will need to manually synchronize.

synchronized (syncMap)
{
if (syncMap.containsKey(key))
throw new AlreadyGotOneException();
syncMap.put(key, value);
}

This "double synchronization" seems inelegant, and possibly wasteful.

Is there a consensus on the best practice in this situation?

Thanks!

That seems to be a good enough implementation.

I have a few questions, though.
Is the "AlreadyGotOneException" a sort of assertion against programmer
error?
Whether or not it is, does this mean the state of your application is
probably foobar?

If you are simply testing for programmer (or configuration) error,
then it might be better to do this instead:
oldValue = syncMap.put(key, value);
assert oldValue != null;
This has two side effects:
1. No extra sync
2. the new value does replace the old value, but an exception is still
thrown.

Hope this all helps,
Daniel.

 

[Tom Hawtin]

I've got a map that will be access by multiple threads, so I'm
considering using Collections.synchronizedMap to "wrap" it. There will
be occasions, though, where I will need to manually synchronize.

synchronized (syncMap)
{
if (syncMap.containsKey(key))
throw new AlreadyGotOneException();
syncMap.put(key, value);
}

This "double synchronization" seems inelegant, and possibly wasteful.

That probably wont make a particularly large impact on performance. As a
matter of style, if I have to externally synchronise like that, then I
do it everywhere.

A better approach is to use a concurrent map (assuming value cannot be
null).

Value old = map.putIfAbsent(key, value);
if (old != null) {
throw new AlreadyGotOneException();
}

Tom Hawtin