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/09 08:39:54 UTC
[lucene-solr] branch branch_8x updated: SOLR-14610:
ReflectMapWriter to use MethodHandle instead of old reflection
This is an automated email from the ASF dual-hosted git repository.
noble pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 35d39ee SOLR-14610: ReflectMapWriter to use MethodHandle instead of old reflection
35d39ee is described below
commit 35d39ee097c7160b7f8d86f321d245fc77af9be6
Author: noblepaul <no...@gmail.com>
AuthorDate: Thu Jul 9 18:39:27 2020 +1000
SOLR-14610: ReflectMapWriter to use MethodHandle instead of old reflection
---
solr/CHANGES.txt | 4 +-
.../apache/solr/common/util/ReflectMapWriter.java | 29 +-----
.../java/org/apache/solr/common/util/Utils.java | 78 ++++++++++++++++
.../solr/common/util/TestSolrJsonWriter.java | 103 ++++++++++++++++++++-
4 files changed, 181 insertions(+), 33 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index caeed64..e13b99e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -27,7 +27,9 @@ Improvements
Optimizations
---------------------
-(No changes)
+
+* SOLR-14610: ReflectMapWriter to use MethodHandle instead of old reflection (noble)
+
Bug Fixes
---------------------
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 e86d77e..498a827 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
@@ -30,6 +30,7 @@ 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;
@@ -53,7 +54,10 @@ import java.io.Reader;
import java.io.StringReader;
import java.io.UnsupportedEncodingException;
import java.io.Writer;
+import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
import java.net.URL;
import java.net.URLDecoder;
import java.nio.BufferOverflowException;
@@ -76,6 +80,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;
@@ -826,5 +831,78 @@ 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 (IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ for (FieldWriter fieldWriter : fieldWriters) {
+ try {
+ fieldWriter.write(ew, o);
+ } catch( Throwable e) {
+ throw new RuntimeException(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 : 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();
+ try {
+ if (field.getType() == int.class) {
+ MethodHandle mh = lookup.findGetter(c, field.getName(), int.class);
+ l.add((ew, inst) -> ew.put(fname, (int) mh.invoke(inst)));
+ } else if (field.getType() == long.class) {
+ MethodHandle mh = lookup.findGetter(c, field.getName(), long.class);
+ l.add((ew, inst) -> ew.put(fname, (long) mh.invoke(inst)));
+ } else if (field.getType() == boolean.class) {
+ MethodHandle mh = lookup.findGetter(c, field.getName(), boolean.class);
+ l.add((ew, inst) -> ew.put(fname, (boolean) mh.invoke(inst)));
+ } else if (field.getType() == double.class) {
+ MethodHandle mh = lookup.findGetter(c, field.getName(), double.class);
+ l.add((ew, inst) -> ew.put(fname, (double) mh.invoke(inst)));
+ } else if (field.getType() == float.class) {
+ MethodHandle mh = lookup.findGetter(c, field.getName(), float.class);
+ l.add((ew, inst) -> ew.put(fname, (float) mh.invoke(inst)));
+ } else {
+ MethodHandle mh = lookup.findGetter(c, field.getName(), field.getType());
+ l.add((ew, inst) -> ew.put(fname, mh.invoke(inst)));
+ }
+ } catch (NoSuchFieldException e) {
+ //this is unlikely
+ throw new RuntimeException(e);
+ }
+ }}
+
+ if(sameClassLoader){
+ storedReflectData.put(c, reflectData = Collections.unmodifiableList(new ArrayList<>(l)));
+ }
+ }
+ return reflectData;
+ }
+
+
+
+ @SuppressWarnings("rawtypes")
+ private static Map<Class, List<FieldWriter>> storedReflectData = new ConcurrentHashMap<>();
+
+ interface FieldWriter {
+ void write(MapWriter.EntryWriter ew, Object inst) throws Throwable;
+ }
}
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..08b8d80 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,21 @@
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 com.google.common.collect.ImmutableList;
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 +45,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 +60,95 @@ 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));
+ */
+
+ }
+
+ @SuppressForbidden(reason="used for perf testing numbers only")
+ 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;
+
+ }
+ @SuppressForbidden(reason="used for perf testing numbers only")
+ 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 : 76
+// JSON Map write time : 42
+// javabin REFLECT write time : 50
+// javabin Map write time : 32
+//
+// before
+// 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= ImmutableList.of("v631","v632");
+ }
+
+ public static class C2 implements ReflectMapWriter {
+ @JsonProperty
+ public String k41 = "v41";
+
+ @JsonProperty
+ public String k42 = "v42";
+
}
}