Skip to content

Commit 556dbee

Browse files
Make Parsing SnapshotInfo more Efficient (#74005) (#74047)
Flatting the logic for parsing `SnapshotInfo` to go field by field like we do for `RepositoryData` which is both easier to read and also faster (mostly when moving to batch multiple of these blobs into one and doing on-the-fly filtering in an upcoming PR where the approach allows for more tricks). Also, simplified/deduplicated parsing out (mostly/often) empty lists in the deserialization code and used the new utility in a few more spots as well to save empty lists.
1 parent 92abe73 commit 556dbee

File tree

8 files changed

+143
-123
lines changed

8 files changed

+143
-123
lines changed

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponse.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public CreateSnapshotResponse(@Nullable SnapshotInfo snapshotInfo) {
5454

5555
public CreateSnapshotResponse(StreamInput in) throws IOException {
5656
super(in);
57-
snapshotInfo = in.readOptionalWriteable(SnapshotInfo::new);
57+
snapshotInfo = in.readOptionalWriteable(SnapshotInfo::readFrom);
5858
}
5959

6060
private void setSnapshotInfoFromBuilder(SnapshotInfoBuilder snapshotInfoBuilder) {

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public GetSnapshotsResponse(List<SnapshotInfo> snapshots) {
5353

5454
GetSnapshotsResponse(StreamInput in) throws IOException {
5555
super(in);
56-
snapshots = Collections.unmodifiableList(in.readList(SnapshotInfo::new));
56+
snapshots = Collections.unmodifiableList(in.readList(SnapshotInfo::readFrom));
5757
}
5858

5959
/**

server/src/main/java/org/elasticsearch/common/settings/Setting.java

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@
2727
import org.elasticsearch.common.xcontent.XContentBuilder;
2828
import org.elasticsearch.common.xcontent.XContentFactory;
2929
import org.elasticsearch.common.xcontent.XContentParser;
30+
import org.elasticsearch.common.xcontent.XContentParserUtils;
3031
import org.elasticsearch.common.xcontent.XContentType;
3132

3233
import java.io.IOException;
33-
import java.util.ArrayList;
3434
import java.util.Arrays;
3535
import java.util.Collections;
3636
import java.util.EnumSet;
@@ -1535,18 +1535,11 @@ private static List<String> parseableStringToList(String parsableString) {
15351535
// fromXContent doesn't use named xcontent or deprecation.
15361536
try (XContentParser xContentParser = XContentType.JSON.xContent()
15371537
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, parsableString)) {
1538-
XContentParser.Token token = xContentParser.nextToken();
1539-
if (token != XContentParser.Token.START_ARRAY) {
1540-
throw new IllegalArgumentException("expected START_ARRAY but got " + token);
1541-
}
1542-
ArrayList<String> list = new ArrayList<>();
1543-
while ((token = xContentParser.nextToken()) != XContentParser.Token.END_ARRAY) {
1544-
if (token != XContentParser.Token.VALUE_STRING) {
1545-
throw new IllegalArgumentException("expected VALUE_STRING but got " + token);
1546-
}
1547-
list.add(xContentParser.text());
1548-
}
1549-
return list;
1538+
xContentParser.nextToken();
1539+
return XContentParserUtils.parseList(xContentParser, p -> {
1540+
XContentParserUtils.ensureExpectedToken(XContentParser.Token.VALUE_STRING, p.currentToken(), p);
1541+
return p.text();
1542+
});
15501543
} catch (IOException e) {
15511544
throw new IllegalArgumentException("failed to parse array", e);
15521545
}

server/src/main/java/org/elasticsearch/common/xcontent/XContentParserUtils.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@
1212
import org.elasticsearch.common.Strings;
1313
import org.elasticsearch.common.bytes.BytesArray;
1414
import org.elasticsearch.common.xcontent.XContentParser.Token;
15+
import org.elasticsearch.core.CheckedFunction;
1516

1617
import java.io.IOException;
18+
import java.util.ArrayList;
19+
import java.util.List;
1720
import java.util.Locale;
1821
import java.util.function.Consumer;
1922

@@ -151,4 +154,25 @@ public static <T> void parseTypedKeysObject(XContentParser parser, String delimi
151154
throw new ParsingException(parser.getTokenLocation(), "Failed to parse object: empty key");
152155
}
153156
}
157+
158+
/**
159+
* Parses a list of a given type from the given {@code parser}. Assumes that the parser is currently positioned on a
160+
* {@link Token#START_ARRAY} token and will fail if it is not. The returned list may or may not be mutable.
161+
*
162+
* @param parser x-content parser
163+
* @param valueParser parser for expected list value type
164+
* @return list parsed from parser
165+
*/
166+
public static <T> List<T> parseList(XContentParser parser,
167+
CheckedFunction<XContentParser, T, IOException> valueParser) throws IOException {
168+
XContentParserUtils.ensureExpectedToken(Token.START_ARRAY, parser.currentToken(), parser);
169+
if (parser.nextToken() == Token.END_ARRAY) {
170+
return org.elasticsearch.core.List.of();
171+
}
172+
final ArrayList<T> list = new ArrayList<>();
173+
do {
174+
list.add(valueParser.apply(parser));
175+
} while (parser.nextToken() != Token.END_ARRAY);
176+
return list;
177+
}
154178
}

server/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
import org.elasticsearch.index.store.StoreFileMetadata;
2424

2525
import java.io.IOException;
26-
import java.util.ArrayList;
27-
import java.util.Collections;
2826
import java.util.List;
2927
import java.util.stream.IntStream;
3028

@@ -379,7 +377,7 @@ public BlobStoreIndexShardSnapshot(
379377
assert indexVersion >= 0;
380378
this.snapshot = snapshot;
381379
this.indexVersion = indexVersion;
382-
this.indexFiles = Collections.unmodifiableList(new ArrayList<>(indexFiles));
380+
this.indexFiles = org.elasticsearch.core.List.copyOf(indexFiles);
383381
this.startTime = startTime;
384382
this.time = time;
385383
this.incrementalFileCount = incrementalFileCount;
@@ -516,7 +514,7 @@ public static BlobStoreIndexShardSnapshot fromXContent(XContentParser parser) th
516514
int incrementalFileCount = 0;
517515
long incrementalSize = 0;
518516

519-
List<FileInfo> indexFiles = new ArrayList<>();
517+
List<FileInfo> indexFiles = null;
520518
if (parser.currentToken() == null) { // fresh parser? move to the first token
521519
parser.nextToken();
522520
}
@@ -545,9 +543,7 @@ public static BlobStoreIndexShardSnapshot fromXContent(XContentParser parser) th
545543
}
546544
} else if (token == XContentParser.Token.START_ARRAY) {
547545
if (PARSE_FILES.match(currentFieldName, parser.getDeprecationHandler())) {
548-
while ((parser.nextToken()) != XContentParser.Token.END_ARRAY) {
549-
indexFiles.add(FileInfo.fromXContent(parser));
550-
}
546+
indexFiles = XContentParserUtils.parseList(parser, FileInfo::fromXContent);
551547
} else {
552548
XContentParserUtils.throwUnknownField(currentFieldName, parser.getTokenLocation());
553549
}
@@ -559,7 +555,7 @@ public static BlobStoreIndexShardSnapshot fromXContent(XContentParser parser) th
559555
return new BlobStoreIndexShardSnapshot(
560556
snapshot,
561557
indexVersion,
562-
Collections.unmodifiableList(indexFiles),
558+
indexFiles == null ? org.elasticsearch.core.List.of() : indexFiles,
563559
startTime,
564560
time,
565561
incrementalFileCount,

server/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshots.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -273,11 +273,7 @@ public static BlobStoreIndexShardSnapshots fromXContent(XContentParser parser) t
273273
currentFieldName = parser.currentName();
274274
if (ParseFields.FILES.match(currentFieldName, parser.getDeprecationHandler())
275275
&& parser.nextToken() == XContentParser.Token.START_ARRAY) {
276-
List<String> fileNames = new ArrayList<>();
277-
while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
278-
fileNames.add(parser.text());
279-
}
280-
snapshotsMap.put(snapshot, fileNames);
276+
snapshotsMap.put(snapshot, XContentParserUtils.parseList(parser, XContentParser::text));
281277
} else if (ParseFields.SHARD_STATE_ID.match(currentFieldName, parser.getDeprecationHandler())) {
282278
parser.nextToken();
283279
historyUUIDs.put(snapshot, parser.text());

0 commit comments

Comments
 (0)