Implement #550

cowtowncoder · cowtowncoder · commit 3eb74af17d39 · 2014-09-23T14:04:57.000-07:00
diff --git a/release-notes/VERSION b/release-notes/VERSION
@@ -9,6 +9,7 @@ Version: 2.5.0 (xx-xxx-2014)
  (reported by Fabien R, fabienrenaud@github)
 #543: Problem resolving self-referential recursive types
  (reported by ahgittin@github)
+#550: Minor optimization: prune introspection of "well-known" JDK types
 #552: Improved handling for ISO-8601 (date) format
  (contributed by Jerome G, geronimo-iia@github)
 
diff --git a/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java b/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java
@@ -198,11 +198,6 @@ public boolean useForType(JavaType t)
     // Quick little shortcut, to avoid having to use global TypeFactory instance...
     private final static JavaType JSON_NODE_TYPE = SimpleType.constructUnsafe(JsonNode.class);
 
-    /* !!! 03-Apr-2009, tatu: Should try to avoid direct reference... but not
-     *   sure what'd be simple and elegant way. So until then:
-     */
-    protected final static ClassIntrospector DEFAULT_INTROSPECTOR = BasicClassIntrospector.instance;
-
     // 16-May-2009, tatu: Ditto ^^^
     protected final static AnnotationIntrospector DEFAULT_ANNOTATION_INTROSPECTOR = new JacksonAnnotationIntrospector();
 
@@ -214,8 +209,10 @@ public boolean useForType(JavaType t)
      * Base settings contain defaults used for all {@link ObjectMapper}
      * instances.
      */
-    protected final static BaseSettings DEFAULT_BASE = new BaseSettings(DEFAULT_INTROSPECTOR,
-            DEFAULT_ANNOTATION_INTROSPECTOR, STD_VISIBILITY_CHECKER, null, TypeFactory.defaultInstance(),
+    protected final static BaseSettings DEFAULT_BASE = new BaseSettings(
+            null, // can not share global ClassIntrospector any more (2.5+)
+            DEFAULT_ANNOTATION_INTROSPECTOR,
+            STD_VISIBILITY_CHECKER, null, TypeFactory.defaultInstance(),
             null, StdDateFormat.instance, null,
             Locale.getDefault(),
 //            TimeZone.getDefault()
@@ -449,9 +446,11 @@ public ObjectMapper(JsonFactory jf,
 
         HashMap<ClassKey,Class<?>> mixins = new HashMap<ClassKey,Class<?>>();
         _mixInAnnotations = mixins;
-        _serializationConfig = new SerializationConfig(DEFAULT_BASE,
+
+        BaseSettings base = DEFAULT_BASE.withClassIntrospector(defaultClassIntrospector());
+        _serializationConfig = new SerializationConfig(base,
                     _subtypeResolver, mixins);
-        _deserializationConfig = new DeserializationConfig(DEFAULT_BASE,
+        _deserializationConfig = new DeserializationConfig(base,
                     _subtypeResolver, mixins);
 
         // Some overrides we may need
@@ -467,7 +466,7 @@ public ObjectMapper(JsonFactory jf,
         // Default serializer factory is stateless, can just assign
         _serializerFactory = BeanSerializerFactory.instance;
     }
-
+    
     /**
      * Method for creating a new {@link ObjectMapper} instance that
      * has same initial configuration as this instance. Note that this
@@ -491,7 +490,6 @@ public ObjectMapper copy()
 
     /**
      * @since 2.1
-     * @param exp
      */
     protected void _checkInvalidCopy(Class<?> exp)
     {
@@ -500,6 +498,16 @@ protected void _checkInvalidCopy(Class<?> exp)
                     +" (version: "+version()+") does not override copy(); it has to");
         }
     }
+
+    /**
+     * Overridable helper method used to construct default {@link ClassIntrospector}
+     * to use.
+     * 
+     * @since 2.5
+     */
+    protected ClassIntrospector defaultClassIntrospector() {
+        return new BasicClassIntrospector();
+    }
     
     /*
     /**********************************************************
diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicClassIntrospector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicClassIntrospector.java
@@ -1,12 +1,16 @@
 package com.fasterxml.jackson.databind.introspect;
 
+import java.util.Collection;
+import java.util.Map;
+
 import com.fasterxml.jackson.databind.AnnotationIntrospector;
 import com.fasterxml.jackson.databind.DeserializationConfig;
 import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.SerializationConfig;
 import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
 import com.fasterxml.jackson.databind.cfg.MapperConfig;
 import com.fasterxml.jackson.databind.type.SimpleType;
+import com.fasterxml.jackson.databind.util.LRUMap;
 
 public class BasicClassIntrospector
     extends ClassIntrospector
@@ -19,6 +23,8 @@ public class BasicClassIntrospector
      * This is strictly performance optimization to reduce what is
      * usually one-time cost, but seems useful for some cases considering
      * simplicity.
+     * 
+     * @since 2.4
      */
     
     protected final static BasicBeanDescription STRING_DESC;
@@ -48,9 +54,21 @@ public class BasicClassIntrospector
     /**********************************************************
      */
 
+    @Deprecated // since 2.5: construct instance directly
     public final static BasicClassIntrospector instance = new BasicClassIntrospector();
 
-    public BasicClassIntrospector() { }
+    /**
+     * Looks like 'forClassAnnotations()' gets called so frequently that we
+     * should consider caching to avoid some of the lookups.
+     * 
+     * @since 2.5
+     */
+    protected final LRUMap<JavaType,BasicBeanDescription> _cachedFCA;
+
+    public BasicClassIntrospector() {
+        // a small cache should go a long way here
+        _cachedFCA = new LRUMap<JavaType,BasicBeanDescription>(16, 64);
+    }
     
     /*
     /**********************************************************
@@ -62,11 +80,18 @@ public BasicClassIntrospector() { }
     public BasicBeanDescription forSerialization(SerializationConfig cfg,
             JavaType type, MixInResolver r)
     {
-        // minor optimization: for JDK types do minimal introspection
-        BasicBeanDescription desc = _findCachedDesc(type);
+        // minor optimization: for some JDK types do minimal introspection
+        BasicBeanDescription desc = _findStdTypeDesc(type);
         if (desc == null) {
-            desc = BasicBeanDescription.forSerialization(collectProperties(cfg,
-            		type, r, true, "set"));
+            // As per [Databind#550], skip full introspection for some of standard
+            // structured types as well
+            desc = _findStdJdkCollectionDesc(cfg, type, r);
+            if (desc == null) {
+                desc = BasicBeanDescription.forSerialization(collectProperties(cfg,
+                        type, r, true, "set"));
+            }
+            // Also: this is a superset of "forClassAnnotations", so may optimize by optional add:
+            _cachedFCA.putIfAbsent(type, desc);
         }
         return desc;
     }
@@ -75,11 +100,18 @@ public BasicBeanDescription forSerialization(SerializationConfig cfg,
     public BasicBeanDescription forDeserialization(DeserializationConfig cfg,
             JavaType type, MixInResolver r)
     {
-        // minor optimization: for JDK types do minimal introspection
-        BasicBeanDescription desc = _findCachedDesc(type);
+        // minor optimization: for some JDK types do minimal introspection
+        BasicBeanDescription desc = _findStdTypeDesc(type);
         if (desc == null) {
-            desc = BasicBeanDescription.forDeserialization(collectProperties(cfg,
-            		type, r, false, "set"));
+            // As per [Databind#550], skip full introspection for some of standard
+            // structured types as well
+            desc = _findStdJdkCollectionDesc(cfg, type, r);
+            if (desc == null) {
+                desc = BasicBeanDescription.forDeserialization(collectProperties(cfg,
+                        		type, r, false, "set"));
+            }
+            // Also: this is a superset of "forClassAnnotations", so may optimize by optional add:
+            _cachedFCA.putIfAbsent(type, desc);
         }
         return desc;
     }
@@ -88,42 +120,65 @@ public BasicBeanDescription forDeserialization(DeserializationConfig cfg,
     public BasicBeanDescription forDeserializationWithBuilder(DeserializationConfig cfg,
             JavaType type, MixInResolver r)
     {
-    	// no caching for Builders (no standard JDK builder types):
-    	return BasicBeanDescription.forDeserialization(collectPropertiesWithBuilder(cfg,
-            		type, r, false));
+        // no std JDK types with Builders, so:
+
+        BasicBeanDescription desc = BasicBeanDescription.forDeserialization(collectPropertiesWithBuilder(cfg,
+                type, r, false));
+        // this is still a superset of "forClassAnnotations", so may optimize by optional add:
+        _cachedFCA.putIfAbsent(type, desc);
+        return desc;
     }
     
     @Override
     public BasicBeanDescription forCreation(DeserializationConfig cfg,
             JavaType type, MixInResolver r)
     {
-        BasicBeanDescription desc = _findCachedDesc(type);
+        BasicBeanDescription desc = _findStdTypeDesc(type);
         if (desc == null) {
-            desc = BasicBeanDescription.forDeserialization(
+
+            // As per [Databind#550], skip full introspection for some of standard
+            // structured types as well
+            desc = _findStdJdkCollectionDesc(cfg, type, r);
+            if (desc == null) {
+                desc = BasicBeanDescription.forDeserialization(
             		collectProperties(cfg, type, r, false, "set"));
+            }
         }
+        // should this be cached for FCA?
         return desc;
     }
 
     @Override
     public BasicBeanDescription forClassAnnotations(MapperConfig<?> cfg,
             JavaType type, MixInResolver r)
     {
-        boolean useAnnotations = cfg.isAnnotationProcessingEnabled();
-        AnnotatedClass ac = AnnotatedClass.construct(type.getRawClass(),
-                (useAnnotations ? cfg.getAnnotationIntrospector() : null), r);
-        return BasicBeanDescription.forOtherUse(cfg, type, ac);
+        BasicBeanDescription desc = _findStdTypeDesc(type);
+        if (desc == null) {
+            desc = _cachedFCA.get(type);
+            if (desc == null) {
+                boolean useAnnotations = cfg.isAnnotationProcessingEnabled();
+                AnnotatedClass ac = AnnotatedClass.construct(type.getRawClass(),
+                        (useAnnotations ? cfg.getAnnotationIntrospector() : null), r);
+                desc = BasicBeanDescription.forOtherUse(cfg, type, ac);
+                _cachedFCA.put(type, desc);
+            }
+        }
+        return desc;
     }
 
     @Override
     public BasicBeanDescription forDirectClassAnnotations(MapperConfig<?> cfg,
             JavaType type, MixInResolver r)
     {
-        boolean useAnnotations = cfg.isAnnotationProcessingEnabled();
-        AnnotationIntrospector ai =  cfg.getAnnotationIntrospector();
-        AnnotatedClass ac = AnnotatedClass.constructWithoutSuperTypes(type.getRawClass(),
-                (useAnnotations ? ai : null), r);
-        return BasicBeanDescription.forOtherUse(cfg, type, ac);
+        BasicBeanDescription desc = _findStdTypeDesc(type);
+        if (desc == null) {
+            boolean useAnnotations = cfg.isAnnotationProcessingEnabled();
+            AnnotationIntrospector ai =  cfg.getAnnotationIntrospector();
+            AnnotatedClass ac = AnnotatedClass.constructWithoutSuperTypes(type.getRawClass(),
+                    (useAnnotations ? ai : null), r);
+            desc = BasicBeanDescription.forOtherUse(cfg, type, ac);
+        }
+        return desc;
     }
     
     /*
@@ -167,20 +222,62 @@ protected POJOPropertiesCollector constructPropertyCollector(MapperConfig<?> con
      * Method called to see if type is one of core JDK types
      * that we have cached for efficiency.
      */
-    protected BasicBeanDescription _findCachedDesc(JavaType type)
+    protected BasicBeanDescription _findStdTypeDesc(JavaType type)
     {
         Class<?> cls = type.getRawClass();
-        if (cls == String.class) {
-            return STRING_DESC;
+        if (cls.isPrimitive()) {
+            if (cls == Boolean.TYPE) {
+                return BOOLEAN_DESC;
+            }
+            if (cls == Integer.TYPE) {
+                return INT_DESC;
+            }
+            if (cls == Long.TYPE) {
+                return LONG_DESC;
+            }
+        } else {
+            if (cls == String.class) {
+                return STRING_DESC;
+            }
         }
-        if (cls == Boolean.TYPE) {
-            return BOOLEAN_DESC;
+        return null;
+    }
+
+    /**
+     * Helper method used to decide whether we can omit introspection
+     * for members (methods, fields, constructors); we may do so for
+     * a limited number of container types JDK provides.
+     */
+    protected boolean _isStdJDKCollection(JavaType type)
+    {
+        if (!type.isContainerType() || type.isArrayType()) {
+            return false;
         }
-        if (cls == Integer.TYPE) {
-            return INT_DESC;
+        Class<?> raw = type.getRawClass();
+        Package pkg = raw.getPackage();
+        if (pkg != null) {
+            String pkgName = pkg.getName();
+            if (pkgName.startsWith("java.lang")
+                    || pkgName.startsWith("java.util")) {
+                /* 23-Sep-2014, tatu: Should we be conservative here (minimal number
+                 *    of matches), or ambitious? Let's do latter for now.
+                 */
+                if (Collection.class.isAssignableFrom(raw)
+                        || Map.class.isAssignableFrom(raw)) {
+                    return true;
+                }
+            }
         }
-        if (cls == Long.TYPE) {
-            return LONG_DESC;
+        return false;
+    }
+
+    protected BasicBeanDescription _findStdJdkCollectionDesc(MapperConfig<?> cfg,
+            JavaType type, MixInResolver r)
+    {
+        if (_isStdJDKCollection(type)) {
+            AnnotatedClass ac = AnnotatedClass.construct(type.getRawClass(),
+                    (cfg.isAnnotationProcessingEnabled() ? cfg.getAnnotationIntrospector() : null), r);
+            return BasicBeanDescription.forOtherUse(cfg, type, ac);
         }
         return null;
     }
diff --git a/src/main/java/com/fasterxml/jackson/databind/util/LRUMap.java b/src/main/java/com/fasterxml/jackson/databind/util/LRUMap.java
@@ -13,7 +13,12 @@
  * NOTE: since version 2.4.2, this is <b>NOT</b> an LRU-based at all; reason
  * being that it is not possible to use JDK components that do LRU _AND_ perform
  * well wrt synchronization on multi-core systems. So we choose efficient synchronization
- * over potentially more effecient handling of entries.
+ * over potentially more efficient handling of entries.
+ *<p>
+ * And yes, there are efficient LRU implementations such as
+ * <a href="https://code.google.com/p/concurrentlinkedhashmap/">concurrentlinkedhashmap</a>;
+ * but at this point we really try to keep external deps to minimum. But perhaps
+ * a shaded variant may be used one day.
  */
 public class LRUMap<K,V>
     implements java.io.Serializable
@@ -43,6 +48,22 @@ public V put(K key, V value) {
         return _map.put(key, value);
     }
 
+    /**
+     * @since 2.5
+     */
+    public V putIfAbsent(K key, V value) {
+        // not 100% optimal semantically, but better from correctness (never exceeds
+        // defined maximum) and close enough all in all:
+        if (_map.size() >= _maxEntries) {
+            synchronized (this) {
+                if (_map.size() >= _maxEntries) {
+                    clear();
+                }
+            }
+        }
+        return _map.putIfAbsent(key, value);
+    }
+    
     // NOTE: key is of type Object only to retain binary backwards-compatibility
     public V get(Object key) {  return _map.get(key); }
 
