You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2020/07/04 11:00:57 UTC
[lucene-solr] 14/14: SOLR-14610: ReflectMapWriter to use VarHandle
instead of old legacy reflection (#1635)
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch jira/solr-12847-2
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit acae47f9c3d2c189e2920b803a26eccfc9803438
Author: Noble Paul <no...@users.noreply.github.com>
AuthorDate: Fri Jul 3 15:25:58 2020 +1000
SOLR-14610: ReflectMapWriter to use VarHandle instead of old legacy reflection (#1635)
---
solr/CHANGES.txt | 2 +
.../apache/solr/common/util/ReflectMapWriter.java | 29 +----
.../java/org/apache/solr/common/util/Utils.java | 123 ++++++++++++++++-----
.../solr/common/util/TestSolrJsonWriter.java | 101 ++++++++++++++++-
4 files changed, 195 insertions(+), 60 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index f6f816c..f465a19 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -118,6 +118,8 @@ Other Changes
* SOLR-14592: Upgrade Zookeeper to 3.6.1. NOTE: this required upgrading netty to 4.1.50 (Erick Erickson)
+* SOLR-14610: ReflectMapWriter to use VarHandle instead of reflection (noble)
+
* SOLR-10742: SolrCores.getNamesForCore is quite inefficient and blocks other core operations.
NOTE: this experimental method has been removed (Erick Erickson)
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..b3d2a53 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,94 @@ 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 : 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";
+
}
}