How inefficient is passing Collections.unmodifiable* an instance which is already wrapped with Collections.unmodifiable*?

Question

I have bits of piecework being done by different custom (source code unavailable) frameworks which hand back Map instances. Unfortunately, these frameworks are not consistent in their returning Map instances which have been wrapped with Collections.unmodifiableMap. To ensure a higher degree of immutability (for easier multi-threaded use) in my code, I have just uniformly called Collections.unmodifiableMap on anything returned by these frameworks.

Map<String, Record> immutableMap = framework.getRecordsByName();
//does this created a nested set of unmodifiableMap wrapper instances?
this.immutableField = Collections.unmodifiableMap(immutableMap);
.
.
.
Map<String, Record> maybeImmutableMap = framework.getRecordsByName();
//is there some means to get instanceof to work?
if (!(maybeImmutableMap instanceof Collections.UnmodifiableMap))
{
    this.immutableField = Collections.unmodifiableMap(maybeImmutableMap);
}

I realized that I might have a performance issue around this part of my design. And that in some instances, I was calling Collections.unmodifiableMap passing it an instance which had already been wrapped by the framework by the same call. And that my re-wrapping was likely causing an extra method call across the entire instance.

It appears that using "instanceof Collections.UnmodifiableMap" doesn't work. And I cannot find any way to detect (excluding using reflection which is not an option in this situation - WAY too slow) if the Map instance I am currently referencing needs to be wrapped or not.

Questions:

A) Does the Collections.unmodifiableMap() method check to see if it was passed an instance of UnmodifiableMap, and if so just return the same reference (thereby avoiding the need to check prior to calling the method)?

B) In order to proactively avoid receiving modification exceptions, is there a way to query a Map instance (other than using reflection) to detect if it is mutable (or immutable)?

C) If the answer to A is no, then is there some efficiencies in the JVM/HotSpot which eliminate the overhead of calling through the multiple method hops to get to the core instance?

Answer 1

After reviewing all of the feedback, I came to the conclusion that no matter what I do, the solution is going to be some form of kludge (have a mild odor). I think this is due to the fact that the part of the Collections API which produces unmodifiable instances didn't provide for avoiding nesting unmodifiable instances nor did it provide a "public" way for a client to properly avoid the nesting.

And due to considerations around multiple class loaders and serialization via RMI, the one solution I really liked (class reference comparison by Jorn Horstmann) has issues. However, when I take his approach and combine it with a modification of the class name approach (recommneded by Eugene Kuleshov), I think I get as close as I am going to get to having a solution that will help me in my multi-threaded distributed processing environment. And it goes a little bit like this:

public class MyCollections {
    private static final String UNMODIFIABLE_MAP_CLASS_NAME =
        Collections.unmodifiableMap(new HashMap()).getClass().getName();

    public static <K, V> Map<K, V> unmodifiableMap(Map<K, V> map) {
        return map.getClass().getName().equals(UNMODIFIABLE_MAP_CLASS_NAME)
                 ? map
                 : Collections.unmodifiableMap(map);
    }
}

This will still has all the advantages of a reference comparison assuming everything is happening within the same ClassLoader context and the classname's string has been properly interned. And it does it while politely keeping encapsulation (avoiding my code referencing the class name directly). However, if the two assumptions don't hold, then the evaluation will fall back to a standard string comparison which will work assuming the class name does not change between different versions of the library (which seems to have a pretty low probability).

Is there anything I am forgetting or missing in this approach?

And thank you again, everyone, for your feedback. I really appreciate it.

Answer 2

To the best of my knowledge:

• A) No.
• B) No.
• C) No.

Guava's Immutable* collections don't have this problem. If ImmutableList.copyOf(list) is called with a list that is itself an ImmutableList, the argument itself is returned. Additionally, you can refer to them as (and check with instanceof for) the Immutable* type rather than the interface, making it easy to know if you have an immutable instance or not. So one option is to copy the results from the framework into these immutable collections and use those throughout your own code. (They also have the advantage of being truly immutable... unmodifiable wrappers allow the original mutable instance that they wrap to be mutated itself if something has a reference to it.)

All that said, I wouldn't worry too much about the possible overhead of passing a method call through 1 or 2 unmodifiable wrapper layers, as long as you're not going to somehow wrap them again and again. As others have pointed out, it's highly unlikely that you'll ever notice a performance issue because of this.

Answer 3

You don't have to worry about performance when you wrap one unmodifiable map into another. Have a look at UnmodifiableMap class:

private static class UnmodifiableMap<K,V> implements Map<K,V>, Serializable {
    ...
UnmodifiableMap(Map<? extends K, ? extends V> m) {
        if (m==null)
            throw new NullPointerException();
        this.m = m;
    }

public int size()                {return m.size();}
    ...
public V put(K key, V value) {
    throw new UnsupportedOperationException();
    }
public V remove(Object key) {
    throw new UnsupportedOperationException();
    }

public Set<K> keySet() {
    if (keySet==null)
    keySet = unmodifiableSet(m.keySet());
    return keySet;
}

public Set<Map.Entry<K,V>> entrySet() {
    if (entrySet==null)
    entrySet = new UnmodifiableEntrySet<K,V>(m.entrySet());
    return entrySet;
}
    ...

You can see that this class is only a thin wrapper around the real map. All methods like getSize, isEmpty and other methods that don't affect map's state are delegated to the wrapped map instance. Other methods that affect map's state (put, remove) just throw UnsupportedOperationException So there is almost zero performance overload.

Answer 4

My thoughts on (C) are that the hotspot compiler should be pretty good at inlining small methods that just return the result of another method call. But I doubt that it can see through several levels of indirection, for example a HashMap wrapped in several UnmodifiableMaps. I also don't think it would be premature optimization if you know your library is sometimes returning Unmodifiable Maps. Why not create your own wrapper for Collections.unmodifiableMap like this (untested, and I just noticed that a simple instanceof won't work since the Impl in Collections is private):

public class MyCollections {
    private static final Class UNMODIFIABLE_MAP_CLASS = Collections.unmodifiableMap(new HashMap()).getClass();

    public static <K, V> Map<K, V> unmodifiableMap(Map<K, V> map) {
        return map.getClass() == UNMODIFIABLE_MAP_CLASS ? map : Collections.unmodifiableMap(map);
    }
}

Answer 5

A) No

B) No

C) Possibly, but I would expect that to depend on the JVM implmentation.

The decorator in question is extremely light weight, is there some specific reason that an extra method call that simply makes another method call would cause some performance issues in your environment? This should not be an issue in any standard application, so unless you have some really special application/environment (cpu intensive, realtime, mobile device...) it shouldn't be a problem.

Answer 6

Generally, the first rule is to not optimize it unless it is really necessary. Basically you need to collect some performance data from your application to see if wrapped collection cause any performance hit.

But if you really want to check if collection is unmodifiable, you can check "UnmodifiableMap".equals(Class.getSimpleName())