Backport Stapler performance improvements

jenkinsci/stapler#127
jenkinsci · Oct 9, 2017 · 7c71f1b · 7c71f1b
1 parent 877c395
commit 7c71f1b
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 28 deletions.
diff --git a/...ocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java b/...ocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java
@@ -26,6 +26,7 @@
 import org.kohsuke.stapler.export.ExportedBean;
 
 import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -38,18 +39,39 @@
 public class ModelBuilder {
     /**
      * Instantiated {@link Model}s.
-     * Registration happens in {@link Model#Model(org.kohsuke.stapler.export.ModelBuilder,Class)} so that cyclic references
+     * Registration happens in {@link Model#Model(ModelBuilder, Class, Class, String)} so that cyclic references
      * are handled correctly.
      */
     /*package*/ final Map<Class, Model> models = new ConcurrentHashMap<Class, Model>();
 
+    @Nonnull
     public <T> Model<T> get(Class<T> type) throws NotExportableException {
         return get(type, null, null);
     }
 
+    /**
+     * @throws NotExportableException if type is not exportable
+     * @return model
+     */
+    @Nonnull
     public <T> Model<T> get(Class<T> type, @CheckForNull Class<?> propertyOwner, @Nullable String property) throws NotExportableException {
-        Model m = models.get(type);
-        if(m==null) {
+        Model<T> model = getOrNull(type, propertyOwner, property);
+        if (model == null) {
+            throw new NotExportableException(type);
+        }
+        return model;
+    }
+
+    /**
+     * Instead of throwing {@link NotExportableException} this method will return null
+     * This should be used on hot paths where throwing the exception and catching it would incur a performance hit
+     * @return model
+     * @since 1.253
+     */
+    @CheckForNull
+    public <T> Model<T> getOrNull(Class<T> type, @CheckForNull Class<?> propertyOwner, @Nullable String property) {
+        Model<T> m = models.get(type);
+        if(m==null && type.getAnnotation(ExportedBean.class) != null) {
             m = new Model<T>(this, type, propertyOwner, property);
         }
         return m;

diff --git a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java
@@ -133,24 +133,21 @@ public void writeTo(Object object, TreePruner pruner, DataWriter writer) throws
         TreePruner child = pruner.accept(object, this);
         if (child==null)        return;
 
-        Object d = writer.getExportConfig().getExportInterceptor().getValue(this,object,writer.getExportConfig());
+        Object d = writer.getExportConfig().getExportInterceptor().getValue(this,object, writer.getExportConfig());
 
         if ((d==null && skipNull) || d == ExportInterceptor.SKIP) { // don't write anything
             return;
         }
         if (merge) {
             // merged property will get all its properties written here
             if (d != null) {
-                Model model;
-                try {
-                    model = owner.get(d.getClass(), parent.type, name);
-                } catch (NotExportableException e) {
-                    if(writer.getExportConfig().isSkipIfFail()){
-                        return;
-                    }
-                    throw e;
+                Class<?> objectType = d.getClass();
+                Model model = owner.getOrNull(objectType, parent.type, name);
+                if (model == null && !writer.getExportConfig().isSkipIfFail()) {
+                    throw new NotExportableException(objectType);
+                } else if (model != null) {
+                    model.writeNestedObjectTo(d, new FilteringTreePruner(parent.HAS_PROPERTY_NAME_IN_ANCESTORY,child), writer);
                 }
-                model.writeNestedObjectTo(d, new FilteringTreePruner(parent.HAS_PROPERTY_NAME_IN_ANCESTORY,child), writer);
             }
         } else {
             writer.name(name);
@@ -188,6 +185,7 @@ private void writeBuffered(Type expected, Object value, TreePruner pruner, DataW
     /**
      * Writes one value of the property to {@link DataWriter}.
      */
+    @SuppressWarnings("unchecked")
     private void writeValue(Type expected, Object value, TreePruner pruner, DataWriter writer, boolean skipIfFail) throws IOException {
         if(value==null) {
             writer.valueNull();
@@ -201,10 +199,8 @@ private void writeValue(Type expected, Object value, TreePruner pruner, DataWrit
 
         Class c = value.getClass();
 
-        Model model;
-        try {
-            model = owner.get(c, parent.type, name);
-        } catch (NotExportableException ex) {
+        Model model = owner.getOrNull(c, parent.type, name);
+        if (model == null) {
             if(STRING_TYPES.contains(c)) {
                 writer.value(value.toString());
                 return;
@@ -300,19 +296,13 @@ private void writeValue(Type expected, Object value, TreePruner pruner, DataWrit
                 return;
             }
 
-            throw ex;
-        }
-
-        try {
+            throw new NotExportableException(c);
+        } else {
             writer.type(expected, value.getClass());
-        } catch (AbstractMethodError _) {
-            // legacy impl that doesn't understand it
+            writer.startObject();
+            model.writeNestedObjectTo(value, pruner, writer);
+            writer.endObject();
         }
-
-
-        writer.startObject();
-        model.writeNestedObjectTo(value, pruner, writer);
-        writer.endObject();
     }
 
     private static class BufferedDataWriter implements DataWriter {