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/04 03:00:00 UTC
[lucene-solr] branch jira/solr14610_8x created (now 679c5b2)
This is an automated email from the ASF dual-hosted git repository.
noble pushed a change to branch jira/solr14610_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.
at 679c5b2 SOLR-14610: ReflectMapWriter to use VarHandle instead of reflection
This branch includes the following new commits:
new 679c5b2 SOLR-14610: ReflectMapWriter to use VarHandle instead of 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 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_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 679c5b2fff9251597afbe55b466806dae82f9daa
Author: noblepaul <no...@gmail.com>
AuthorDate: Sat Jul 4 12:59:08 2020 +1000
SOLR-14610: ReflectMapWriter to use VarHandle instead of reflection
---
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 e6c3daf..d11f23e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -33,6 +33,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 e86d77e..40be6fb 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;
@@ -54,6 +26,9 @@ import java.io.StringReader;
import java.io.UnsupportedEncodingException;
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;
@@ -76,6 +51,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;
@@ -83,6 +59,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;
@@ -826,5 +831,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, NoSuchFieldException {
+ 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();
+ 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";
+
}
}