ESQL: Speed up TO_IP (#126338)

benchmarks/README.md

@@ -82,19 +82,21 @@ To get realistic results, you should exercise care when running benchmarks. Here
 NOTE: Linux only. Sorry Mac and Windows.
 
 Disassembling is fun! Maybe not always useful, but always fun! Generally, you'll want to install `perf` and the JDK's `hsdis`.
-`perf` is generally available via `apg-get install perf` or `pacman -S perf`. `hsdis` you'll want to compile from source. is a little more involved. This worked
+`perf` is generally available via `apg-get install perf` or `pacman -S perf linux-tools`. `hsdis` you'll want to compile from source. is a little more involved. This worked
 on 2020-08-01:
 
 ```
 git clone [email protected]:openjdk/jdk.git
 cd jdk
-git checkout jdk-17-ga
-cd src/utils/hsdis
+git checkout jdk-24-ga
 # Get a known good binutils
 wget https://ftp.gnu.org/gnu/binutils/binutils-2.35.tar.gz
 tar xf binutils-2.35.tar.gz
-make BINUTILS=binutils-2.35 ARCH=amd64
-sudo cp build/linux-amd64/hsdis-amd64.so /usr/lib/jvm/java-17-openjdk/lib/server/
+bash configure --with-hsdis=binutils --with-binutils-src=binutils-2.35 \
+    --with-boot-jdk=~/.gradle/jdks/oracle_corporation-24-amd64-linux.2
+make build-hsdis
+cp ./build/linux-x86_64-server-release/jdk/lib/hsdis-amd64.so \
+    ~/.gradle/jdks/oracle_corporation-24-amd64-linux.2/lib/hsdis.so
 ```
 
 If you want to disassemble a single method do something like this:
@@ -105,6 +107,30 @@ gradlew -p benchmarks run --args ' MemoryStatsBenchmark -jvmArgs "-XX:+UnlockDia
 
 If you want `perf` to find the hot methods for you, then do add `-prof perfasm`.
 
+NOTE: `perfasm` will need more access:
+```
+sudo bash
+echo -1 > /proc/sys/kernel/perf_event_paranoid
+exit
+```
+
+If you get warnings like:
+```
+The perf event count is suspiciously low (0).
+```
+then check if you are bumping into [this](https://man.archlinux.org/man/perf-stat.1.en#INTEL_HYBRID_SUPPORT)
+by running:
+```
+perf stat -B dd if=/dev/zero of=/dev/null count=1000000
+```
+
+If you see lines like:
+```
+         765019980      cpu_atom/cycles/                 #    1.728 GHz                         (0.60%)
+        2258845959      cpu_core/cycles/                 #    5.103 GHz                         (99.18%)
+```
+then `perf` is just not going to work for you.
+
 ## Async Profiler
 
 Note: Linux and Mac only. Sorry Windows.

benchmarks/src/main/java/org/elasticsearch/benchmark/compute/operator/ParseIpBenchmark.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.benchmark.compute.operator;
+
+import org.apache.lucene.document.InetAddressPoint;
+import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.breaker.NoopCircuitBreaker;
+import org.elasticsearch.common.network.InetAddresses;
+import org.elasticsearch.compute.operator.BreakingBytesRefBuilder;
+import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ParseIp;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+
+import java.net.InetAddress;
+import java.util.concurrent.TimeUnit;
+
+@Warmup(iterations = 5)
+@Measurement(iterations = 7)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Thread)
+@Fork(1)
+public class ParseIpBenchmark {
+    private final BytesRef ip = new BytesRef("192.168.0.1");
+    private final BreakingBytesRefBuilder scratch = ParseIp.buildScratch(new NoopCircuitBreaker("request"));
+
+    @Benchmark
+    public BytesRef leadingZerosRejected() {
+        return ParseIp.leadingZerosRejected(ip, scratch);
+    }
+
+    @Benchmark
+    public BytesRef leadingZerosAreDecimal() {
+        return ParseIp.leadingZerosAreDecimal(ip, scratch);
+    }
+
+    @Benchmark
+    public BytesRef leadingZerosAreOctal() {
+        return ParseIp.leadingZerosAreOctal(ip, scratch);
+    }
+
+    @Benchmark
+    public BytesRef original() {
+        InetAddress inetAddress = InetAddresses.forString(ip.utf8ToString());
+        return new BytesRef(InetAddressPoint.encode(inetAddress));
+    }
+}

docs/changelog/126338.yaml

@@ -0,0 +1,5 @@
+pr: 126338
+summary: Speed up TO_IP
+area: ES|QL
+type: enhancement
+issues: []

x-pack/plugin/esql/compute/gen/src/main/java/org/elasticsearch/compute/gen/ConsumeProcessor.java

@@ -42,6 +42,7 @@ public Set<String> getSupportedAnnotationTypes() {
             "org.elasticsearch.xpack.esql.expression.function.MapParam",
             "org.elasticsearch.rest.ServerlessScope",
             "org.elasticsearch.xcontent.ParserConstructor",
+            "org.elasticsearch.core.UpdateForV9",
             Fixed.class.getName()
         );
     }

x-pack/plugin/esql/compute/gen/src/main/java/org/elasticsearch/compute/gen/ConvertEvaluatorImplementer.java

@@ -23,7 +23,6 @@
 import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.Elements;
 
-import static org.elasticsearch.compute.gen.Methods.appendMethod;
 import static org.elasticsearch.compute.gen.Methods.buildFromFactory;
 import static org.elasticsearch.compute.gen.Methods.getMethod;
 import static org.elasticsearch.compute.gen.Types.ABSTRACT_CONVERT_FUNCTION_EVALUATOR;
@@ -41,27 +40,34 @@
 public class ConvertEvaluatorImplementer {
 
     private final TypeElement declarationType;
-    private final ExecutableElement processFunction;
+    private final EvaluatorImplementer.ProcessFunction processFunction;
     private final String extraName;
     private final ClassName implementation;
     private final TypeName argumentType;
-    private final TypeName resultType;
     private final List<TypeMirror> warnExceptions;
 
     public ConvertEvaluatorImplementer(
         Elements elements,
+        javax.lang.model.util.Types types,
         ExecutableElement processFunction,
         String extraName,
         List<TypeMirror> warnExceptions
     ) {
         this.declarationType = (TypeElement) processFunction.getEnclosingElement();
-        this.processFunction = processFunction;
-        if (processFunction.getParameters().size() != 1) {
-            throw new IllegalArgumentException("processing function should have exactly one parameter");
+        this.processFunction = new EvaluatorImplementer.ProcessFunction(types, processFunction, warnExceptions);
+
+        if (this.processFunction.args.get(0) instanceof EvaluatorImplementer.StandardProcessFunctionArg == false) {
+            throw new IllegalArgumentException("first argument must be the field to process");
+        }
+        for (int a = 1; a < this.processFunction.args.size(); a++) {
+            if (this.processFunction.args.get(a) instanceof EvaluatorImplementer.FixedProcessFunctionArg == false) {
+                throw new IllegalArgumentException("fixed function args supported after the first");
+                // TODO support more function types when we need them
+            }
         }
+
         this.extraName = extraName;
         this.argumentType = TypeName.get(processFunction.getParameters().get(0).asType());
-        this.resultType = TypeName.get(processFunction.getReturnType());
         this.warnExceptions = warnExceptions;
 
         this.implementation = ClassName.get(
@@ -87,29 +93,36 @@ private TypeSpec type() {
         builder.addModifiers(Modifier.PUBLIC, Modifier.FINAL);
         builder.superclass(ABSTRACT_CONVERT_FUNCTION_EVALUATOR);
 
+        for (EvaluatorImplementer.ProcessFunctionArg a : processFunction.args) {
+            a.declareField(builder);
+        }
         builder.addMethod(ctor());
-        builder.addMethod(name());
+        builder.addMethod(next());
         builder.addMethod(evalVector());
         builder.addMethod(evalValue(true));
         builder.addMethod(evalBlock());
         builder.addMethod(evalValue(false));
+        builder.addMethod(processFunction.toStringMethod(implementation));
+        builder.addMethod(processFunction.close());
         builder.addType(factory());
         return builder.build();
     }
 
     private MethodSpec ctor() {
         MethodSpec.Builder builder = MethodSpec.constructorBuilder().addModifiers(Modifier.PUBLIC);
-        builder.addParameter(EXPRESSION_EVALUATOR, "field");
         builder.addParameter(SOURCE, "source");
+        builder.addStatement("super(driverContext, source)");
+        for (EvaluatorImplementer.ProcessFunctionArg a : processFunction.args) {
+            a.implementCtor(builder);
+        }
         builder.addParameter(DRIVER_CONTEXT, "driverContext");
-        builder.addStatement("super(driverContext, field, source)");
         return builder.build();
     }
 
-    private MethodSpec name() {
-        MethodSpec.Builder builder = MethodSpec.methodBuilder("name").addModifiers(Modifier.PUBLIC);
-        builder.addAnnotation(Override.class).returns(String.class);
-        builder.addStatement("return $S", declarationType.getSimpleName() + extraName);
+    private MethodSpec next() {
+        MethodSpec.Builder builder = MethodSpec.methodBuilder("next").addAnnotation(Override.class).addModifiers(Modifier.PUBLIC);
+        builder.returns(EXPRESSION_EVALUATOR);
+        builder.addStatement("return $N", ((EvaluatorImplementer.StandardProcessFunctionArg) processFunction.args.get(0)).name());
         return builder.build();
     }
 
@@ -129,7 +142,7 @@ private MethodSpec evalVector() {
         builder.beginControlFlow("if (vector.isConstant())");
         {
             catchingWarnExceptions(builder, () -> {
-                var constVectType = blockType(resultType);
+                var constVectType = processFunction.resultDataType(true);
                 builder.addStatement(
                     "return driverContext.blockFactory().newConstant$TWith($N, positionCount)",
                     constVectType,
@@ -139,7 +152,7 @@ private MethodSpec evalVector() {
         }
         builder.endControlFlow();
 
-        ClassName resultBuilderType = builderType(blockType(resultType));
+        ClassName resultBuilderType = builderType(processFunction.resultDataType(true));
         builder.beginControlFlow(
             "try ($T builder = driverContext.blockFactory().$L(positionCount))",
             resultBuilderType,
@@ -150,7 +163,11 @@ private MethodSpec evalVector() {
             {
                 catchingWarnExceptions(
                     builder,
-                    () -> builder.addStatement("builder.$L($N)", appendMethod(resultType), evalValueCall("vector", "p", scratchPadName)),
+                    () -> builder.addStatement(
+                        "builder.$L($N)",
+                        processFunction.appendMethod(),
+                        evalValueCall("vector", "p", scratchPadName)
+                    ),
                     () -> builder.addStatement("builder.appendNull()")
                 );
             }
@@ -185,7 +202,7 @@ private MethodSpec evalBlock() {
         TypeName blockType = blockType(argumentType);
         builder.addStatement("$T block = ($T) b", blockType, blockType);
         builder.addStatement("int positionCount = block.getPositionCount()");
-        TypeName resultBuilderType = builderType(blockType(resultType));
+        TypeName resultBuilderType = builderType(processFunction.resultDataType(true));
         builder.beginControlFlow(
             "try ($T builder = driverContext.blockFactory().$L(positionCount))",
             resultBuilderType,
@@ -196,19 +213,18 @@ private MethodSpec evalBlock() {
             builder.addStatement("BytesRef $N = new BytesRef()", scratchPadName);
         }
 
-        String appendMethod = appendMethod(resultType);
+        String appendMethod = processFunction.appendMethod();
         builder.beginControlFlow("for (int p = 0; p < positionCount; p++)");
         {
             builder.addStatement("int valueCount = block.getValueCount(p)");
             builder.addStatement("int start = block.getFirstValueIndex(p)");
             builder.addStatement("int end = start + valueCount");
             builder.addStatement("boolean positionOpened = false");
             builder.addStatement("boolean valuesAppended = false");
-            // builder.addStatement("builder.beginPositionEntry()");
             builder.beginControlFlow("for (int i = start; i < end; i++)");
             {
                 catchingWarnExceptions(builder, () -> {
-                    builder.addStatement("$T value = $N", resultType, evalValueCall("block", "i", scratchPadName));
+                    builder.addStatement("$T value = $N", processFunction.returnType(), evalValueCall("block", "i", scratchPadName));
                     builder.beginControlFlow("if (positionOpened == false && valueCount > 1)");
                     {
                         builder.addStatement("builder.beginPositionEntry()");
@@ -253,8 +269,8 @@ private String evalValueCall(String container, String index, String scratchPad)
 
     private MethodSpec evalValue(boolean forVector) {
         MethodSpec.Builder builder = MethodSpec.methodBuilder("evalValue")
-            .addModifiers(Modifier.PRIVATE, Modifier.STATIC)
-            .returns(resultType);
+            .addModifiers(Modifier.PRIVATE)
+            .returns(processFunction.returnType());
 
         if (forVector) {
             builder.addParameter(vectorType(argumentType), "container");
@@ -269,8 +285,17 @@ private MethodSpec evalValue(boolean forVector) {
             builder.addStatement("$T value = container.$N(index)", argumentType, getMethod(argumentType));
         }
 
-        builder.addStatement("return $T.$N(value)", declarationType, processFunction.getSimpleName());
-
+        StringBuilder pattern = new StringBuilder();
+        List<Object> args = new ArrayList<>();
+        pattern.append("return $T.$N(value");
+        args.add(declarationType);
+        args.add(processFunction.function.getSimpleName());
+        for (int a = 1; a < processFunction.args.size(); a++) {
+            pattern.append(", ");
+            processFunction.args.get(a).buildInvocation(pattern, args, false /* block style parameter should be unused */);
+        }
+        pattern.append(")");
+        builder.addStatement(pattern.toString(), args.toArray());
         return builder.build();
     }
 
@@ -280,42 +305,11 @@ private TypeSpec factory() {
         builder.addModifiers(Modifier.PUBLIC, Modifier.STATIC);
 
         builder.addField(SOURCE, "source", Modifier.PRIVATE, Modifier.FINAL);
-        builder.addField(EXPRESSION_EVALUATOR_FACTORY, "field", Modifier.PRIVATE, Modifier.FINAL);
-
-        builder.addMethod(factoryCtor());
-        builder.addMethod(factoryGet());
-        builder.addMethod(factoryToString());
-        return builder.build();
-    }
-
-    private MethodSpec factoryCtor() {
-        MethodSpec.Builder builder = MethodSpec.constructorBuilder().addModifiers(Modifier.PUBLIC);
-        builder.addParameter(EXPRESSION_EVALUATOR_FACTORY, "field");
-        builder.addParameter(SOURCE, "source");
-        builder.addStatement("this.field = field");
-        builder.addStatement("this.source = source");
-        return builder.build();
-    }
-
-    private MethodSpec factoryGet() {
-        MethodSpec.Builder builder = MethodSpec.methodBuilder("get").addAnnotation(Override.class);
-        builder.addModifiers(Modifier.PUBLIC);
-        builder.addParameter(DRIVER_CONTEXT, "context");
-        builder.returns(implementation);
-
-        List<String> args = new ArrayList<>();
-        args.add("field.get(context)");
-        args.add("source");
-        args.add("context");
-        builder.addStatement("return new $T($L)", implementation, args.stream().collect(Collectors.joining(", ")));
-        return builder.build();
-    }
+        processFunction.args.forEach(a -> a.declareFactoryField(builder));
 
-    private MethodSpec factoryToString() {
-        MethodSpec.Builder builder = MethodSpec.methodBuilder("toString").addAnnotation(Override.class);
-        builder.addModifiers(Modifier.PUBLIC);
-        builder.returns(String.class);
-        builder.addStatement("return $S + field + $S", declarationType.getSimpleName() + extraName + "Evaluator[field=", "]");
+        builder.addMethod(processFunction.factoryCtor());
+        builder.addMethod(processFunction.factoryGet(implementation));
+        builder.addMethod(processFunction.toStringMethod(implementation));
         return builder.build();
     }
 }