Speed up DocumentParser.innerParseObject (elastic#88715)

original-brownbear · web-flow · commit b7e2d595c5dd · 2022-07-22T13:08:23.000+02:00
Making this a little smaller so it inlines more often and more importantly
avoid getting the current field name redundantly before entering the loop which
is not free when the parser is a `DotExpandingXContentParser`.
This change gives a significant boost to `BeatsMapperBenchmark.benchmarkParseKeywordFields`.

Results below are pretty stable, running with a large number of iterations.

before:

```
Result "org.elasticsearch.benchmark.index.mapper.BeatsMapperBenchmark.benchmarkParseKeywordFields":
  9039.270 ±(99.9%) 28.001 ns/op [Average]
  (min, avg, max) = (9008.778, 9039.270, 9062.246), stdev = 18.521
  CI (99.9%): [9011.269, 9067.271] (assumes normal distribution)
```

after:

```
Result "org.elasticsearch.benchmark.index.mapper.BeatsMapperBenchmark.benchmarkParseKeywordFields":
  8645.649 ±(99.9%) 53.677 ns/op [Average]
  (min, avg, max) = (8568.319, 8645.649, 8688.210), stdev = 35.504
  CI (99.9%): [8591.972, 8699.327] (assumes normal distribution)
```
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
@@ -294,17 +294,18 @@ private static void throwOnConcreteValue(ObjectMapper mapper, String currentFiel
 
     private static void innerParseObject(DocumentParserContext context, ObjectMapper mapper) throws IOException {
 
-        XContentParser.Token token = context.parser().currentToken();
-        String currentFieldName = context.parser().currentName();
+        final XContentParser parser = context.parser();
+        XContentParser.Token token = parser.currentToken();
+        String currentFieldName = null;
         assert token == XContentParser.Token.FIELD_NAME || token == XContentParser.Token.END_OBJECT;
 
         while (token != XContentParser.Token.END_OBJECT) {
             if (token == null) {
-                throwEOF(mapper, currentFieldName);
+                throwEOF(mapper, context);
             }
             switch (token) {
                 case FIELD_NAME:
-                    currentFieldName = context.parser().currentName();
+                    currentFieldName = parser.currentName();
                     if (currentFieldName.isBlank()) {
                         throwFieldNameBlank(context, currentFieldName);
                     }
@@ -324,7 +325,7 @@ private static void innerParseObject(DocumentParserContext context, ObjectMapper
                     }
                     break;
             }
-            token = context.parser().nextToken();
+            token = parser.nextToken();
         }
     }
 
@@ -334,12 +335,12 @@ private static void throwFieldNameBlank(DocumentParserContext context, String cu
         );
     }
 
-    private static void throwEOF(ObjectMapper mapper, String currentFieldName) {
+    private static void throwEOF(ObjectMapper mapper, DocumentParserContext context) throws IOException {
         throw new MapperParsingException(
             "object mapping for ["
                 + mapper.name()
                 + "] tried to parse field ["
-                + currentFieldName
+                + context.parser().currentName()
                 + "] as object, but got EOF, has a concrete value been provided to it?"
         );
     }

Original file line number	Diff line number	Diff line change
`@@ -294,17 +294,18 @@ private static void throwOnConcreteValue(ObjectMapper mapper, String currentFiel`
`294`	`294`
`295`	`295`	`private static void innerParseObject(DocumentParserContext context, ObjectMapper mapper) throws IOException {`
`296`	`296`
`297`		`- XContentParser.Token token = context.parser().currentToken();`
`298`		`- String currentFieldName = context.parser().currentName();`
	`297`	`+ final XContentParser parser = context.parser();`
	`298`	`+ XContentParser.Token token = parser.currentToken();`
	`299`	`+ String currentFieldName = null;`
`299`	`300`	`assert token == XContentParser.Token.FIELD_NAME \|\| token == XContentParser.Token.END_OBJECT;`
`300`	`301`
`301`	`302`	`while (token != XContentParser.Token.END_OBJECT) {`
`302`	`303`	`if (token == null) {`
`303`		`- throwEOF(mapper, currentFieldName);`
	`304`	`+ throwEOF(mapper, context);`
`304`	`305`	`}`
`305`	`306`	`switch (token) {`
`306`	`307`	`case FIELD_NAME:`
`307`		`- currentFieldName = context.parser().currentName();`
	`308`	`+ currentFieldName = parser.currentName();`
`308`	`309`	`if (currentFieldName.isBlank()) {`
`309`	`310`	`throwFieldNameBlank(context, currentFieldName);`
`310`	`311`	`}`
`@@ -324,7 +325,7 @@ private static void innerParseObject(DocumentParserContext context, ObjectMapper`
`324`	`325`	`}`
`325`	`326`	`break;`
`326`	`327`	`}`
`327`		`- token = context.parser().nextToken();`
	`328`	`+ token = parser.nextToken();`
`328`	`329`	`}`
`329`	`330`	`}`
`330`	`331`
`@@ -334,12 +335,12 @@ private static void throwFieldNameBlank(DocumentParserContext context, String cu`
`334`	`335`	`);`
`335`	`336`	`}`
`336`	`337`
`337`		`- private static void throwEOF(ObjectMapper mapper, String currentFieldName) {`
	`338`	`+ private static void throwEOF(ObjectMapper mapper, DocumentParserContext context) throws IOException {`
`338`	`339`	`throw new MapperParsingException(`
`339`	`340`	`"object mapping for ["`
`340`	`341`	`+ mapper.name()`
`341`	`342`	`+ "] tried to parse field ["`
`342`		`- + currentFieldName`
	`343`	`+ + context.parser().currentName()`
`343`	`344`	`+ "] as object, but got EOF, has a concrete value been provided to it?"`
`344`	`345`	`);`
`345`	`346`	`}`