Skip to content

Commit

Permalink
[DROOLS-6642] fix performance regression in executable model generati…
Browse files Browse the repository at this point in the history 
…on (apache#3880) (apache#3882)

* [DROOLS-6642] fix performance regression in executable model generation

* wip

(cherry picked from commit 13214ae)
  • Loading branch information
@mariofusco
mariofusco committed Oct 7, 2021
1 parent 3f43fd6 commit 524bcc2
Show file tree
Hide file tree
Showing 22 changed files with 170 additions and 130 deletions.
87 changes: 67 additions & 20 deletions ...drools-canonical-model/src/main/java/org/drools/model/functions/PredicateInformation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;

/**
* Used to generate a better error message when constraints fail
Expand All @@ -33,35 +33,37 @@ public class PredicateInformation {

// Used to generate a significant error message
private final String stringConstraint;
private final Map<String, Set<String>> ruleNameMap = new HashMap<>();
private final Set<RuleDef> ruleDefs = new TreeSet<>();

public PredicateInformation(String stringConstraint) {
public PredicateInformation(String stringConstraint, String... ruleNames) {
this.stringConstraint = defaultToEmptyString(stringConstraint);
}

public PredicateInformation(String stringConstraint, String ruleName, String ruleFileName) {
this.stringConstraint = defaultToEmptyString(stringConstraint);
ruleName = defaultToEmptyString(ruleName);
ruleFileName = defaultToEmptyString(ruleFileName);
ruleNameMap.computeIfAbsent(ruleFileName, k -> new HashSet<>()).add(ruleName);
addRuleNames(ruleNames);
}

public String getStringConstraint() {
return stringConstraint;
}

public Map<String, Set<String>> getRuleNameMap() {
return ruleNameMap;
public Set<RuleDef> getRuleDefs() {
return ruleDefs;
}

public void addRuleName(String ruleName, String ruleFileName) {
ruleName = defaultToEmptyString(ruleName);
ruleFileName = defaultToEmptyString(ruleFileName);
ruleNameMap.computeIfAbsent(ruleFileName, k -> new HashSet<>()).add(ruleName);
public void addRuleNames(String... ruleNames) {
for (int i = 0; i < ruleNames.length; i+=2) {
ruleDefs.add(new RuleDef(defaultToEmptyString(ruleNames[i+1]), defaultToEmptyString(ruleNames[i])));
}
}

public Map<String, Set<String>> getRuleNameMap() {
Map<String, Set<String>> map = new HashMap<>();
for (RuleDef ruleDef : ruleDefs) {
map.computeIfAbsent(ruleDef.fileName, k -> new HashSet<>()).add(ruleDef.ruleName);
}
return map;
}

public static String defaultToEmptyString(String str) {
return Optional.ofNullable(str).orElse("");
return str == null ? "" : str;
}

public boolean isEmpty() {
Expand All @@ -78,19 +80,64 @@ public boolean equals(Object o) {
}
PredicateInformation that = (PredicateInformation) o;
return Objects.equals(stringConstraint, that.stringConstraint) &&
Objects.equals(ruleNameMap, that.ruleNameMap);
Objects.equals(ruleDefs, that.ruleDefs);
}

@Override
public int hashCode() {
return Objects.hash(stringConstraint, ruleNameMap);
return Objects.hash(stringConstraint, ruleDefs);
}

@Override
public String toString() {
return "PredicateInformation{" +
"stringConstraint='" + stringConstraint + '\'' +
", ruleNameMap='" + ruleNameMap + '\'' +
", ruleNameMap='" + ruleDefs + '\'' +
'}';
}

public static class RuleDef implements Comparable<RuleDef> {
private final String fileName;
private final String ruleName;

public RuleDef(String fileName, String ruleName) {
this.fileName = fileName;
this.ruleName = ruleName;
}

public String getFileName() {
return fileName;
}

public String getRuleName() {
return ruleName;
}

@Override
public int compareTo(RuleDef other) {
int fileNameCompare = this.fileName.compareTo(other.fileName);
return fileNameCompare != 0 ? fileNameCompare : this.ruleName.compareTo(other.ruleName);
}

@Override
public String toString() {
return "RuleDef{" +
"fileName='" + fileName + '\'' +
", ruleName='" + ruleName + '\'' +
'}';
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
RuleDef ruleDef = (RuleDef) o;
return Objects.equals(fileName, ruleDef.fileName) && Objects.equals(ruleName, ruleDef.ruleName);
}

@Override
public int hashCode() {
return Objects.hash(fileName, ruleName);
}
}
}
10 changes: 5 additions & 5 deletions ...rools-model-compiler/src/main/java/org/drools/modelcompiler/builder/ModelSourceClass.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import com.github.javaparser.ast.expr.IntegerLiteralExpr;
import com.github.javaparser.ast.expr.MethodCallExpr;
import com.github.javaparser.ast.expr.NameExpr;
import com.github.javaparser.ast.expr.StringLiteralExpr;
import com.github.javaparser.ast.expr.VariableDeclarationExpr;
import com.github.javaparser.ast.stmt.BlockStmt;
import com.github.javaparser.ast.type.ClassOrInterfaceType;
Expand All @@ -34,6 +33,7 @@
import static com.github.javaparser.ast.Modifier.publicModifier;
import static com.github.javaparser.ast.NodeList.nodeList;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.createSimpleAnnotation;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.toStringLiteral;
import static org.drools.modelcompiler.util.StringUtil.toId;

public class ModelSourceClass {
Expand Down Expand Up @@ -257,13 +257,13 @@ private void eventProcessingType() {

private void kieBaseModelPackages() {
for (String p : kieBaseModel.getPackages()) {
stmt.addStatement(new MethodCallExpr(kieBaseModelNameExpr, "addPackage", nodeList(new StringLiteralExpr(p))));
stmt.addStatement(new MethodCallExpr(kieBaseModelNameExpr, "addPackage", nodeList(toStringLiteral(p))));
}
}

private void kieBaseModelIncludes() {
for (String p : kieBaseModel.getIncludes()) {
stmt.addStatement(new MethodCallExpr(kieBaseModelNameExpr, "addInclude", nodeList(new StringLiteralExpr(p))));
stmt.addStatement(new MethodCallExpr(kieBaseModelNameExpr, "addInclude", nodeList(toStringLiteral(p))));
}
}

Expand Down Expand Up @@ -318,7 +318,7 @@ private void setSessionModelType() {

private void setClockType() {
NameExpr type = new NameExpr(kieSessionModel.getClockType().getClass().getCanonicalName());
MethodCallExpr clockTypeEnum = new MethodCallExpr(type, "get", nodeList(new StringLiteralExpr(kieSessionModel.getClockType().getClockType())));
MethodCallExpr clockTypeEnum = new MethodCallExpr(type, "get", nodeList(toStringLiteral(kieSessionModel.getClockType().getClockType())));
stmt.addStatement(new MethodCallExpr(nameExpr, "setClockType", nodeList(clockTypeEnum)));

confBlock.addStatement(new MethodCallExpr(confExpr, "setOption", nodeList(clockTypeEnum.clone())));
Expand Down Expand Up @@ -347,7 +347,7 @@ private NameExpr moduleModelNameExpr() {
}

private AssignExpr newInstance( String type, String variableName, NameExpr scope, String methodName, String parameter) {
MethodCallExpr initMethod = new MethodCallExpr(scope, methodName, nodeList(new StringLiteralExpr(parameter)));
MethodCallExpr initMethod = new MethodCallExpr(scope, methodName, nodeList(toStringLiteral(parameter)));
VariableDeclarationExpr var = new VariableDeclarationExpr(new ClassOrInterfaceType(null, type), variableName);
return new AssignExpr(var, initMethod, AssignExpr.Operator.ASSIGN);
}
Expand Down
8 changes: 4 additions & 4 deletions ...el/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/PackageModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import com.github.javaparser.ast.expr.NameExpr;
import com.github.javaparser.ast.expr.ObjectCreationExpr;
import com.github.javaparser.ast.expr.SimpleName;
import com.github.javaparser.ast.expr.StringLiteralExpr;
import com.github.javaparser.ast.expr.ThisExpr;
import com.github.javaparser.ast.stmt.BlockStmt;
import com.github.javaparser.ast.stmt.ReturnStmt;
Expand Down Expand Up @@ -91,6 +90,7 @@
import static org.drools.core.impl.StatefulKnowledgeSessionImpl.DEFAULT_RULE_UNIT;
import static org.drools.core.util.StringUtils.getPkgUUID;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.toClassOrInterfaceType;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.toStringLiteral;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.toVar;
import static org.drools.modelcompiler.builder.generator.DslMethodNames.GLOBAL_OF_CALL;
import static org.drools.modelcompiler.builder.generator.DslMethodNames.createDslTopLevelMethod;
Expand Down Expand Up @@ -809,8 +809,8 @@ private static void addGlobalField(ClassOrInterfaceDeclaration classDeclaration,

MethodCallExpr declarationOfCall = createDslTopLevelMethod(GLOBAL_OF_CALL);
declarationOfCall.addArgument(new ClassExpr(declType ));
declarationOfCall.addArgument(new StringLiteralExpr(packageName));
declarationOfCall.addArgument(new StringLiteralExpr(globalName));
declarationOfCall.addArgument(toStringLiteral(packageName));
declarationOfCall.addArgument(toStringLiteral(globalName));

FieldDeclaration field = classDeclaration.addField(varType, toVar(globalName), publicModifier().getKeyword(), staticModifier().getKeyword(), finalModifier().getKeyword());

Expand Down Expand Up @@ -896,7 +896,7 @@ public void indexConstraint(String exprId, String constraint, String ruleName, S
if (info == null) {
return new PredicateInformation(constraint, ruleName, ruleFileName);
} else {
info.addRuleName(ruleName, ruleFileName);
info.addRuleNames(ruleName, ruleFileName);
return info;
}
});
Expand Down
4 changes: 2 additions & 2 deletions ...-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/Consequence.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import com.github.javaparser.ast.expr.MethodCallExpr;
import com.github.javaparser.ast.expr.NameExpr;
import com.github.javaparser.ast.expr.SimpleName;
import com.github.javaparser.ast.expr.StringLiteralExpr;
import com.github.javaparser.ast.expr.VariableDeclarationExpr;
import com.github.javaparser.ast.stmt.BlockStmt;
import com.github.javaparser.ast.stmt.ExpressionStmt;
Expand Down Expand Up @@ -80,6 +79,7 @@
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.isNameExprWithName;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.parseBlock;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.toClassOrInterfaceType;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.toStringLiteral;
import static org.drools.modelcompiler.builder.generator.DslMethodNames.BREAKING_CALL;
import static org.drools.modelcompiler.builder.generator.DslMethodNames.EXECUTE_CALL;
import static org.drools.modelcompiler.builder.generator.DslMethodNames.GET_CHANNEL_CALL;
Expand Down Expand Up @@ -399,7 +399,7 @@ private MethodCallExpr createBitMaskInitialization(Class<?> updatedClass, Set<St
String domainClassSourceName = asJavaSourceName( updatedClass );
bitMaskCreation = new MethodCallExpr(new NameExpr(BitMask.class.getCanonicalName()), "getPatternMask");
bitMaskCreation.addArgument( DOMAIN_CLASSESS_METADATA_FILE_NAME + packageModel.getPackageUUID() + "." + domainClassSourceName + DOMAIN_CLASS_METADATA_INSTANCE );
modifiedProps.forEach(s -> bitMaskCreation.addArgument(new StringLiteralExpr(s)));
modifiedProps.forEach(s -> bitMaskCreation.addArgument(toStringLiteral(s)));
} else {
bitMaskCreation = new MethodCallExpr(new NameExpr(AllSetButLastBitMask.class.getCanonicalName()), "get");
}
Expand Down
8 changes: 5 additions & 3 deletions ...odel-compiler/src/main/java/org/drools/modelcompiler/builder/generator/DrlxParseUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
import org.drools.core.util.MethodUtils;
import org.drools.core.util.StringUtils;
import org.drools.model.Index;
import org.drools.modelcompiler.builder.errors.GetterOverloadWarning;
import org.drools.modelcompiler.builder.errors.IncompatibleGetterOverloadError;
import org.drools.modelcompiler.builder.errors.InvalidExpressionErrorResult;
import org.drools.mvel.parser.DrlxParser;
Expand All @@ -116,7 +115,6 @@
import static org.drools.modelcompiler.builder.generator.DslMethodNames.isDslTopLevelNamespace;
import static org.drools.modelcompiler.builder.generator.expressiontyper.ExpressionTyper.findLeftLeafOfNameExprTraversingParent;
import static org.drools.modelcompiler.util.ClassUtil.toRawClass;
import static org.drools.mvelcompiler.util.TypeUtils.toJPType;

public class DrlxParseUtil {

Expand Down Expand Up @@ -523,7 +521,7 @@ public static Expression generateLambdaWithoutParameters(Collection<String> used
if (!skipFirstParamAsThis) {
Type type;
if (canResolve) {
type = toJPType(patternClass.get());
type = toClassOrInterfaceType(patternClass.get());
} else {
type = new UnknownType();
}
Expand Down Expand Up @@ -641,6 +639,10 @@ public static ClassOrInterfaceType toClassOrInterfaceType( String className ) {
return withoutDollars.indexOf('<') >= 0 ? StaticJavaParser.parseClassOrInterfaceType(withoutDollars) : new ClassOrInterfaceType(null, withoutDollars);
}

public static StringLiteralExpr toStringLiteral(String s) {
return new StringLiteralExpr(null, s);
}

public static Optional<String> findBindingIdFromDotExpression(String expression) {
int dot = expression.indexOf( '.' );
if ( dot < 0 ) {
Expand Down
19 changes: 10 additions & 9 deletions ...del-compiler/src/main/java/org/drools/modelcompiler/builder/generator/ModelGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.classToReferenceType;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.generateLambdaWithoutParameters;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.toClassOrInterfaceType;
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.toStringLiteral;
import static org.drools.modelcompiler.builder.generator.DslMethodNames.ATTRIBUTE_CALL;
import static org.drools.modelcompiler.builder.generator.DslMethodNames.BUILD_CALL;
import static org.drools.modelcompiler.builder.generator.DslMethodNames.DECLARATION_OF_CALL;
Expand Down Expand Up @@ -223,9 +224,9 @@ private static void processRule(PackageDescr packageDescr, RuleContext context)

MethodCallExpr ruleCall = createDslTopLevelMethod(RULE_CALL);
if (!ruleDescr.getNamespace().isEmpty()) {
ruleCall.addArgument( new StringLiteralExpr( ruleDescr.getNamespace() ) );
ruleCall.addArgument( toStringLiteral( ruleDescr.getNamespace() ) );
}
ruleCall.addArgument( new StringLiteralExpr( ruleDescr.getName() ) );
ruleCall.addArgument( toStringLiteral( ruleDescr.getName() ) );

MethodCallExpr buildCallScope = ruleUnitDescr != null ?
new MethodCallExpr(ruleCall, UNIT_CALL).addArgument( new ClassExpr( toClassOrInterfaceType(ruleUnitDescr.getCanonicalName()) ) ) :
Expand Down Expand Up @@ -308,11 +309,11 @@ private static List<MethodCallExpr> ruleAttributes(RuleContext context, RuleDesc
case "activation-group":
case "ruleflow-group":
case "duration":
attributeCall.addArgument( new StringLiteralExpr( value ) );
attributeCall.addArgument( toStringLiteral( value ) );
break;
case "timer":
if (validateTimer(value)) {
attributeCall.addArgument( new StringLiteralExpr( value ) );
attributeCall.addArgument( toStringLiteral( value ) );
} else {
context.addCompilationError( new InvalidExpressionErrorResult(value) );
}
Expand Down Expand Up @@ -374,7 +375,7 @@ private static List<MethodCallExpr> ruleMetaAttributes(RuleContext context, Rule
List<MethodCallExpr> ruleMetaAttributes = new ArrayList<>();
for (String metaAttr : ruleDescr.getAnnotationNames()) {
MethodCallExpr metaAttributeCall = new MethodCallExpr(METADATA_CALL);
metaAttributeCall.addArgument(new StringLiteralExpr(metaAttr));
metaAttributeCall.addArgument(toStringLiteral(metaAttr));
AnnotationDescr ad = ruleDescr.getAnnotation( metaAttr );
String adFqn = ad.getFullyQualifiedName();
if ("Propagation".equals(metaAttr)) { // legacy case, as explained in the javadoc annotation above, ref. DROOLS-5685
Expand All @@ -390,7 +391,7 @@ private static List<MethodCallExpr> ruleMetaAttributes(RuleContext context, Rule
}
if ( annotationDefinition.getValues().size() == 1 && annotationDefinition.getValues().containsKey( AnnotationDescr.VALUE ) ) {
Object annValue = annotationDefinition.getPropertyValue(AnnotationDescr.VALUE);
metaAttributeCall.addArgument(new StringLiteralExpr(annValue.toString()));
metaAttributeCall.addArgument(toStringLiteral(annValue.toString()));
} else {
Map<String, Object> map = new HashMap<>( annotationDefinition.getValues().size() );
for ( String key : annotationDefinition.getValues().keySet() ) {
Expand Down Expand Up @@ -467,7 +468,7 @@ private static void addUnitData(RuleContext context, RuleUnitVariable unitVar, B
MethodCallExpr unitDataCall = createDslTopLevelMethod(UNIT_DATA_CALL);

unitDataCall.addArgument(new ClassExpr( declType ));
unitDataCall.addArgument(new StringLiteralExpr(unitVar.getName()));
unitDataCall.addArgument(toStringLiteral(unitVar.getName()));

AssignExpr var_assign = new AssignExpr(var_, unitDataCall, AssignExpr.Operator.ASSIGN);
ruleBlock.addStatement(var_assign);
Expand Down Expand Up @@ -502,13 +503,13 @@ private static void addVariable(BlockStmt ruleBlock, DeclarationSpec declaration
declarationOfCall.addArgument( DOMAIN_CLASSESS_METADATA_FILE_NAME + context.getPackageModel().getPackageUUID() + "." + domainClassSourceName + DOMAIN_CLASS_METADATA_INSTANCE );
}

declarationOfCall.addArgument(new StringLiteralExpr(declaration.getVariableName().orElse(declaration.getBindingId())));
declarationOfCall.addArgument(toStringLiteral(declaration.getVariableName().orElse(declaration.getBindingId())));

declaration.getDeclarationSource().ifPresent(declarationOfCall::addArgument);

declaration.getEntryPoint().ifPresent( ep -> {
MethodCallExpr entryPointCall = createDslTopLevelMethod(ENTRY_POINT_CALL);
entryPointCall.addArgument( new StringLiteralExpr(ep ) );
entryPointCall.addArgument( toStringLiteral(ep ) );
declarationOfCall.addArgument( entryPointCall );
} );
for ( BehaviorDescr behaviorDescr : declaration.getBehaviors() ) {
Expand Down
Loading

0 comments on commit 524bcc2

Please sign in to comment.