Speed up String array writes to XContent (#98957)

Jackson has a direct method for writing string arrays that saves us some of the indirection we have when looping over a string array. This normally doesn't gain much, but for extreme cases like long index name lists in field caps it saves a couple percent in CPU time.
elastic · Aug 30, 2023 · 37d55da · 37d55da
1 parent b6c85db
commit 37d55da
Show file tree

Hide file tree

Showing 18 changed files with 51 additions and 85 deletions.
diff --git a/...nt/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentGenerator.java b/...nt/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentGenerator.java
@@ -364,6 +364,31 @@ public void writeString(String value) throws IOException {
         }
     }
 
+    @Override
+    public void writeStringArray(String[] array) throws IOException {
+        try {
+            if (isFiltered()) {
+                // filtered serialization does not work correctly with the bulk array serializer, so we need to fall back to serializing
+                // the array one-by-one
+                // TODO: this can probably be removed after upgrading Jackson to 2.15.1 or later, see
+                // https://github.com/FasterXML/jackson-core/issues/1023
+                writeStringArrayFiltered(array);
+            } else {
+                generator.writeArray(array, 0, array.length);
+            }
+        } catch (JsonGenerationException e) {
+            throw new XContentGenerationException(e);
+        }
+    }
+
+    private void writeStringArrayFiltered(String[] array) throws IOException {
+        writeStartArray();
+        for (String s : array) {
+            writeString(s);
+        }
+        writeEndArray();
+    }
+
     @Override
     public void writeString(char[] value, int offset, int len) throws IOException {
         try {

diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentBuilder.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentBuilder.java
@@ -740,11 +740,7 @@ private XContentBuilder values(String[] values) throws IOException {
         if (values == null) {
             return nullValue();
         }
-        startArray();
-        for (String s : values) {
-            value(s);
-        }
-        endArray();
+        generator.writeStringArray(values);
         return this;
     }
 
@@ -1055,8 +1051,7 @@ public XContentBuilder stringStringMap(String name, Map<String, String> values)
         }
         startObject();
         for (Map.Entry<String, String> value : values.entrySet()) {
-            field(value.getKey());
-            value(value.getValue());
+            generator.writeStringField(value.getKey(), value.getValue());
         }
         return endObject();
     }

diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentGenerator.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentGenerator.java
@@ -76,6 +76,8 @@ public interface XContentGenerator extends Closeable, Flushable {
 
     void writeString(String value) throws IOException;
 
+    void writeStringArray(String[] array) throws IOException;
+
     void writeString(char[] text, int offset, int len) throws IOException;
 
     void writeUTF8String(byte[] value, int offset, int length) throws IOException;

diff --git a/...ain/java/org/elasticsearch/action/admin/cluster/snapshots/clone/CloneSnapshotRequest.java b/...ain/java/org/elasticsearch/action/admin/cluster/snapshots/clone/CloneSnapshotRequest.java
@@ -139,11 +139,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         builder.field("source", source);
         builder.field("target", target);
         if (indices != null) {
-            builder.startArray("indices");
-            for (String index : indices) {
-                builder.value(index);
-            }
-            builder.endArray();
+            builder.array("indices", indices);
         }
         if (indicesOptions != null) {
             indicesOptions.toXContent(builder, params);

diff --git a/...n/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java b/...n/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java
@@ -428,17 +428,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         builder.startObject();
         builder.field("repository", repository);
         builder.field("snapshot", snapshot);
-        builder.startArray("indices");
-        for (String index : indices) {
-            builder.value(index);
-        }
-        builder.endArray();
+        builder.array("indices", indices);
         if (featureStates != null) {
-            builder.startArray("feature_states");
-            for (String plugin : featureStates) {
-                builder.value(plugin);
-            }
-            builder.endArray();
+            builder.array("feature_states", featureStates);
         }
         builder.field("partial", partial);
         builder.field("include_global_state", includeGlobalState);

diff --git a/...java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/...java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java
@@ -580,11 +580,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
     }
 
     private void toXContentFragment(XContentBuilder builder, Params params) throws IOException {
-        builder.startArray("indices");
-        for (String index : indices) {
-            builder.value(index);
-        }
-        builder.endArray();
+        builder.array("indices", indices);
         if (indicesOptions != null) {
             indicesOptions.toXContent(builder, params);
         }
@@ -595,11 +591,7 @@ private void toXContentFragment(XContentBuilder builder, Params params) throws I
             builder.field("rename_replacement", renameReplacement);
         }
         if (featureStates != null && featureStates.length > 0) {
-            builder.startArray("feature_states");
-            for (String plugin : featureStates) {
-                builder.value(plugin);
-            }
-            builder.endArray();
+            builder.array("feature_states", featureStates);
         }
         builder.field("include_global_state", includeGlobalState);
         builder.field("partial", partial);
@@ -611,11 +603,7 @@ private void toXContentFragment(XContentBuilder builder, Params params) throws I
             }
             builder.endObject();
         }
-        builder.startArray("ignore_index_settings");
-        for (String ignoreIndexSetting : ignoreIndexSettings) {
-            builder.value(ignoreIndexSetting);
-        }
-        builder.endArray();
+        builder.array("ignore_index_settings", ignoreIndexSettings);
     }
 
     @Override

diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java
@@ -300,9 +300,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             List<Map.Entry<String, Set<String>>> entries = new ArrayList<>(meta.entrySet());
             entries.sort(Map.Entry.comparingByKey()); // provide predictable order
             for (Map.Entry<String, Set<String>> entry : entries) {
-                List<String> values = new ArrayList<>(entry.getValue());
-                values.sort(String::compareTo); // provide predictable order
-                builder.stringListField(entry.getKey(), values);
+                String[] values = entry.getValue().toArray(Strings.EMPTY_ARRAY);
+                Arrays.sort(values, String::compareTo); // provide predictable order
+                builder.array(entry.getKey(), values);
             }
             builder.endObject();
         }

diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateHandlerMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateHandlerMetadata.java
@@ -59,7 +59,7 @@ public static ReservedStateHandlerMetadata readFrom(StreamInput in) throws IOExc
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(name());
-        builder.stringListField(KEYS.getPreferredName(), keys().stream().sorted().toList()); // ordered keys for output consistency
+        builder.array(KEYS.getPreferredName(), keys().stream().sorted().toArray(String[]::new)); // ordered keys for output consistency
         builder.endObject();
         return builder;
     }

diff --git a/...va/org/elasticsearch/search/aggregations/pipeline/AbstractPipelineAggregationBuilder.java b/...va/org/elasticsearch/search/aggregations/pipeline/AbstractPipelineAggregationBuilder.java
@@ -92,11 +92,7 @@ public final XContentBuilder toXContent(XContentBuilder builder, Params params)
         builder.startObject(type);
 
         if (overrideBucketsPath() == false && bucketsPaths != null) {
-            builder.startArray(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName());
-            for (String path : bucketsPaths) {
-                builder.value(path);
-            }
-            builder.endArray();
+            builder.array(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName(), bucketsPaths);
         }
 
         internalXContent(builder, params);

diff --git a/...c/main/java/org/elasticsearch/search/aggregations/pipeline/InternalBucketMetricValue.java b/...c/main/java/org/elasticsearch/search/aggregations/pipeline/InternalBucketMetricValue.java
@@ -96,11 +96,7 @@ public XContentBuilder doXContentBody(XContentBuilder builder, Params params) th
         if (hasValue && format != DocValueFormat.RAW) {
             builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), format.format(value).toString());
         }
-        builder.startArray(KEYS_FIELD.getPreferredName());
-        for (String key : keys) {
-            builder.value(key);
-        }
-        builder.endArray();
+        builder.array(KEYS_FIELD.getPreferredName(), keys);
         return builder;
     }
 

diff --git a/...rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/...rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java
@@ -490,7 +490,7 @@ public void test() throws IOException {
             final XContentBuilder template = jsonBuilder();
             template.startObject();
             {
-                template.startArray("index_patterns").value("*").endArray();
+                template.array("index_patterns", "*");
                 if (useComponentTemplate) {
                     template.field("priority", 4); // relatively low priority, but hopefully uncommon enough not to conflict
                     template.startObject("template");

diff --git a/x-pack/plugin/ccr/qa/src/main/java/org/elasticsearch/xpack/ccr/ESCCRRestTestCase.java b/x-pack/plugin/ccr/qa/src/main/java/org/elasticsearch/xpack/ccr/ESCCRRestTestCase.java
@@ -347,11 +347,7 @@ protected static void createAutoFollowPattern(
         try (XContentBuilder bodyBuilder = JsonXContent.contentBuilder()) {
             bodyBuilder.startObject();
             {
-                bodyBuilder.startArray("leader_index_patterns");
-                {
-                    bodyBuilder.value(pattern);
-                }
-                bodyBuilder.endArray();
+                bodyBuilder.array("leader_index_patterns", pattern);
                 if (followIndexPattern != null) {
                     bodyBuilder.field("follow_index_pattern", followIndexPattern);
                 }

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PostStartBasicResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PostStartBasicResponse.java
@@ -110,11 +110,7 @@ protected void addCustomFields(XContentBuilder builder, Params params) throws IO
             builder.startObject("acknowledge");
             builder.field(MESSAGE_FIELD.getPreferredName(), acknowledgeMessage);
             for (Map.Entry<String, String[]> entry : acknowledgeMessages.entrySet()) {
-                builder.startArray(entry.getKey());
-                for (String message : entry.getValue()) {
-                    builder.value(message);
-                }
-                builder.endArray();
+                builder.array(entry.getKey(), entry.getValue());
             }
             builder.endObject();
         }

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/RestPostStartTrialLicense.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/RestPostStartTrialLicense.java
@@ -56,11 +56,7 @@ public RestResponse buildResponse(PostStartTrialResponse response, XContentBuild
                     builder.startObject("acknowledge");
                     builder.field("message", response.getAcknowledgementMessage());
                     for (Map.Entry<String, String[]> entry : acknowledgementMessages.entrySet()) {
-                        builder.startArray(entry.getKey());
-                        for (String message : entry.getValue()) {
-                            builder.value(message);
-                        }
-                        builder.endArray();
+                        builder.array(entry.getKey(), entry.getValue());
                     }
                     builder.endObject();
                 }

diff --git a/...lugin/core/src/main/java/org/elasticsearch/protocol/xpack/license/PutLicenseResponse.java b/...lugin/core/src/main/java/org/elasticsearch/protocol/xpack/license/PutLicenseResponse.java
@@ -82,11 +82,7 @@ protected void addCustomFields(XContentBuilder builder, Params params) throws IO
             builder.startObject("acknowledge");
             builder.field("message", acknowledgeHeader);
             for (Map.Entry<String, String[]> entry : acknowledgeMessages.entrySet()) {
-                builder.startArray(entry.getKey());
-                for (String message : entry.getValue()) {
-                    builder.value(message);
-                }
-                builder.endArray();
+                builder.array(entry.getKey(), entry.getValue());
             }
             builder.endObject();
         }

diff --git a/.../src/main/java/org/elasticsearch/xpack/core/watcher/execution/WatchExecutionSnapshot.java b/.../src/main/java/org/elasticsearch/xpack/core/watcher/execution/WatchExecutionSnapshot.java
@@ -112,11 +112,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         builder.timeField("execution_time", executionTime);
         builder.field("execution_phase", phase);
         if (executedActions != null) {
-            builder.startArray("executed_actions");
-            for (String executedAction : executedActions) {
-                builder.value(executedAction);
-            }
-            builder.endArray();
+            builder.array("executed_actions", executedActions);
         }
         if (params.paramAsBoolean("emit_stacktraces", false)) {
             builder.startArray("stack_trace");

diff --git a/...ownsample/src/main/java/org/elasticsearch/xpack/downsample/TransportDownsampleAction.java b/...ownsample/src/main/java/org/elasticsearch/xpack/downsample/TransportDownsampleAction.java
@@ -595,12 +595,12 @@ private static void addMetricFieldMapping(final XContentBuilder builder, final S
             // only one value (the last value of the counter)
             builder.startObject(field).field("type", fieldProperties.get("type")).field(TIME_SERIES_METRIC_PARAM, metricType).endObject();
         } else {
-            final List<String> supportedAggs = List.of(metricType.supportedAggs());
+            final String[] supportedAggsArray = metricType.supportedAggs();
             // We choose max as the default metric
-            final String defaultMetric = supportedAggs.contains("max") ? "max" : supportedAggs.get(0);
+            final String defaultMetric = List.of(supportedAggsArray).contains("max") ? "max" : supportedAggsArray[0];
             builder.startObject(field)
                 .field("type", AggregateDoubleMetricFieldMapper.CONTENT_TYPE)
-                .stringListField(AggregateDoubleMetricFieldMapper.Names.METRICS, supportedAggs)
+                .array(AggregateDoubleMetricFieldMapper.Names.METRICS, supportedAggsArray)
                 .field(AggregateDoubleMetricFieldMapper.Names.DEFAULT_METRIC, defaultMetric)
                 .field(TIME_SERIES_METRIC_PARAM, metricType)
                 .endObject();

diff --git a/.../src/main/java/org/elasticsearch/xpack/watcher/notification/slack/message/Attachment.java b/.../src/main/java/org/elasticsearch/xpack/watcher/notification/slack/message/Attachment.java
@@ -162,11 +162,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             builder.field(XField.THUMB_URL.getPreferredName(), thumbUrl);
         }
         if (markdownSupportedFields != null) {
-            builder.startArray(XField.MARKDOWN_IN.getPreferredName());
-            for (String field : markdownSupportedFields) {
-                builder.value(field);
-            }
-            builder.endArray();
+            builder.array(XField.MARKDOWN_IN.getPreferredName(), markdownSupportedFields);
         }
         if (actions != null && actions.isEmpty() == false) {
             builder.startArray("actions");