Merge pull request #26668 from vespa-engine/bratseth/use-visitor

Replace reflection by visitor

config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java

@@ -62,7 +62,7 @@ private static class MyConverter extends ExpressionConverter {
         final Set<String> prevNames = new HashSet<>();
 
         @Override
-        protected ExpressionConverter branch() {
+        public ExpressionConverter branch() {
             MyConverter ret = new MyConverter();
             ret.outputs.addAll(outputs);
             ret.prevNames.addAll(prevNames);

config-model/src/test/derived/indexswitches/ilscripts.cfg

@@ -4,7 +4,7 @@ ilscript[].doctype "indexswitches"
 ilscript[].docfield[] "title"
 ilscript[].docfield[] "descr"
 ilscript[].docfield[] "source_src"
-ilscript[].content[] "clear_state | guard { input source_src | switch { case \"theweb\": input source_src | tokenize normalize | summary source | index source; case \"amg\": input source_src | tokenize normalize | summary source; default: input source_src . \" partner\" | tokenize normalize | summary source | index source; }; }"
+ilscript[].content[] "clear_state | guard { input source_src | switch { case \"amg\": input source_src | tokenize normalize | summary source; case \"theweb\": input source_src | tokenize normalize | summary source | index source; default: input source_src . \" partner\" | tokenize normalize | summary source | index source; }; }"
 ilscript[].content[] "clear_state | guard { input title | tokenize normalize stem:\"BEST\" | summary title | index title; }"
 ilscript[].content[] "clear_state | guard { input descr | tokenize normalize stem:\"BEST\" | summary descr | index descr; }"
 ilscript[].content[] "input source_src | passthrough source_src"

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ExpressionConverter.java

@@ -1,127 +1,22 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.indexinglanguage;
 
-import com.yahoo.collections.Pair;
-import com.yahoo.vespa.indexinglanguage.expressions.*;
-
-import java.lang.reflect.InvocationTargetException;
-import java.util.HashMap;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
+import com.yahoo.vespa.indexinglanguage.expressions.Expression;
 
 /**
  * @author Simon Thoresen Hult
  */
-@SuppressWarnings({ "UnusedDeclaration" })
 public abstract class ExpressionConverter implements Cloneable {
 
-    public final Expression convert(Expression exp) {
-        if (exp == null) {
-            return null;
-        }
-        if (shouldConvert(exp)) {
-            return doConvert(exp);
-        }
-        if (!(exp instanceof CompositeExpression)) {
-            return exp;
-        }
-        try {
-            // The class.getMethod here takes 8% of the cpu time in reading the SSBE application package
-            // TODO: Implement double dispatch through visitor instead?
-            return (Expression)ExpressionConverter.class.getMethod("innerConvert", exp.getClass()).invoke(this, exp);
-        } catch (IllegalAccessException | NoSuchMethodException e) {
-            throw new UnsupportedOperationException(exp.getClass().getName(), e);
-        } catch (InvocationTargetException e) {
-            Throwable t = e.getTargetException();
-            throw t instanceof RuntimeException ? (RuntimeException)t : new RuntimeException(t);
-        }
-    }
-
-    public Expression innerConvert(ArithmeticExpression exp) {
-        return new ArithmeticExpression(convert(exp.getLeftHandSide()),
-                                        exp.getOperator(),
-                                        convert(exp.getRightHandSide()));
-    }
-
-    public Expression innerConvert(CatExpression exp) {
-        List<Expression> lst = new LinkedList<>();
-        for (Expression innerExp : exp) {
-            Expression next = convert(innerExp);
-            if (next != null) {
-                lst.add(next);
-            }
-        }
-        return new CatExpression(lst);
-    }
-
-    public Expression innerConvert(ChoiceExpression exp) {
-        var convertedInnerExpressions = exp.asList().stream().map(inner -> convert(inner)).toList();
-        return new ChoiceExpression(convertedInnerExpressions);
-    }
-
-    public Expression innerConvert(ForEachExpression exp) {
-        return new ForEachExpression(convert(exp.getInnerExpression()));
-    }
-
-    public Expression innerConvert(GuardExpression exp) {
-        return new GuardExpression(convert(exp.getInnerExpression()));
-    }
-
-    public Expression innerConvert(IfThenExpression exp) {
-        return new IfThenExpression(branch().convert(exp.getLeftHandSide()),
-                                    exp.getComparator(),
-                                    branch().convert(exp.getRightHandSide()),
-                                    branch().convert(exp.getIfTrueExpression()),
-                                    branch().convert(exp.getIfFalseExpression()));
-    }
-
-    public Expression innerConvert(ParenthesisExpression exp) {
-        return new ParenthesisExpression(convert(exp.getInnerExpression()));
-    }
-
-    public Expression innerConvert(ScriptExpression exp) {
-        List<StatementExpression> lst = new LinkedList<>();
-        for (Expression innerExp : exp) {
-            StatementExpression next = (StatementExpression)branch().convert(innerExp);
-            if (next != null) {
-                lst.add(next);
-            }
-        }
-        return new ScriptExpression(lst);
-    }
-
-    public Expression innerConvert(SelectInputExpression exp) {
-        List<Pair<String, Expression>> cases = new LinkedList<>();
-        for (Pair<String, Expression> pair : exp.getCases()) {
-            cases.add(new Pair<>(pair.getFirst(), branch().convert(pair.getSecond())));
-        }
-        return new SelectInputExpression(cases);
-    }
-
-    public Expression innerConvert(StatementExpression exp) {
-        List<Expression> lst = new LinkedList<>();
-        for (Expression innerExp : exp) {
-            Expression next = convert(innerExp);
-            if (next != null) {
-                lst.add(next);
-            }
-        }
-        return new StatementExpression(lst);
-    }
-
-    public Expression innerConvert(SwitchExpression exp) {
-        Map<String, Expression> cases = new HashMap<>();
-        for (Map.Entry<String, Expression> entry : exp.getCases().entrySet()) {
-            Expression next = branch().convert(entry.getValue());
-            if (next != null) {
-                cases.put(entry.getKey(), next);
-            }
-        }
-        return new SwitchExpression(cases, branch().convert(exp.getDefaultExpression()));
+    public final Expression convert(Expression expression) {
+        if (expression == null) return null;
+        if (shouldConvert(expression))
+            return doConvert(expression);
+        else
+            return expression.convertChildren(this);
     }
 
-    protected ExpressionConverter branch() {
+    public ExpressionConverter branch() {
         return this;
     }
 

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ValueTransformProvider.java

@@ -20,7 +20,7 @@ public ValueTransformProvider(Class<? extends Expression> transformClass) {
     }
 
     @Override
-    protected final ExpressionConverter branch() {
+    public final ExpressionConverter branch() {
         return clone();
     }
 

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ArithmeticExpression.java

@@ -4,6 +4,7 @@
 import com.yahoo.document.DataType;
 import com.yahoo.document.NumericDataType;
 import com.yahoo.document.datatypes.*;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 import com.yahoo.vespa.objects.ObjectOperation;
 import com.yahoo.vespa.objects.ObjectPredicate;
 
@@ -55,6 +56,12 @@ public ArithmeticExpression(Expression lhs, Operator op, Expression rhs) {
         this.rhs = rhs;
     }
 
+    @Override
+    public ArithmeticExpression convertChildren(ExpressionConverter converter) {
+        // TODO: branch()?
+        return new ArithmeticExpression(converter.convert(lhs), op, converter.convert(rhs));
+    }
+
     public Expression getLeftHandSide() {
         return lhs;
     }

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CatExpression.java

@@ -9,6 +9,7 @@
 import com.yahoo.document.datatypes.FieldValue;
 import com.yahoo.document.datatypes.StringFieldValue;
 import com.yahoo.document.datatypes.WeightedSet;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 
 import java.util.*;
 
@@ -25,6 +26,11 @@ public CatExpression(Collection<? extends Expression> lst) {
         super(lst, resolveInputType(lst));
     }
 
+    @Override
+    public CatExpression convertChildren(ExpressionConverter converter) {
+        return new CatExpression(convertChildList(converter));
+    }
+
     @Override
     protected void doExecute(ExecutionContext context) {
         FieldValue input = context.getValue();

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java

@@ -3,6 +3,7 @@
 
 import com.yahoo.document.DataType;
 import com.yahoo.document.datatypes.FieldValue;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -31,6 +32,11 @@ public ChoiceExpression(Collection<? extends Expression> choices) {
         super(choices, resolveInputType(choices));
     }
 
+    @Override
+    public ChoiceExpression convertChildren(ExpressionConverter converter) {
+        return new ChoiceExpression(asList().stream().map(choice -> converter.branch().convert(choice)).toList());
+    }
+
     @Override
     protected void doExecute(ExecutionContext context) {
         FieldValue input = context.getValue();

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CompositeExpression.java

@@ -2,12 +2,16 @@
 package com.yahoo.vespa.indexinglanguage.expressions;
 
 import com.yahoo.document.DataType;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 
 /**
  * @author Simon Thoresen Hult
  */
 public abstract class CompositeExpression extends Expression {
 
+    @Override
+    public abstract CompositeExpression convertChildren(ExpressionConverter converter);
+
     protected CompositeExpression(DataType inputType) {
         super(inputType);
     }

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java

@@ -35,6 +35,12 @@ protected Expression(DataType inputType) {
         this.inputType = inputType;
     }
 
+    /**
+     * Returns an expression where the children of this has been converted using the given converter.
+     * This default implementation returns this as it has no children.
+     */
+    public Expression convertChildren(ExpressionConverter converter) { return this; }
+
     /** Sets the document type and field the statement this expression is part of will write to */
     public void setStatementOutput(DocumentType documentType, Field field) {}
 

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExpressionList.java

@@ -4,13 +4,15 @@
 import com.yahoo.document.DataType;
 import com.yahoo.document.DocumentType;
 import com.yahoo.document.Field;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 import com.yahoo.vespa.objects.ObjectOperation;
 import com.yahoo.vespa.objects.ObjectPredicate;
 
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Objects;
 
 /**
  * @author Simon Thoresen Hult
@@ -26,6 +28,10 @@ protected ExpressionList(Iterable<? extends T> lst, DataType inputType) {
         }
     }
 
+    protected List<Expression> convertChildList(ExpressionConverter converter) {
+        return asList().stream().map(converter::convert).filter(Objects::nonNull).toList();
+    }
+
     @Override
     public void setStatementOutput(DocumentType documentType, Field field) {
         for (Expression expression : expressions)

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ForEachExpression.java

@@ -6,6 +6,7 @@
 import com.yahoo.document.datatypes.FieldValue;
 import com.yahoo.document.datatypes.Struct;
 import com.yahoo.document.datatypes.WeightedSet;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 import com.yahoo.vespa.indexinglanguage.FieldValueConverter;
 import com.yahoo.vespa.objects.ObjectOperation;
 import com.yahoo.vespa.objects.ObjectPredicate;
@@ -26,6 +27,11 @@ public Expression getInnerExpression() {
         return exp;
     }
 
+    @Override
+    public ForEachExpression convertChildren(ExpressionConverter converter) {
+        return new ForEachExpression(converter.convert(exp));
+    }
+
     @Override
     public void setStatementOutput(DocumentType documentType, Field field) {
         exp.setStatementOutput(documentType, field);

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/GuardExpression.java

@@ -4,6 +4,7 @@
 import com.yahoo.document.DataType;
 import com.yahoo.document.DocumentType;
 import com.yahoo.document.Field;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 import com.yahoo.vespa.indexinglanguage.ExpressionVisitor;
 import com.yahoo.vespa.indexinglanguage.UpdateAdapter;
 import com.yahoo.vespa.objects.ObjectOperation;
@@ -27,6 +28,11 @@ public Expression getInnerExpression() {
         return exp;
     }
 
+    @Override
+    public GuardExpression convertChildren(ExpressionConverter converter) {
+        return new GuardExpression(converter.convert(exp));
+    }
+
     @Override
     public void setStatementOutput(DocumentType documentType, Field field) {
         exp.setStatementOutput(documentType, field);

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/IfThenExpression.java

@@ -6,6 +6,7 @@
 import com.yahoo.document.Field;
 import com.yahoo.document.datatypes.FieldValue;
 import com.yahoo.document.datatypes.NumericFieldValue;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 import com.yahoo.vespa.objects.ObjectOperation;
 import com.yahoo.vespa.objects.ObjectPredicate;
 
@@ -56,6 +57,15 @@ public IfThenExpression(Expression lhs, Comparator cmp, Expression rhs, Expressi
         this.ifFalse = ifFalse;
     }
 
+    @Override
+    public IfThenExpression convertChildren(ExpressionConverter converter) {
+        return new IfThenExpression(converter.branch().convert(lhs),
+                                    cmp,
+                                    converter.branch().convert(rhs),
+                                    converter.branch().convert(ifTrue),
+                                    converter.branch().convert(ifFalse));
+    }
+
     @Override
     public void setStatementOutput(DocumentType documentType, Field field) {
         lhs.setStatementOutput(documentType, field);

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ParenthesisExpression.java

@@ -4,6 +4,7 @@
 import com.yahoo.document.DataType;
 import com.yahoo.document.DocumentType;
 import com.yahoo.document.Field;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 import com.yahoo.vespa.objects.ObjectOperation;
 import com.yahoo.vespa.objects.ObjectPredicate;
 
@@ -23,6 +24,11 @@ public Expression getInnerExpression() {
         return innerExp;
     }
 
+    @Override
+    public ParenthesisExpression convertChildren(ExpressionConverter converter) {
+        return new ParenthesisExpression(converter.convert(innerExp));
+    }
+
     @Override
     public void setStatementOutput(DocumentType documentType, Field field) {
         innerExp.setStatementOutput(documentType, field);

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ScriptExpression.java

@@ -6,6 +6,7 @@
 import com.yahoo.language.Linguistics;
 import com.yahoo.language.process.Embedder;
 import com.yahoo.language.simple.SimpleLinguistics;
+import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
 import com.yahoo.vespa.indexinglanguage.ScriptParser;
 import com.yahoo.vespa.indexinglanguage.ScriptParserContext;
 import com.yahoo.vespa.indexinglanguage.parser.IndexingInput;
@@ -17,6 +18,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 /**
  * @author Simon Thoresen Hult
@@ -35,6 +37,14 @@ public ScriptExpression(Collection<? extends StatementExpression> lst) {
         super(lst, resolveInputType(lst));
     }
 
+    @Override
+    public ScriptExpression convertChildren(ExpressionConverter converter) {
+        return new ScriptExpression(asList().stream()
+                                            .map(child -> (StatementExpression)converter.branch().convert(child))
+                                            .filter(Objects::nonNull)
+                                            .toList());
+    }
+
     @Override
     protected void doExecute(ExecutionContext context) {
         FieldValue input = context.getValue();