You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by no...@apache.org on 2020/07/01 00:29:13 UTC

[lucene-solr] branch jira/solr14610 created (now f979148)

This is an automated email from the ASF dual-hosted git repository.

noble pushed a change to branch jira/solr14610
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


      at f979148  SOLR-14610: ReflectMapWriter to use VarHandle instead of old reflection

This branch includes the following new commits:

     new f979148  SOLR-14610: ReflectMapWriter to use VarHandle instead of old reflection

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[lucene-solr] 01/01: SOLR-14610: ReflectMapWriter to use VarHandle instead of old reflection

Posted by no...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

noble pushed a commit to branch jira/solr14610
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit f9791484d8b238941e06311afa4939d792ff4cdc
Author: noble <no...@apache.org>
AuthorDate: Wed Jul 1 10:28:22 2020 +1000

    SOLR-14610:
    ReflectMapWriter to use VarHandle instead of old reflection
---
 .../apache/solr/common/util/ReflectMapWriter.java  |  29 +----
 .../java/org/apache/solr/common/util/Utils.java    | 123 ++++++++++++++++-----
 .../solr/common/util/TestSolrJsonWriter.java       |  99 ++++++++++++++++-
 3 files changed, 191 insertions(+), 60 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ReflectMapWriter.java b/solr/solrj/src/java/org/apache/solr/common/util/ReflectMapWriter.java
index 1142e62..1390e11 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/ReflectMapWriter.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/ReflectMapWriter.java
@@ -18,42 +18,15 @@
 package org.apache.solr.common.util;
 
 import java.io.IOException;
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
 
 import org.apache.solr.common.MapWriter;
-import org.apache.solr.common.annotation.JsonProperty;
 
 // An implementation of MapWriter which is annotated with Jackson annotations
 public interface ReflectMapWriter extends MapWriter {
 
   @Override
   default void writeMap(EntryWriter ew) throws IOException {
-    for (Field field : this.getClass().getDeclaredFields()) {
-      JsonProperty prop = field.getAnnotation(JsonProperty.class);
-      if (prop == null) continue;
-      int modifiers = field.getModifiers();
-      if (Modifier.isPublic(modifiers) && !Modifier.isStatic(modifiers)) {
-        String fname = prop.value().isEmpty() ? field.getName() : prop.value();
-        try {
-          if (field.getType() == int.class) {
-            ew.put(fname, field.getInt(this));
-          } else if (field.getType() == float.class) {
-            ew.put(fname, field.getFloat(this));
-          } else if (field.getType() == double.class) {
-            ew.put(fname, field.getDouble(this));
-          } else if (field.getType() == boolean.class) {
-            ew.put(fname, field.getBoolean(this));
-          } else if (field.getType() == long.class) {
-            ew.put(fname, field.getLong(this));
-          } else {
-            ew.putIfNotNull(fname, field.get(this));
-          }
-        } catch (IllegalAccessException e) {
-          //it should not happen
-        }
-      }
-    }
+    Utils.reflectWrite(ew, this);
   }
 
 }
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 229417a..211827e 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -16,34 +16,6 @@
  */
 package org.apache.solr.common.util;
 
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.util.EntityUtils;
-import org.apache.solr.client.solrj.cloud.DistribStateManager;
-import org.apache.solr.client.solrj.cloud.autoscaling.VersionedData;
-import org.apache.solr.client.solrj.impl.BinaryRequestWriter;
-import org.apache.solr.common.IteratorWriter;
-import org.apache.solr.common.LinkedHashMapWriter;
-import org.apache.solr.common.MapWriter;
-import org.apache.solr.common.MapWriterMap;
-import org.apache.solr.common.SolrException;
-import org.apache.solr.common.SpecProvider;
-import org.apache.solr.common.cloud.SolrZkClient;
-import org.apache.solr.common.cloud.ZkOperation;
-import org.apache.solr.common.cloud.ZkStateReader;
-import org.apache.solr.common.params.CommonParams;
-import org.apache.zookeeper.KeeperException;
-import org.apache.zookeeper.server.ByteBufferInputStream;
-import org.noggit.CharArr;
-import org.noggit.JSONParser;
-import org.noggit.JSONWriter;
-import org.noggit.ObjectBuilder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.slf4j.MDC;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -53,6 +25,9 @@ import java.io.Reader;
 import java.io.StringReader;
 import java.io.Writer;
 import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.net.URL;
 import java.net.URLDecoder;
 import java.nio.BufferOverflowException;
@@ -75,6 +50,7 @@ import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.BiConsumer;
@@ -82,6 +58,35 @@ import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.util.EntityUtils;
+import org.apache.solr.client.solrj.cloud.DistribStateManager;
+import org.apache.solr.client.solrj.cloud.autoscaling.VersionedData;
+import org.apache.solr.client.solrj.impl.BinaryRequestWriter;
+import org.apache.solr.common.IteratorWriter;
+import org.apache.solr.common.LinkedHashMapWriter;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.MapWriterMap;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SpecProvider;
+import org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkOperation;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.server.ByteBufferInputStream;
+import org.noggit.CharArr;
+import org.noggit.JSONParser;
+import org.noggit.JSONWriter;
+import org.noggit.ObjectBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.MDC;
+
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Collections.singletonList;
 import static java.util.Collections.unmodifiableList;
@@ -821,5 +826,67 @@ public class Utils {
     return result;
   }
 
+  public static void reflectWrite(MapWriter.EntryWriter ew, Object o) throws IOException{
+    List<FieldWriter> fieldWriters = null;
+    try {
+      fieldWriters = getReflectData(o.getClass());
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+    for (FieldWriter fieldWriter : fieldWriters) {
+      try {
+        fieldWriter.write(ew, o);
+      } catch (IllegalAccessException e) {
+        //should not happen
+      }
+    }
+  }
+
+  @SuppressWarnings("rawtypes")
+  private static List<FieldWriter> getReflectData(Class c) throws IllegalAccessException {
+    boolean sameClassLoader = c.getClassLoader() == Utils.class.getClassLoader();
+    //we should not cache the class references of objects loaded from packages because they will not get garbage collected
+    //TODO fix that later
+    List<FieldWriter> reflectData = sameClassLoader ? storedReflectData.get(c): null;
+    if(reflectData == null) {
+      ArrayList<FieldWriter> l = new ArrayList<>();
+      MethodHandles.Lookup lookup = MethodHandles.publicLookup();
+      for (Field field : lookup.accessClass(c).getFields()) {
+        JsonProperty prop = field.getAnnotation(JsonProperty.class);
+        if (prop == null) continue;
+        int modifiers = field.getModifiers();
+        if (Modifier.isPublic(modifiers) && !Modifier.isStatic(modifiers)) {
+          String fname = prop.value().isEmpty() ? field.getName() : prop.value();
+          final VarHandle vhandle = lookup.unreflectVarHandle(field);
+          if (field.getType() == int.class) {
+            l.add((ew, inst) -> ew.put(fname, (int) vhandle.get(inst)));
+          } else if (field.getType() == long.class) {
+            l.add((ew, inst) -> ew.put(fname, (long) vhandle.get(inst)));
+          } else if (field.getType() == boolean.class) {
+            l.add((ew, inst) -> ew.put(fname, (boolean) vhandle.get(inst)));
+          } else if (field.getType() == double.class) {
+            l.add((ew, inst) -> ew.put(fname, (double) vhandle.get(inst)));
+          } else if (field.getType() == float.class) {
+            l.add((ew, inst) -> ew.put(fname, (float) vhandle.get(inst)));
+          } else {
+            l.add((ew, inst) -> ew.put(fname, vhandle.get(inst)));
+          }
+        }}
+
+      if(sameClassLoader){
+        storedReflectData.put(c, reflectData = Collections.unmodifiableList(new ArrayList<>(l)));
+      }
+    }
+    return reflectData;
+  }
+
+
+
+  @SuppressWarnings("rawtypes")
+  static Map<Class, List<FieldWriter>> storedReflectData =   new ConcurrentHashMap<>();
+
+  interface FieldWriter {
+    void write(MapWriter.EntryWriter ew, Object inst) throws IllegalAccessException, IOException;
+  }
 
 }
diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java b/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java
index fe2849e..b51c024 100644
--- a/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java
+++ b/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java
@@ -17,18 +17,20 @@
 
 package org.apache.solr.common.util;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.IteratorWriter;
 import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.annotation.JsonProperty;
 
 public class TestSolrJsonWriter  extends SolrTestCaseJ4 {
   public void test() throws IOException {
-    StringWriter writer = new StringWriter();
 
     Map<String, Object> map = new HashMap<>();
     map.put("k1","v1");
@@ -42,10 +44,12 @@ public class TestSolrJsonWriter  extends SolrTestCaseJ4 {
           .add("v632"));
     });
 
-    try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer)) {
-      jsonWriter.setIndent(true).writeObj(map);
+    StringWriter writer = new StringWriter();
+    try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer).setIndent(false)) {
+      jsonWriter.writeObj(map);
     }
-    Object o = Utils.fromJSONString(writer.toString());
+    String json = writer.toString();
+    Object o = Utils.fromJSONString(json);
     assertEquals("v1", Utils.getObjectByPath(o, true, "k1"));
     assertEquals(1l, Utils.getObjectByPath(o, true, "k2"));
     assertEquals(Boolean.FALSE, Utils.getObjectByPath(o, true, "k3"));
@@ -55,5 +59,92 @@ public class TestSolrJsonWriter  extends SolrTestCaseJ4 {
     assertEquals("v62", Utils.getObjectByPath(o, true, "k5/k62"));
     assertEquals("v631", Utils.getObjectByPath(o, true, "k5/k63[0]"));
     assertEquals("v632", Utils.getObjectByPath(o, true, "k5/k63[1]"));
+    C1 c1 = new C1();
+
+    int iters = 10000;
+    writer = new StringWriter();
+    try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer).setIndent(false)) {
+      jsonWriter.writeObj(c1);
+    }
+   assertEquals(json, writer.toString());
+
+
+   /*Used in perf testing
+   System.out.println("JSON REFLECT write time : "+write2String(c1,iters));
+    System.out.println("JSON Map write time : "+write2String(map, iters));
+
+    System.out.println("javabin REFLECT write time : "+write2Javabin(c1,iters));
+    System.out.println("javabin Map write time : "+write2Javabin(map, iters));*/
+
+  }
+
+  private long write2String(Object o, int iters) throws IOException {
+    long start = System.currentTimeMillis() ;
+    for
+    (int i = 0;i<iters;i++) {
+      StringWriter writer = new StringWriter();
+
+      try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer)) {
+        jsonWriter.setIndent(true).writeObj(o);
+      }
+    }
+    return System.currentTimeMillis()-start;
+
+  }
+  private long write2Javabin(Object o, int iters) throws IOException {
+    long start = System.currentTimeMillis() ;
+    for (int i = 0;i<iters;i++) {
+      ByteArrayOutputStream baos = new ByteArrayOutputStream();
+      new JavaBinCodec(null).marshal(o, baos);
+    }
+    return System.currentTimeMillis()-start;
+
+//
+//    JSON REFLECT write time : 107
+//    JSON Map write time : 62
+//    javabin REFLECT write time : 48
+//    javabin Map write time : 34
+//
+//    JSON REFLECT write time : 181
+//    JSON Map write time : 38
+//    javabin REFLECT write time : 111
+//    javabin Map write time : 33
+  }
+
+
+  public static class C1 implements ReflectMapWriter {
+    @JsonProperty
+    public String k1 = "v1";
+    @JsonProperty
+    public int k2 = 1;
+    @JsonProperty
+    public boolean k3 = false;
+    @JsonProperty
+    public C2 k4 = new C2();
+
+    @JsonProperty
+    public C3 k5 = new C3() ;
+
+
+  }
+
+  public static class C3 implements ReflectMapWriter {
+    @JsonProperty
+    public String k61 = "v61";
+
+    @JsonProperty
+    public String k62 = "v62";
+
+    @JsonProperty
+    public List<String> k63= List.of("v631","v632");
+  }
+
+  public static class C2 implements ReflectMapWriter {
+    @JsonProperty
+    public String k41 = "v41";
+
+    @JsonProperty
+    public String k42 = "v42";
+
   }
 }