[KIE-1085] avoid wasteful creation of a custom operator per constrain…

…t at runtime when using the executable model (apache#5844)
kiegroup · Apr 16, 2024 · 01a3d93 · 01a3d93
1 parent f02a64d
commit 01a3d93
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 10 deletions.
diff --git a/...l/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/PackageModel.java b/...l/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/PackageModel.java
@@ -87,6 +87,7 @@
 import org.drools.model.codegen.execmodel.generator.QueryParameter;
 import org.drools.model.codegen.execmodel.generator.TypedExpression;
 import org.drools.model.codegen.execmodel.generator.WindowReferenceGenerator;
+import org.drools.model.codegen.execmodel.generator.operatorspec.CustomOperatorSpec;
 import org.drools.model.codegen.execmodel.util.lambdareplace.CreatedClass;
 import org.drools.model.functions.PredicateInformation;
 import org.drools.modelcompiler.util.StringUtil;
@@ -187,6 +188,8 @@ public class PackageModel {
 
     private final boolean prototypesAllowed;
 
+    private final CustomOperatorSpec customOperatorSpec = new CustomOperatorSpec();
+
     private PackageModel( ReleaseId releaseId, String name, KnowledgeBuilderConfigurationImpl configuration, DialectCompiletimeRegistry dialectCompiletimeRegistry, DRLIdGenerator exprIdGenerator) {
         this(name, configuration, dialectCompiletimeRegistry, exprIdGenerator, getPkgUUID(configuration, releaseId, name));
     }
@@ -293,6 +296,10 @@ public DRLIdGenerator getExprIdGenerator() {
         return exprIdGenerator;
     }
 
+    public CustomOperatorSpec getCustomOperatorSpec() {
+        return customOperatorSpec;
+    }
+
     public void addImports(Collection<String> imports) {
         this.imports.addAll(imports);
     }
@@ -612,6 +619,8 @@ public RuleSourceResult getRulesSource() {
 
         rulesClass.addMember( generateListField("org.drools.model.Global", "globals", globals.isEmpty() && !hasRuleUnit) );
 
+        customOperatorSpec.getOperatorDeclarations().forEach( op -> rulesClass.addMember( parseBodyDeclaration( op ) ) );
+
         if ( !typeMetaDataExpressions.isEmpty() ) {
             BodyDeclaration<?> typeMetaDatasList = parseBodyDeclaration("java.util.List<org.drools.model.TypeMetaData> typeMetaDatas = new java.util.ArrayList<>();");
             rulesClass.addMember(typeMetaDatasList);

diff --git a/...in/java/org/drools/model/codegen/execmodel/generator/expressiontyper/ExpressionTyper.java b/...in/java/org/drools/model/codegen/execmodel/generator/expressiontyper/ExpressionTyper.java
@@ -66,14 +66,13 @@
 import com.github.javaparser.ast.type.Type;
 import org.drools.model.codegen.execmodel.errors.InvalidExpressionErrorResult;
 import org.drools.model.codegen.execmodel.errors.ParseExpressionErrorResult;
-import org.drools.model.codegen.execmodel.generator.TypedDeclarationSpec;
 import org.drools.model.codegen.execmodel.generator.DrlxParseUtil;
 import org.drools.model.codegen.execmodel.generator.ModelGenerator;
 import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedDeclarationSpec;
 import org.drools.model.codegen.execmodel.generator.TypedExpression;
 import org.drools.model.codegen.execmodel.generator.UnificationTypedExpression;
 import org.drools.model.codegen.execmodel.generator.drlxparse.NumberAndStringArithmeticOperationCoercion;
-import org.drools.model.codegen.execmodel.generator.operatorspec.CustomOperatorSpec;
 import org.drools.model.codegen.execmodel.generator.operatorspec.NativeOperatorSpec;
 import org.drools.model.codegen.execmodel.generator.operatorspec.OperatorSpec;
 import org.drools.model.codegen.execmodel.generator.operatorspec.TemporalOperatorSpec;
@@ -126,8 +125,8 @@
 import static org.drools.mvel.parser.MvelParser.parseType;
 import static org.drools.mvel.parser.printer.PrintUtil.printNode;
 import static org.drools.util.ClassUtils.extractGenericType;
-import static org.drools.util.ClassUtils.getter2property;
 import static org.drools.util.ClassUtils.getTypeArgument;
+import static org.drools.util.ClassUtils.getter2property;
 import static org.drools.util.ClassUtils.toRawClass;
 import static org.kie.internal.ruleunit.RuleUnitUtil.isDataSource;
 
@@ -482,7 +481,7 @@ private OperatorSpec getOperatorSpec(NodeList<Expression> rightExpressions, Simp
         if ( org.drools.model.functions.Operator.Register.hasOperator( operator ) ) {
             return NativeOperatorSpec.INSTANCE;
         }
-        return CustomOperatorSpec.INSTANCE;
+        return ruleContext.getPackageModel().getCustomOperatorSpec();
     }
 
     private TypedExpressionResult toTypedExpressionFromMethodCallOrField(Expression drlxExpr) {

diff --git a/...in/java/org/drools/model/codegen/execmodel/generator/operatorspec/CustomOperatorSpec.java b/...in/java/org/drools/model/codegen/execmodel/generator/operatorspec/CustomOperatorSpec.java
@@ -18,14 +18,22 @@
  */
 package org.drools.model.codegen.execmodel.generator.operatorspec;
 
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import com.github.javaparser.ast.expr.MethodCallExpr;
 import org.drools.base.base.ValueType;
 import org.drools.compiler.rule.builder.EvaluatorDefinition;
-import com.github.javaparser.ast.expr.MethodCallExpr;
-import org.drools.model.functions.Operator;
 import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.functions.Operator;
 
 public class CustomOperatorSpec extends NativeOperatorSpec {
-    public static final CustomOperatorSpec INSTANCE = new CustomOperatorSpec();
+
+    private Set<String> generatedOperators = new HashSet<>();
+
+    private List<String> operatorDeclarations = new ArrayList<>();
 
     @Override
     protected Operator addOperatorArgument( RuleContext context, MethodCallExpr methodCallExpr, String opName ) {
@@ -34,10 +42,20 @@ protected Operator addOperatorArgument( RuleContext context, MethodCallExpr meth
             throw new RuntimeException( "Unknown custom operator: " + opName );
         }
 
-        String arg = "new " + CustomOperatorWrapper.class.getCanonicalName() + "( new " + evalDef.getClass().getCanonicalName() + "().getEvaluator(" +
-                ValueType.class.getCanonicalName() + ".OBJECT_TYPE, \"" + opName + "\", false, null), \"" + opName + "\")";
+        String operatorInstance = "OPERATOR_" + opName + "_INSTANCE";
+
+        if (generatedOperators.add(opName)) {
+            String operatorFieldDeclaration = "public static final " + Operator.class.getCanonicalName() + ".SingleValue<Object, Object> " + operatorInstance +
+                    " = new " + CustomOperatorWrapper.class.getCanonicalName() + "( new " + evalDef.getClass().getCanonicalName() + "().getEvaluator(" +
+                    ValueType.class.getCanonicalName() + ".OBJECT_TYPE, \"" + opName + "\", false, null), \"" + opName + "\");";
+            operatorDeclarations.add(operatorFieldDeclaration);
+        }
 
-        methodCallExpr.addArgument( arg );
+        methodCallExpr.addArgument( context.getPackageModel().getRulesFileNameWithPackage() + "." + operatorInstance );
         return null;
     }
+
+    public List<String> getOperatorDeclarations() {
+        return operatorDeclarations;
+    }
 }
diff --git a/...er-integration/src/test/java/org/drools/compiler/integrationtests/CustomOperatorTest.java b/...er-integration/src/test/java/org/drools/compiler/integrationtests/CustomOperatorTest.java
@@ -69,6 +69,18 @@ public void testCustomOperatorUsingCollections() {
         customOperatorUsingCollections(constraints);
     }
 
+    @Test
+    public void testNoOperatorInstancesCreatedAtRuntime() {
+        String constraints =
+                "    $alice : Person(name == \"Alice\")\n" +
+                "    $bob : Person(name == \"Bob\", addresses supersetOf $alice.addresses)\n" +
+                "    Person(name == \"Bob\", addresses supersetOf $alice.addresses)\n";
+
+        customOperatorUsingCollections(constraints);
+
+        assertThat(SupersetOfEvaluatorDefinition.INSTANCES_COUNTER).isEqualTo(0);
+    }
+
     @Test
     public void testCustomOperatorUsingCollectionsInverted() {
         // DROOLS-6983
@@ -90,6 +102,9 @@ private void customOperatorUsingCollections(String constraints) {
         System.setProperty(EvaluatorOption.PROPERTY_NAME + "supersetOf", SupersetOfEvaluatorDefinition.class.getName());
         try {
             final KieBase kbase = KieBaseUtil.getKieBaseFromKieModuleFromDrl("custom-operator-test", kieBaseTestConfiguration, drl);
+
+            SupersetOfEvaluatorDefinition.INSTANCES_COUNTER = 0;
+
             final KieSession ksession = kbase.newKieSession();
             try {
                 final Person alice = new Person("Alice", 30);
@@ -118,6 +133,12 @@ public static class SupersetOfEvaluatorDefinition implements EvaluatorDefinition
 
         private Evaluator[] evaluator;
 
+        static int INSTANCES_COUNTER = 0;
+
+        public SupersetOfEvaluatorDefinition() {
+            INSTANCES_COUNTER++;
+        }
+
         public String[] getEvaluatorIds() {
             return SupersetOfEvaluatorDefinition.SUPPORTED_IDS;
         }