The current implementation of org.apache.myfaces.trinidadinternal.skin.SkinImpl uses a ConcurrentHashMap to store the properties. While it is a nice thought to be more thread safe, ConcurrentHashMap is a poor collection for performance. ConcurrentHashMap.get is approximately 4 times slower than a normal HashMap.
ConcurrentHashMap.get is not special, it does not block or updates to the map. Therefore, ConcurrentHashMap is really to protect changes to the map. AFAIK, Skin properties are pretty much immutable, once they are read from the XSS or CSS, they usually not changed. I wonder, is it worth developing an immutable Skin implementation that doesn't support setProperty once loaded? Then there could be some API to mark a skin as mutable for an environment. What do you think, would it be worth investigating to improve property lookup performance? -Andrew
