lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a.@apache.org
Subject [lucene-solr] 01/01: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Date Tue, 06 Oct 2020 08:26:23 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit 615a1fe0421836c88bb1f20d761a5a9cf9d377f5
Author: Andrzej Bialecki <ab@apache.org>
AuthorDate: Tue Oct 6 10:25:39 2020 +0200

    SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
---
 .../apache/solr/handler/admin/MetricsHandler.java  |  13 +-
 .../org/apache/solr/util/stats/MetricUtils.java    | 223 ++++++++++++---------
 .../solr/handler/admin/MetricsHandlerTest.java     |  65 +++---
 .../apache/solr/util/stats/MetricUtilsTest.java    |  45 +++--
 .../java/org/apache/solr/common/util/Utils.java    |  57 +++++-
 5 files changed, 243 insertions(+), 160 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
index e6d8017..26c7a2b 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
@@ -36,6 +36,7 @@ import com.codahale.metrics.Metric;
 import com.codahale.metrics.MetricFilter;
 import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.Timer;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.CommonTestInjection;
@@ -117,12 +118,10 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName
     List<MetricFilter> metricFilters = metricTypes.stream().map(MetricType::asMetricFilter).collect(Collectors.toList());
     Set<String> requestedRegistries = parseRegistries(params);
 
-    @SuppressWarnings({"rawtypes"})
-    NamedList response = new SimpleOrderedMap();
+    NamedList<Object> response = new SimpleOrderedMap<>();
     for (String registryName : requestedRegistries) {
       MetricRegistry registry = metricManager.registry(registryName);
-      @SuppressWarnings({"rawtypes"})
-      SimpleOrderedMap result = new SimpleOrderedMap();
+      SimpleOrderedMap<Object> result = new SimpleOrderedMap<>();
       MetricUtils.toMaps(registry, metricFilters, mustMatchFilter, propertyFilter, false,
           false, compact, false, (k, v) -> result.add(k, v));
       if (result.size() > 0) {
@@ -134,8 +133,8 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName
 
   @SuppressWarnings({"unchecked", "rawtypes"})
   public void handleKeyRequest(String[] keys, BiConsumer<String, Object> consumer)
throws Exception {
-    SimpleOrderedMap result = new SimpleOrderedMap();
-    SimpleOrderedMap errors = new SimpleOrderedMap();
+    SimpleOrderedMap<Object> result = new SimpleOrderedMap<>();
+    SimpleOrderedMap<Object> errors = new SimpleOrderedMap<>();
     for (String key : keys) {
       if (key == null || key.isEmpty()) {
         continue;
@@ -173,6 +172,8 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName
       MetricUtils.convertMetric(key, m, propertyFilter, false, true, true, false, ":", (k,
v) -> {
         if ((v instanceof Map) && propertyName != null) {
           ((Map)v).forEach((k1, v1) -> result.add(k + ":" + k1, v1));
+        } else if ((v instanceof MapWriter) && propertyName != null) {
+          ((MapWriter) v)._forEachEntry((k1, v1) -> result.add(k + ":" + k1, v1));
         } else {
           result.add(k, v);
         }
diff --git a/solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java b/solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java
index c4fdd51..544c0c2 100644
--- a/solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java
+++ b/solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java
@@ -20,14 +20,13 @@ import java.beans.BeanInfo;
 import java.beans.IntrospectionException;
 import java.beans.Introspector;
 import java.beans.PropertyDescriptor;
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.lang.management.OperatingSystemMXBean;
 import java.lang.management.PlatformManagedObject;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedSet;
@@ -46,6 +45,8 @@ import com.codahale.metrics.MetricFilter;
 import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.Snapshot;
 import com.codahale.metrics.Timer;
+import org.apache.solr.common.IteratorWriter;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.core.SolrInfoBean;
@@ -169,20 +170,42 @@ public class MetricUtils {
    * @param o an instance of converted metric, either a Map or a flat Object
    */
   static void toSolrInputDocument(String prefix, SolrInputDocument doc, Object o) {
-    if (!(o instanceof Map)) {
-      String key = prefix != null ? prefix : VALUE;
-      doc.addField(key, o);
-      return;
-    }
-    @SuppressWarnings({"unchecked"})
-    Map<String, Object> map = (Map<String, Object>)o;
-    for (Map.Entry<String, Object> entry : map.entrySet()) {
-      if (entry.getValue() instanceof Map) { // flatten recursively
-        toSolrInputDocument(entry.getKey(), doc, entry.getValue());
+    final BiConsumer<Object, Object> consumer = (k, v) -> {
+      if ((v instanceof Map) || (v instanceof MapWriter) || (v instanceof IteratorWriter))
{
+        toSolrInputDocument(k.toString(), doc, v);
       } else {
-        String key = prefix != null ? prefix + "." + entry.getKey() : entry.getKey();
-        doc.addField(key, entry.getValue());
+        String key = prefix != null ? prefix + "." + k : k.toString();
+        doc.addField(key, v);
+      }
+    };
+    if (o instanceof MapWriter) {
+      @SuppressWarnings({"unchecked"})
+      MapWriter writer = (MapWriter) o;
+      writer._forEachEntry(consumer);
+    } else if (o instanceof Map) {
+      @SuppressWarnings({"unchecked"})
+      Map<String, Object> map = (Map<String, Object>) o;
+      for (Map.Entry<String, Object> entry : map.entrySet()) {
+        consumer.accept(entry.getKey(), entry.getValue());
+      }
+    } else if (o instanceof IteratorWriter) {
+      @SuppressWarnings({"unchecked"})
+      IteratorWriter writer = (IteratorWriter) o;
+      final String name = prefix != null ? prefix : "value";
+      try {
+        writer.writeIter(new IteratorWriter.ItemWriter() {
+          @Override
+          public IteratorWriter.ItemWriter add(Object o) throws IOException {
+            consumer.accept(name, o);
+            return this;
+          }
+        });
+      } catch (IOException e) {
+        throw new RuntimeException("this should never happen", e);
       }
+    } else {
+      String key = prefix != null ? prefix : VALUE;
+      doc.addField(key, o);
     }
   }
 
@@ -316,31 +339,30 @@ public class MetricUtils {
         consumer.accept(name + separator + MEAN, metric.getMean());
       }
     } else {
-      Map<String, Object> response = new LinkedHashMap<>();
-      BiConsumer<String, Object> filter = (k, v) -> {
-        if (propertyFilter.accept(k)) {
-          response.put(k, v);
+      MapWriter writer = ew -> {
+        BiConsumer<String, Object> filter = (k, v) -> {
+          if (propertyFilter.accept(k)) {
+            ew.putNoEx(k, v);
+          }
+        };
+        filter.accept("count", metric.size());
+        filter.accept(MAX, metric.getMax());
+        filter.accept(MIN, metric.getMin());
+        filter.accept(MEAN, metric.getMean());
+        filter.accept(STDDEV, metric.getStdDev());
+        filter.accept(SUM, metric.getSum());
+        if (!(metric.isEmpty() || skipAggregateValues)) {
+          ew.putNoEx(VALUES, (MapWriter) ew1 -> {
+            metric.getValues().forEach((k, v) -> {
+              ew1.putNoEx(k, (MapWriter) ew2 -> {
+                ew2.putNoEx("value", v.value);
+                ew2.putNoEx("updateCount", v.updateCount.get());
+              });
+            });
+          });
         }
       };
-      filter.accept("count", metric.size());
-      filter.accept(MAX, metric.getMax());
-      filter.accept(MIN, metric.getMin());
-      filter.accept(MEAN, metric.getMean());
-      filter.accept(STDDEV, metric.getStdDev());
-      filter.accept(SUM, metric.getSum());
-      if (!(metric.isEmpty() || skipAggregateValues)) {
-        Map<String, Object> values = new LinkedHashMap<>();
-        response.put(VALUES, values);
-        metric.getValues().forEach((k, v) -> {
-          Map<String, Object> map = new LinkedHashMap<>();
-          map.put("value", v.value);
-          map.put("updateCount", v.updateCount.get());
-          values.put(k, map);
-        });
-      }
-      if (!response.isEmpty()) {
-        consumer.accept(name, response);
-      }
+      consumer.accept(name, writer);
     }
   }
 
@@ -362,16 +384,15 @@ public class MetricUtils {
         consumer.accept(name + separator + MEAN, snapshot.getMean());
       }
     } else {
-      Map<String, Object> response = new LinkedHashMap<>();
-      String prop = "count";
-      if (propertyFilter.accept(prop)) {
-        response.put(prop, histogram.getCount());
-      }
-      // non-time based values
-      addSnapshot(response, snapshot, propertyFilter, false);
-      if (!response.isEmpty()) {
-        consumer.accept(name, response);
-      }
+      MapWriter writer = ew -> {
+        String prop = "count";
+        if (propertyFilter.accept(prop)) {
+          ew.putNoEx(prop, histogram.getCount());
+        }
+        // non-time based values
+        addSnapshot(ew, snapshot, propertyFilter, false);
+      };
+      consumer.accept(name, writer);
     }
   }
 
@@ -385,10 +406,10 @@ public class MetricUtils {
   }
 
   // some snapshots represent time in ns, other snapshots represent raw values (eg. chunk
size)
-  static void addSnapshot(Map<String, Object> response, Snapshot snapshot, PropertyFilter
propertyFilter, boolean ms) {
+  static void addSnapshot(MapWriter.EntryWriter ew, Snapshot snapshot, PropertyFilter propertyFilter,
boolean ms) {
     BiConsumer<String, Object> filter = (k, v) -> {
       if (propertyFilter.accept(k)) {
-        response.put(k, v);
+        ew.putNoEx(k, v);
       }
     };
     filter.accept((ms ? MIN_MS: MIN), nsToMs(ms, snapshot.getMin()));
@@ -420,24 +441,23 @@ public class MetricUtils {
         consumer.accept(name + separator + prop, timer.getMeanRate());
       }
     } else {
-      Map<String, Object> response = new LinkedHashMap<>();
-      BiConsumer<String,Object> filter = (k, v) -> {
-        if (propertyFilter.accept(k)) {
-          response.put(k, v);
+      MapWriter writer = ew -> {
+        BiConsumer<String,Object> filter = (k, v) -> {
+          if (propertyFilter.accept(k)) {
+            ew.putNoEx(k, v);
+          }
+        };
+        filter.accept("count", timer.getCount());
+        filter.accept("meanRate", timer.getMeanRate());
+        filter.accept("1minRate", timer.getOneMinuteRate());
+        filter.accept("5minRate", timer.getFiveMinuteRate());
+        filter.accept("15minRate", timer.getFifteenMinuteRate());
+        if (!skipHistograms) {
+          // time-based values in nanoseconds
+          addSnapshot(ew, timer.getSnapshot(), propertyFilter, true);
         }
       };
-      filter.accept("count", timer.getCount());
-      filter.accept("meanRate", timer.getMeanRate());
-      filter.accept("1minRate", timer.getOneMinuteRate());
-      filter.accept("5minRate", timer.getFiveMinuteRate());
-      filter.accept("15minRate", timer.getFifteenMinuteRate());
-      if (!skipHistograms) {
-        // time-based values in nanoseconds
-        addSnapshot(response, timer.getSnapshot(), propertyFilter, true);
-      }
-      if (!response.isEmpty()) {
-        consumer.accept(name, response);
-      }
+      consumer.accept(name, writer);
     }
   }
 
@@ -456,20 +476,19 @@ public class MetricUtils {
         consumer.accept(name + separator + "count", meter.getCount());
       }
     } else {
-      Map<String, Object> response = new LinkedHashMap<>();
-      BiConsumer<String, Object> filter = (k, v) -> {
-        if (propertyFilter.accept(k)) {
-          response.put(k, v);
-        }
+      MapWriter writer = ew -> {
+        BiConsumer<String, Object> filter = (k, v) -> {
+          if (propertyFilter.accept(k)) {
+            ew.putNoEx(k, v);
+          }
+        };
+        filter.accept("count", meter.getCount());
+        filter.accept("meanRate", meter.getMeanRate());
+        filter.accept("1minRate", meter.getOneMinuteRate());
+        filter.accept("5minRate", meter.getFiveMinuteRate());
+        filter.accept("15minRate", meter.getFifteenMinuteRate());
       };
-      filter.accept("count", meter.getCount());
-      filter.accept("meanRate", meter.getMeanRate());
-      filter.accept("1minRate", meter.getOneMinuteRate());
-      filter.accept("5minRate", meter.getFiveMinuteRate());
-      filter.accept("15minRate", meter.getFifteenMinuteRate());
-      if (!response.isEmpty()) {
-        consumer.accept(name, response);
-      }
+      consumer.accept(name, writer);
     }
   }
 
@@ -499,15 +518,18 @@ public class MetricUtils {
             }
           }
         } else {
-          Map<String, Object> val = new HashMap<>();
-          for (Map.Entry<?, ?> entry : ((Map<?, ?>)o).entrySet()) {
-            String prop = entry.getKey().toString();
-            if (propertyFilter.accept(prop)) {
-              val.put(prop, entry.getValue());
+          boolean notEmpty = ((Map<?, ?>)o).entrySet().stream()
+              .anyMatch(entry -> propertyFilter.accept(entry.getKey().toString()));
+          MapWriter writer = ew -> {
+            for (Map.Entry<?, ?> entry : ((Map<?, ?>)o).entrySet()) {
+              String prop = entry.getKey().toString();
+              if (propertyFilter.accept(prop)) {
+                ew.putNoEx(prop, entry.getValue());
+              }
             }
-          }
-          if (!val.isEmpty()) {
-            consumer.accept(name, val);
+          };
+          if (notEmpty) {
+            consumer.accept(name, writer);
           }
         }
       } else {
@@ -515,21 +537,24 @@ public class MetricUtils {
       }
     } else {
       Object o = gauge.getValue();
-      Map<String, Object> response = new LinkedHashMap<>();
       if (o instanceof Map) {
-        for (Map.Entry<?, ?> entry : ((Map<?, ?>)o).entrySet()) {
-          String prop = entry.getKey().toString();
-          if (propertyFilter.accept(prop)) {
-            response.put(prop, entry.getValue());
-          }
-        }
-        if (!response.isEmpty()) {
-          consumer.accept(name, Collections.singletonMap("value", response));
+        boolean notEmpty = ((Map<?, ?>)o).entrySet().stream()
+            .anyMatch(entry -> propertyFilter.accept(entry.getKey().toString()));
+        if (notEmpty) {
+          consumer.accept(name, (MapWriter) ew -> {
+            ew.putNoEx("value", (MapWriter) ew1 -> {
+              for (Map.Entry<?, ?> entry : ((Map<?, ?>)o).entrySet()) {
+                String prop = entry.getKey().toString();
+                if (propertyFilter.accept(prop)) {
+                  ew1.put(prop, entry.getValue());
+                }
+              }
+            });
+          });
         }
       } else {
         if (propertyFilter.accept("value")) {
-          response.put("value", o);
-          consumer.accept(name, response);
+          consumer.accept(name, (MapWriter) ew -> ew.putNoEx("value", o));
         }
       }
     }
@@ -547,9 +572,7 @@ public class MetricUtils {
       consumer.accept(name, counter.getCount());
     } else {
       if (propertyFilter.accept("count")) {
-        Map<String, Object> response = new LinkedHashMap<>();
-        response.put("count", counter.getCount());
-        consumer.accept(name, response);
+        consumer.accept(name, (MapWriter) ew -> ew.putNoEx("count", counter.getCount()));
       }
     }
   }
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java
index 4ab33df..cc4e1f6 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java
@@ -18,10 +18,12 @@
 package org.apache.solr.handler.admin;
 
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.Map;
 
 import com.codahale.metrics.Counter;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
@@ -84,15 +86,17 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     assertNotNull(nl);
     Object o = nl.get("SEARCHER.new.errors");
     assertNotNull(o); // counter type
-    assertTrue(o instanceof Map);
+    assertTrue(o instanceof MapWriter);
     // response wasn't serialized so we get here whatever MetricUtils produced instead of
NamedList
-    assertNotNull(((Map) o).get("count"));
-    assertEquals(0L, ((Map) nl.get("SEARCHER.new.errors")).get("count"));
+    assertNotNull(((MapWriter) o)._get("count", null));
+    assertEquals(0L, ((MapWriter) nl.get("SEARCHER.new.errors"))._get("count", null));
     nl = (NamedList) values.get("solr.node");
     assertNotNull(nl.get("CONTAINER.cores.loaded")); // int gauge
-    assertEquals(1, ((Map) nl.get("CONTAINER.cores.loaded")).get("value"));
+    assertEquals(1, ((MapWriter) nl.get("CONTAINER.cores.loaded"))._get("value", null));
     assertNotNull(nl.get("ADMIN./admin/authorization.clientErrors")); // timer type
-    assertEquals(5, ((Map) nl.get("ADMIN./admin/authorization.clientErrors")).size());
+    Map<String, Object> map = new HashMap<>();
+    ((MapWriter) nl.get("ADMIN./admin/authorization.clientErrors")).toMap(map);
+    assertEquals(5, map.size());
 
     resp = new SolrQueryResponse();
     handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", MetricsHandler.COMPACT_PARAM,
"false", CommonParams.WT, "json", "group", "jvm,jetty"), resp);
@@ -184,9 +188,9 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     values = (NamedList) values.get("solr.core.collection1");
     assertEquals(1, values.size());
     @SuppressWarnings({"rawtypes"})
-    Map m = (Map) values.get("CACHE.core.fieldCache");
-    assertNotNull(m);
-    assertNotNull(m.get("entries_count"));
+    MapWriter writer = (MapWriter) values.get("CACHE.core.fieldCache");
+    assertNotNull(writer);
+    assertNotNull(writer._get("entries_count", null));
 
     resp = new SolrQueryResponse();
     handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", MetricsHandler.COMPACT_PARAM,
"false", CommonParams.WT, "json", "group", "jvm", "prefix", "CONTAINER.cores"), resp);
@@ -200,8 +204,8 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     values = resp.getValues();
     assertNotNull(values.get("metrics"));
     @SuppressWarnings({"rawtypes"})
-    SimpleOrderedMap map = (SimpleOrderedMap) values.get("metrics");
-    assertEquals(0, map.size());
+    SimpleOrderedMap map1 = (SimpleOrderedMap) values.get("metrics");
+    assertEquals(0, map1.size());
     handler.close();
   }
 
@@ -243,9 +247,9 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     assertNotNull(nl);
     assertTrue(nl.size() > 0);
     nl.forEach((k, v) -> {
-      assertTrue(v instanceof Map);
-      @SuppressWarnings({"rawtypes"})
-      Map map = (Map) v;
+      assertTrue(v instanceof MapWriter);
+      Map<String, Object> map = new HashMap<>();
+      ((MapWriter) v).toMap(map);
       assertTrue(map.size() > 2);
     });
 
@@ -259,10 +263,10 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     assertNotNull(nl);
     assertTrue(nl.size() > 0);
     nl.forEach((k, v) -> {
-      assertTrue(v instanceof Map);
-      @SuppressWarnings({"rawtypes"})
-      Map map = (Map) v;
-      assertEquals(2, map.size());
+      assertTrue(v instanceof MapWriter);
+      Map<String, Object> map = new HashMap<>();
+      ((MapWriter) v).toMap(map);
+      assertEquals("k=" + k + ", v=" + map, 2, map.size());
       assertNotNull(map.get("inserts"));
       assertNotNull(map.get("size"));
     });
@@ -281,15 +285,19 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     NamedList values = resp.getValues();
     Object val = values.findRecursive("metrics", key1);
     assertNotNull(val);
-    assertTrue(val instanceof Map);
-    assertTrue(((Map) val).size() >= 2);
+    assertTrue(val instanceof MapWriter);
+    Map<String, Object> map = new HashMap<>();
+    ((MapWriter) val).toMap(map);
+    assertTrue(map.size() >= 2);
 
     String key2 = "solr.core.collection1:CACHE.core.fieldCache:entries_count";
     resp = new SolrQueryResponse();
     handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json",
         MetricsHandler.KEY_PARAM, key2), resp);
     values = resp.getValues();
-    val = values.findRecursive("metrics", key2);
+    map = new HashMap<>();
+    values.toMap(map);
+    val = Utils.getObjectByPath(map, true, "metrics/" + key2);
     assertNotNull(val);
     assertTrue(val instanceof Number);
 
@@ -298,7 +306,9 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json",
         MetricsHandler.KEY_PARAM, key3), resp);
     values = resp.getValues();
-    val = values.findRecursive("metrics", key3);
+    map = new HashMap<>();
+    values.toMap(map);
+    val = Utils.getObjectByPath(map, true, "metrics/" + key3);
     assertNotNull(val);
     assertTrue(val instanceof Number);
     assertEquals(3, ((Number) val).intValue());
@@ -308,11 +318,13 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json",
         MetricsHandler.KEY_PARAM, key1, MetricsHandler.KEY_PARAM, key2, MetricsHandler.KEY_PARAM,
key3), resp);
     values = resp.getValues();
-    val = values.findRecursive("metrics", key1);
+    map = new HashMap<>();
+    values.toMap(map);
+    val = Utils.getObjectByPath(map, false, "metrics/" + key1);
     assertNotNull(val);
-    val = values.findRecursive("metrics", key2);
+    val = Utils.getObjectByPath(map, true, "metrics/" + key2);
     assertNotNull(val);
-    val = values.findRecursive("metrics", key3);
+    val = Utils.getObjectByPath(map, true, "metrics/" + key3);
     assertNotNull(val);
 
     String key4 = "solr.core.collection1:QUERY./select.requestTimes:1minRate";
@@ -320,7 +332,10 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
     handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json",
         MetricsHandler.KEY_PARAM, key4), resp);
     values = resp.getValues();
-    val = values.findRecursive("metrics", key4);
+    map = new HashMap<>();
+    values.toMap(map);
+    // the key contains a slash, need explicit list of path elements
+    val = Utils.getObjectByPath(map, true, Arrays.asList("metrics", key4));
     assertNotNull(val);
     assertTrue(val instanceof Number);
 
diff --git a/solr/core/src/test/org/apache/solr/util/stats/MetricUtilsTest.java b/solr/core/src/test/org/apache/solr/util/stats/MetricUtilsTest.java
index f9595e1..efc6c94 100644
--- a/solr/core/src/test/org/apache/solr/util/stats/MetricUtilsTest.java
+++ b/solr/core/src/test/org/apache/solr/util/stats/MetricUtilsTest.java
@@ -31,6 +31,7 @@ import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.Snapshot;
 import com.codahale.metrics.Timer;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.metrics.AggregateMetric;
 import org.junit.Test;
@@ -49,7 +50,7 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
     // obtain timer metrics
     Map<String,Object> map = new HashMap<>();
     MetricUtils.convertTimer("", timer, MetricUtils.PropertyFilter.ALL, false, false, ".",
(k, v) -> {
-      map.putAll((Map<String,Object>)v);
+      ((MapWriter) v).toMap(map);
     });
     @SuppressWarnings({"rawtypes"})
     NamedList lst = new NamedList(map);
@@ -106,7 +107,10 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
     MetricUtils.toMaps(registry, Collections.singletonList(MetricFilter.ALL), MetricFilter.ALL,
         MetricUtils.PropertyFilter.ALL, false, false, false, false, (k, o) -> {
       @SuppressWarnings({"rawtypes"})
-      Map v = (Map)o;
+      Map<String, Object> v = new HashMap<>();
+      if (o != null) {
+        ((MapWriter) o).toMap(v);
+      }
       if (k.startsWith("counter")) {
         assertEquals(1L, v.get("count"));
       } else if (k.startsWith("gauge")) {
@@ -139,7 +143,7 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
         assertEquals(0D, v.get("max"));
         assertEquals(0D, v.get("mean"));
       } else if (k.startsWith("memory.expected.error")) {
-        assertNull(v);
+        assertTrue(v.isEmpty());
       }
     });
     // test compact format
@@ -152,25 +156,25 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
             assertTrue(o instanceof String);
             assertEquals("foobar", o);
           } else if (k.startsWith("timer")) {
-            assertTrue(o instanceof Map);
-            @SuppressWarnings({"rawtypes"})
-            Map v = (Map)o;
+            assertTrue(o instanceof MapWriter);
+            Map<String, Object> v = new HashMap<>();
+            ((MapWriter) o).toMap(v);
             assertEquals(1L, v.get("count"));
             assertTrue(((Number)v.get("min_ms")).intValue() > 100);
           } else if (k.startsWith("meter")) {
-            assertTrue(o instanceof Map);
-            @SuppressWarnings({"rawtypes"})
-            Map v = (Map)o;
+            assertTrue(o instanceof MapWriter);
+            Map<String, Object> v = new HashMap<>();
+            ((MapWriter) o).toMap(v);
             assertEquals(1L, v.get("count"));
           } else if (k.startsWith("histogram")) {
-            assertTrue(o instanceof Map);
-            @SuppressWarnings({"rawtypes"})
-            Map v = (Map)o;
+            assertTrue(o instanceof MapWriter);
+            Map<String, Object> v = new HashMap<>();
+            ((MapWriter) o).toMap(v);
             assertEquals(1L, v.get("count"));
           } else if (k.startsWith("aggregate1")) {
-            assertTrue(o instanceof Map);
-            @SuppressWarnings({"rawtypes"})
-            Map v = (Map)o;
+            assertTrue(o instanceof MapWriter);
+            Map<String, Object> v = new HashMap<>();
+            ((MapWriter) o).toMap(v);
             assertEquals(4, v.get("count"));
             Map<String, Object> values = (Map<String, Object>)v.get("values");
             assertNotNull(values);
@@ -182,9 +186,9 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
             assertEquals(-2, update.get("value"));
             assertEquals(2, update.get("updateCount"));
           } else if (k.startsWith("aggregate2")) {
-            assertTrue(o instanceof Map);
-            @SuppressWarnings({"rawtypes"})
-            Map v = (Map)o;
+            assertTrue(o instanceof MapWriter);
+            Map<String, Object> v = new HashMap<>();
+            ((MapWriter) o).toMap(v);
             assertEquals(2, v.get("count"));
             Map<String, Object> values = (Map<String, Object>)v.get("values");
             assertNotNull(values);
@@ -198,8 +202,9 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
           } else if (k.startsWith("memory.expected.error")) {
             assertNull(o);
           } else {
-            @SuppressWarnings({"rawtypes"})
-            Map v = (Map)o;
+            assertTrue(o instanceof MapWriter);
+            Map<String, Object> v = new HashMap<>();
+            ((MapWriter) o).toMap(v);
             assertEquals(1L, v.get("count"));
           }
         });
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 3cb817b..c05ce23 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
@@ -227,9 +227,48 @@ public class Utils {
     @Override
     @SuppressWarnings({"rawtypes"})
     public void handleUnknownClass(Object o) {
+      // avoid materializing MapWriter / IteratorWriter to Map / List
+      // instead serialize them directly
       if (o instanceof MapWriter) {
-        Map m = ((MapWriter) o).toMap(new LinkedHashMap<>());
-        write(m);
+        MapWriter mapWriter = (MapWriter) o;
+        startObject();
+        final boolean[] first = new boolean[1];
+        first[0] = true;
+        mapWriter._forEachEntry((k, v) -> {
+          if (first[0]) {
+            first[0] = false;
+          } else {
+            writeValueSeparator();
+          }
+          indent();
+          writeString(k.toString());
+          writeNameSeparator();
+          write(v);
+        });
+        endObject();
+      } else if (o instanceof IteratorWriter) {
+        IteratorWriter iteratorWriter = (IteratorWriter) o;
+        startArray();
+        final boolean[] first = new boolean[1];
+        first[0] = true;
+        try {
+          iteratorWriter.writeIter(new IteratorWriter.ItemWriter() {
+            @Override
+            public IteratorWriter.ItemWriter add(Object o) throws IOException {
+              if (first[0]) {
+                first[0] = false;
+              } else {
+                writeValueSeparator();
+              }
+              indent();
+              write(o);
+              return this;
+            }
+          });
+        } catch (IOException e) {
+          throw new RuntimeException("this should never happen", e);
+        }
+        endArray();
       } else {
         super.handleUnknownClass(o);
       }
@@ -239,13 +278,13 @@ public class Utils {
   public static byte[] toJSON(Object o) {
     if (o == null) return new byte[0];
     CharArr out = new CharArr();
-    if (!(o instanceof List) && !(o instanceof Map)) {
-      if (o instanceof MapWriter) {
-        o = ((MapWriter) o).toMap(new LinkedHashMap<>());
-      } else if (o instanceof IteratorWriter) {
-        o = ((IteratorWriter) o).toList(new ArrayList<>());
-      }
-    }
+//    if (!(o instanceof List) && !(o instanceof Map)) {
+//      if (o instanceof MapWriter) {
+//        o = ((MapWriter) o).toMap(new LinkedHashMap<>());
+//      } else if (o instanceof IteratorWriter) {
+//        o = ((IteratorWriter) o).toList(new ArrayList<>());
+//      }
+//    }
     new MapWriterJSONWriter(out, 2).write(o); // indentation by default
     return toUTF8(out);
   }


Mime
View raw message