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/05 11:57:56 UTC
[lucene-solr] branch branch_8x updated: Revert "SOLR-14610:
ReflectMapWriter to use VarHandle instead of old legacy reflection (#1635)"
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 7f9328d Revert "SOLR-14610: ReflectMapWriter to use VarHandle instead of old legacy reflection (#1635)"
7f9328d is described below
commit 7f9328dbe19eb5fb3362aa8dfa1ff38600e2f976
Author: noble <no...@apache.org>
AuthorDate: Sun Jul 5 21:56:25 2020 +1000
Revert "SOLR-14610: ReflectMapWriter to use VarHandle instead of old legacy reflection (#1635)"
This reverts commit 1de187edb41694f9329ea9a2bbef7d94aa57aa25.
This does not work with java8. Must have committed accidently
---
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, 60 insertions(+), 195 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 8909766..95b466b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -31,8 +31,6 @@ 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 1390e11..1142e62 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,15 +18,42 @@
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 {
- Utils.reflectWrite(ew, this);
+ 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
+ }
+ }
+ }
}
}
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 aa2738b..e86d77e 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,6 +16,34 @@
*/
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;
@@ -26,9 +54,6 @@ 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;
@@ -51,7 +76,6 @@ 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;
@@ -59,35 +83,6 @@ 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;
@@ -831,67 +826,5 @@ 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 b3d2a53..fe2849e 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,20 +17,18 @@
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");
@@ -44,12 +42,10 @@ public class TestSolrJsonWriter extends SolrTestCaseJ4 {
.add("v632"));
});
- StringWriter writer = new StringWriter();
- try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer).setIndent(false)) {
- jsonWriter.writeObj(map);
+ try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer)) {
+ jsonWriter.setIndent(true).writeObj(map);
}
- String json = writer.toString();
- Object o = Utils.fromJSONString(json);
+ Object o = Utils.fromJSONString(writer.toString());
assertEquals("v1", Utils.getObjectByPath(o, true, "k1"));
assertEquals(1l, Utils.getObjectByPath(o, true, "k2"));
assertEquals(Boolean.FALSE, Utils.getObjectByPath(o, true, "k3"));
@@ -59,94 +55,5 @@ 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";
-
}
}