From 905eb422f67ab14d307b79c9c215eadb882944e2 Mon Sep 17 00:00:00 2001 From: Koen De Groote Date: Thu, 18 May 2017 12:02:29 +0200 Subject: [PATCH] Use StringBuilder to construct a String instead of relying on appending where possible (#24753) This PR revolves around places in the code where introducing a StringBuilder might make the construction of a String easier to follow and also, maybe avoid a case where the compiler's very safe way of introducing StringBuilder instead of String might not always be optimal for performance. --- .../elasticsearch/client/RequestLogger.java | 11 +++++----- .../elasticsearch/monitor/jvm/JvmInfo.java | 12 +++++------ .../elasticsearch/rest/BaseRestHandler.java | 18 +++++++++++------ .../ingest/RandomDocumentPicks.java | 8 ++++---- .../hamcrest/ElasticsearchAssertions.java | 20 +++++++++++-------- 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java b/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java index ad2348762dd..07ff89b7e3f 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java @@ -139,11 +139,12 @@ final class RequestLogger { * Creates curl output for given response */ static String buildTraceResponse(HttpResponse httpResponse) throws IOException { - String responseLine = "# " + httpResponse.getStatusLine().toString(); + StringBuilder responseLine = new StringBuilder(); + responseLine.append("# ").append(httpResponse.getStatusLine()); for (Header header : httpResponse.getAllHeaders()) { - responseLine += "\n# " + header.getName() + ": " + header.getValue(); + responseLine.append("\n# ").append(header.getName()).append(": ").append(header.getValue()); } - responseLine += "\n#"; + responseLine.append("\n#"); HttpEntity entity = httpResponse.getEntity(); if (entity != null) { if (entity.isRepeatable() == false) { @@ -158,11 +159,11 @@ final class RequestLogger { try (BufferedReader reader = new BufferedReader(new InputStreamReader(entity.getContent(), charset))) { String line; while( (line = reader.readLine()) != null) { - responseLine += "\n# " + line; + responseLine.append("\n# ").append(line); } } } - return responseLine; + return responseLine.toString(); } private static String getUri(RequestLine requestLine) { diff --git a/core/src/main/java/org/elasticsearch/monitor/jvm/JvmInfo.java b/core/src/main/java/org/elasticsearch/monitor/jvm/JvmInfo.java index c18a1c5e3fd..1674ba33c01 100644 --- a/core/src/main/java/org/elasticsearch/monitor/jvm/JvmInfo.java +++ b/core/src/main/java/org/elasticsearch/monitor/jvm/JvmInfo.java @@ -299,19 +299,19 @@ public class JvmInfo implements Writeable, ToXContent { public int versionAsInteger() { try { int i = 0; - String sVersion = ""; + StringBuilder sVersion = new StringBuilder(); for (; i < version.length(); i++) { if (!Character.isDigit(version.charAt(i)) && version.charAt(i) != '.') { break; } if (version.charAt(i) != '.') { - sVersion += version.charAt(i); + sVersion.append(version.charAt(i)); } } if (i == 0) { return -1; } - return Integer.parseInt(sVersion); + return Integer.parseInt(sVersion.toString()); } catch (Exception e) { return -1; } @@ -320,19 +320,19 @@ public class JvmInfo implements Writeable, ToXContent { public int versionUpdatePack() { try { int i = 0; - String sVersion = ""; + StringBuilder sVersion = new StringBuilder(); for (; i < version.length(); i++) { if (!Character.isDigit(version.charAt(i)) && version.charAt(i) != '.') { break; } if (version.charAt(i) != '.') { - sVersion += version.charAt(i); + sVersion.append(version.charAt(i)); } } if (i == 0) { return -1; } - Integer.parseInt(sVersion); + Integer.parseInt(sVersion.toString()); int from; if (version.charAt(i) == '_') { // 1.7.0_4 diff --git a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java index 81620ec8a7c..3db635b044c 100644 --- a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java +++ b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java @@ -85,12 +85,12 @@ public abstract class BaseRestHandler extends AbstractComponent implements RestH final Set invalids, final Set candidates, final String detail) { - String message = String.format( + StringBuilder message = new StringBuilder(String.format( Locale.ROOT, "request [%s] contains unrecognized %s%s: ", request.path(), detail, - invalids.size() > 1 ? "s" : ""); + invalids.size() > 1 ? "s" : "")); boolean first = true; for (final String invalid : invalids) { final LevensteinDistance ld = new LevensteinDistance(); @@ -108,17 +108,23 @@ public abstract class BaseRestHandler extends AbstractComponent implements RestH else return a.v2().compareTo(b.v2()); }); if (first == false) { - message += ", "; + message.append(", "); } - message += "[" + invalid + "]"; + message.append("[").append(invalid).append("]"); final List keys = scoredParams.stream().map(Tuple::v2).collect(Collectors.toList()); if (keys.isEmpty() == false) { - message += " -> did you mean " + (keys.size() == 1 ? "[" + keys.get(0) + "]" : "any of " + keys.toString()) + "?"; + message.append(" -> did you mean "); + if (keys.size() == 1) { + message.append("[").append(keys.get(0)).append("]"); + } else { + message.append("any of ").append(keys.toString()); + } + message.append("?"); } first = false; } - return message; + return message.toString(); } /** diff --git a/test/framework/src/main/java/org/elasticsearch/ingest/RandomDocumentPicks.java b/test/framework/src/main/java/org/elasticsearch/ingest/RandomDocumentPicks.java index f1c19710850..fb5c701908f 100644 --- a/test/framework/src/main/java/org/elasticsearch/ingest/RandomDocumentPicks.java +++ b/test/framework/src/main/java/org/elasticsearch/ingest/RandomDocumentPicks.java @@ -42,14 +42,14 @@ public final class RandomDocumentPicks { */ public static String randomFieldName(Random random) { int numLevels = RandomNumbers.randomIntBetween(random, 1, 5); - String fieldName = ""; + StringBuilder fieldName = new StringBuilder(); for (int i = 0; i < numLevels; i++) { if (i > 0) { - fieldName += "."; + fieldName.append('.'); } - fieldName += randomString(random); + fieldName.append(randomString(random)); } - return fieldName; + return fieldName.toString(); } /** diff --git a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java index 4e57503df4b..94be3486244 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java +++ b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java @@ -196,21 +196,25 @@ public class ElasticsearchAssertions { } public static String formatShardStatus(BroadcastResponse response) { - String msg = " Total shards: " + response.getTotalShards() + " Successful shards: " + response.getSuccessfulShards() + " & " - + response.getFailedShards() + " shard failures:"; + StringBuilder msg = new StringBuilder(); + msg.append(" Total shards: ").append(response.getTotalShards()) + .append(" Successful shards: ").append(response.getSuccessfulShards()) + .append(" & ").append(response.getFailedShards()).append(" shard failures:"); for (ShardOperationFailedException failure : response.getShardFailures()) { - msg += "\n " + failure.toString(); + msg.append("\n ").append(failure); } - return msg; + return msg.toString(); } public static String formatShardStatus(SearchResponse response) { - String msg = " Total shards: " + response.getTotalShards() + " Successful shards: " + response.getSuccessfulShards() + " & " - + response.getFailedShards() + " shard failures:"; + StringBuilder msg = new StringBuilder(); + msg.append(" Total shards: ").append(response.getTotalShards()) + .append(" Successful shards: ").append(response.getSuccessfulShards()) + .append(" & ").append(response.getFailedShards()).append(" shard failures:"); for (ShardSearchFailure failure : response.getShardFailures()) { - msg += "\n " + failure.toString(); + msg.append("\n ").append(failure); } - return msg; + return msg.toString(); } public static void assertNoSearchHits(SearchResponse searchResponse) {