From 6a7931c78a5f43d3e9b8e5c238c64a6da8be3539 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 11 Jul 2016 16:10:11 -0700 Subject: [PATCH] Minor fix to non-working earlier optimization for creating ArrayList/HashMap --- .../databind/deser/impl/CreatorCollector.java | 203 +++++++++++++----- .../introspect/AnnotatedConstructor.java | 2 +- .../introspect/AnnotatedWithParams.java | 35 +-- 3 files changed, 164 insertions(+), 76 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java index bd3b4ec985..87ebe99063 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java @@ -1,7 +1,9 @@ package com.fasterxml.jackson.databind.deser.impl; import java.io.IOException; +import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Member; +import java.lang.reflect.Type; import java.util.*; import com.fasterxml.jackson.databind.*; @@ -77,7 +79,7 @@ public class CreatorCollector /* Life-cycle /********************************************************** */ - + public CreatorCollector(BeanDescription beanDesc, MapperConfig config) { _beanDesc = beanDesc; @@ -91,26 +93,12 @@ public ValueInstantiator constructValueInstantiator(DeserializationConfig config final JavaType arrayDelegateType = _computeDelegateType(_creators[C_ARRAY_DELEGATE], _arrayDelegateArgs); final JavaType type = _beanDesc.getType(); - // Any non-standard creator will prevent; with one exception: int-valued constructor - // that standard containers have can be ignored - if (!_hasNonDefaultCreator) { - /* 10-May-2014, tatu: If we have nothing special, and we are dealing with one - * of "well-known" types, can create a non-reflection-based instantiator. - */ - final Class rawType = type.getRawClass(); - if (rawType == Collection.class || rawType == List.class || rawType == ArrayList.class) { - return new Vanilla(Vanilla.TYPE_COLLECTION); - } - if (rawType == Map.class || rawType == LinkedHashMap.class) { - return new Vanilla(Vanilla.TYPE_MAP); - } - if (rawType == HashMap.class) { - return new Vanilla(Vanilla.TYPE_HASH_MAP); - } - } - + // 11-Jul-2016, tatu: Earlier optimization by replacing the whole instantiator did not + // work well, so let's replace by lower-level check: + AnnotatedWithParams defaultCtor = StdTypeConstructor.tryToOptimize(_creators[C_DEFAULT]); + StdValueInstantiator inst = new StdValueInstantiator(config, type); - inst.configureFromObjectSettings(_creators[C_DEFAULT], + inst.configureFromObjectSettings(defaultCtor, _creators[C_DELEGATE], delegateType, _delegateArgs, _creators[C_PROPS], _propertyBasedArgs); inst.configureFromArraySettings(_creators[C_ARRAY_DELEGATE], arrayDelegateType, _arrayDelegateArgs); @@ -122,13 +110,13 @@ public ValueInstantiator constructValueInstantiator(DeserializationConfig config inst.configureIncompleteParameter(_incompleteParameter); return inst; } - + /* /********************************************************** /* Setters /********************************************************** */ - + /** * Method called to indicate the default creator: no-arguments * constructor or factory method that is called to instantiate @@ -179,15 +167,16 @@ public void addPropertyCreator(AnnotatedWithParams creator, boolean explicit, HashMap names = new HashMap(); for (int i = 0, len = properties.length; i < len; ++i) { String name = properties[i].getName(); - /* [Issue-13]: Need to consider Injectables, which may not have - * a name at all, and need to be skipped - */ + // Need to consider Injectables, which may not have + // a name at all, and need to be skipped if (name.length() == 0 && properties[i].getInjectableValueId() != null) { continue; } Integer old = names.put(name, Integer.valueOf(i)); if (old != null) { - throw new IllegalArgumentException("Duplicate creator property \""+name+"\" (index "+old+" vs "+i+")"); + throw new IllegalArgumentException(String.format( + "Duplicate creator property \"%s\" (index %s vs %d)", + name, old, i)); } } } @@ -344,46 +333,164 @@ protected void verifyNonDup(AnnotatedWithParams newOne, int typeIndex, boolean e /********************************************************** */ - protected final static class Vanilla - extends ValueInstantiator.Base + /** + * Replacement for default constructor to use for a small set of + * "well-known" types. + *

+ * Note: replaces earlier Vanilla ValueInstantiator + * implementation + * + * @since 2.8.1 (replacing earlier Vanilla instantiator + */ + protected final static class StdTypeConstructor + extends AnnotatedWithParams implements java.io.Serializable { private static final long serialVersionUID = 1L; - public final static int TYPE_COLLECTION = 1; - public final static int TYPE_MAP = 2; - public final static int TYPE_HASH_MAP = 3; + public final static int TYPE_ARRAY_LIST = 1; + public final static int TYPE_HASH_MAP = 2; + public final static int TYPE_LINKED_HASH_MAP = 3; + + private final AnnotatedWithParams _base; private final int _type; - public Vanilla(int t) { - super(_type(t)); + public StdTypeConstructor(AnnotatedWithParams base, int t) + { + super(base, null); + _base = base; _type = t; } - private static Class _type(int t) { - switch (t) { - case TYPE_COLLECTION: return ArrayList.class; - case TYPE_MAP: return LinkedHashMap.class; - case TYPE_HASH_MAP: return HashMap.class; + public static AnnotatedWithParams tryToOptimize(AnnotatedWithParams src) + { + if (src != null) { + final Class rawType = src.getDeclaringClass(); + if (rawType == List.class || rawType == ArrayList.class) { + return new StdTypeConstructor(src, TYPE_ARRAY_LIST); + } + if (rawType == LinkedHashMap.class) { + return new StdTypeConstructor(src, TYPE_LINKED_HASH_MAP); + } + if (rawType == HashMap.class) { + return new StdTypeConstructor(src, TYPE_HASH_MAP); + } + } + return src; + } + + protected final Object _construct() { + switch (_type) { + case TYPE_ARRAY_LIST: + return new ArrayList(); + case TYPE_LINKED_HASH_MAP: + return new LinkedHashMap(); + case TYPE_HASH_MAP: + return new HashMap(); } - return Object.class; + throw new IllegalStateException("Unknown type "+_type); } @Override - public boolean canInstantiate() { return true; } + public int getParameterCount() { + return _base.getParameterCount(); + } @Override - public boolean canCreateUsingDefault() { return true; } + public Class getRawParameterType(int index) { + return _base.getRawParameterType(index); + } @Override - public Object createUsingDefault(DeserializationContext ctxt) throws IOException { - switch (_type) { - case TYPE_COLLECTION: return new ArrayList(); - case TYPE_MAP: return new LinkedHashMap(); - case TYPE_HASH_MAP: return new HashMap(); - } - throw new IllegalStateException("Unknown type "+_type); + public JavaType getParameterType(int index) { + return _base.getParameterType(index); + } + + @SuppressWarnings("deprecation") + @Override + public Type getGenericParameterType(int index) { + return _base.getGenericParameterType(index); + } + + @Override + public Object call() throws Exception { + return _construct(); + } + + @Override + public Object call(Object[] args) throws Exception { + return _construct(); + } + + @Override + public Object call1(Object arg) throws Exception { + return _construct(); + } + + @Override + public Class getDeclaringClass() { + return _base.getDeclaringClass(); + } + + @Override + public Member getMember() { + return _base.getMember(); + } + + @Override + public void setValue(Object pojo, Object value) throws UnsupportedOperationException, IllegalArgumentException { + throw new UnsupportedOperationException(); + } + + @Override + public Object getValue(Object pojo) throws UnsupportedOperationException, IllegalArgumentException { + throw new UnsupportedOperationException(); + } + + @Override + public Annotated withAnnotations(AnnotationMap fallback) { + throw new UnsupportedOperationException(); + } + + @Override + public AnnotatedElement getAnnotated() { + return _base.getAnnotated(); + } + + @Override + protected int getModifiers() { + return _base.getMember().getModifiers(); + } + + @Override + public String getName() { + return _base.getName(); + } + + @Override + public JavaType getType() { + return _base.getType(); + } + + @Override + public Class getRawType() { + return _base.getRawType(); + } + + @Override + public boolean equals(Object o) { + return (o == this); + } + + @Override + public int hashCode() { + return _base.hashCode(); + } + + @Override + public String toString() { + return _base.toString(); } } } diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedConstructor.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedConstructor.java index 93f087645c..a5e81ad03e 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedConstructor.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedConstructor.java @@ -19,7 +19,7 @@ public final class AnnotatedConstructor * @since 2.1 */ protected Serialization _serialization; - + /* /********************************************************** /* Life-cycle diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedWithParams.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedWithParams.java index c68aa28fed..8cf5a79214 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedWithParams.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedWithParams.java @@ -32,6 +32,14 @@ protected AnnotatedWithParams(TypeResolutionContext ctxt, AnnotationMap annotati _paramAnnotations = paramAnnotations; } + /** + * @since 2.8.1 + */ + protected AnnotatedWithParams(AnnotatedWithParams base, AnnotationMap[] paramAnnotations) { + super(base); + _paramAnnotations = paramAnnotations; + } + /** * Method called to override a method parameter annotation, * usually due to a mix-in @@ -58,33 +66,6 @@ protected AnnotatedParameter replaceParameterAnnotations(int index, AnnotationMa return getParameter(index); } - /* - /********************************************************** - /* Helper methods for subclasses - /********************************************************** - */ - - /* - protected JavaType getType(TypeBindings bindings, TypeVariable[] typeParams) - { - // [JACKSON-468] Need to consider local type binding declarations too... - if (typeParams != null && typeParams.length > 0) { - bindings = bindings.childInstance(); - for (TypeVariable var : typeParams) { - String name = var.getName(); - // to prevent infinite loops, need to first add placeholder (">" etc) - bindings._addPlaceholder(name); - // About only useful piece of information is the lower bound (which is at least Object.class) - Type lowerBound = var.getBounds()[0]; - JavaType type = (lowerBound == null) ? TypeFactory.unknownType() - : bindings.resolveType(lowerBound); - bindings.addBinding(var.getName(), type); - } - } - return bindings.resolveType(getGenericType()); - } - */ - /* /********************************************************** /* Extended API