Skip to content

Commit

Permalink
[KIE-DROOLS-6190] fix removal of detached tuples during incremental c…
Browse files Browse the repository at this point in the history
…ompilation (#6192)

* [KIE-DROOLS-6190] fix removal of detached tuples during incremental compilation

* add missing license

* wip

* wip

* wip
  • Loading branch information
mariofusco authored Dec 13, 2024
1 parent c4711a9 commit 14f5601
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -507,30 +507,6 @@ public void addLastLeftTuple( TupleImpl leftTuple) {
lastLeftTuple = leftTuple;
}

private void addLastTuple(TupleImpl tuple, boolean left) {
if (left) {
addLastLeftTuple(tuple);
} else {
addLastRightTuple(tuple);
}
}

private void setFirstTuple(TupleImpl tuple, boolean left) {
if (left) {
firstLeftTuple = tuple;
} else {
firstRightTuple = tuple;
}
}

private void setLastTuple(TupleImpl tuple, boolean left) {
if (left) {
lastLeftTuple = tuple;
} else {
lastRightTuple = tuple;
}
}

@Override
public void removeLeftTuple( TupleImpl leftTuple ) {
TupleImpl previous = leftTuple.getHandlePrevious();
Expand Down Expand Up @@ -683,6 +659,7 @@ public TupleImpl detachLeftTupleAfter(RuleBasePartitionId partitionId, ObjectTyp
if (detached != null) {
if (firstLeftTuple == detached) {
firstLeftTuple = null;
lastLeftTuple = null;
}

if (lastLeftTuple == detached) {
Expand All @@ -695,7 +672,6 @@ public TupleImpl detachLeftTupleAfter(RuleBasePartitionId partitionId, ObjectTyp
lastLeftTuple.setHandleNext(null);
}
}

return detached;
}

Expand All @@ -711,6 +687,7 @@ public TupleImpl detachRightTupleAfter(RuleBasePartitionId partitionId, ObjectTy
if (detached != null) {
if (firstRightTuple == detached) {
firstRightTuple = null;
lastRightTuple = null;
}

if (lastRightTuple == detached) {
Expand All @@ -723,7 +700,6 @@ public TupleImpl detachRightTupleAfter(RuleBasePartitionId partitionId, ObjectTy
lastRightTuple.setHandleNext(null);
}
}

return detached;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,6 @@ public void removeRule(TerminalNode tn, Collection<InternalWorkingMemory> wms, I
}
}

Set<SegmentMemoryPair> smemsToNotify = new HashSet<>();

if (exclBranchRoots.isEmpty()) {
LeftTupleNode lian = tn.getPathNodes()[0];
processLeftTuples(lian, false, tn, wms);
Expand All @@ -163,7 +161,7 @@ public void removeRule(TerminalNode tn, Collection<InternalWorkingMemory> wms, I

// Process existing branches from the split points
Set<Integer> visited = new HashSet<>();
exclBranchRoots.forEach(pair -> Remove.processMerges(pair.parent, tn, kBase, wms, visited, smemsToNotify));
exclBranchRoots.forEach(pair -> Remove.processMerges(pair.parent, tn, kBase, wms, visited));
}

for (InternalWorkingMemory wm : wms) {
Expand All @@ -174,8 +172,6 @@ public void removeRule(TerminalNode tn, Collection<InternalWorkingMemory> wms, I
pmem.getRuleAgendaItem().dequeue();
}
}

smemsToNotify.forEach(pair -> pair.sm.notifyRuleLinkSegment(pair.wm));
}

public static void notifyImpactedSegments(SegmentMemory smem, InternalWorkingMemory wm, Set<SegmentMemoryPair> segmentsToNotify) {
Expand Down Expand Up @@ -736,8 +732,7 @@ private static void removeExistingPaths(List<Pair> exclBranchRoots, TerminalNode
}
}


private static void processMerges(LeftTupleNode splitNode, TerminalNode tn, InternalRuleBase kBase, Collection<InternalWorkingMemory> wms, Set<Integer> visited, Set<SegmentMemoryPair> smemsToNotify) {
private static void processMerges(LeftTupleNode splitNode, TerminalNode tn, InternalRuleBase kBase, Collection<InternalWorkingMemory> wms, Set<Integer> visited) {
// it's possible for a rule to have multiple exclBranches, pointing to the same parent. So need to ensure it's processed once.
if ( !visited.add(splitNode.getId())) {
return;
Expand Down Expand Up @@ -766,10 +761,7 @@ private static void processMerges(LeftTupleNode splitNode, TerminalNode tn, Inte
}

SegmentPrototype proto2 = kBase.getSegmentPrototype(ltn);

mergeSegments(proto1, proto2, kBase, wms);

notifyImpactedSegments(wms, proto1, smemsToNotify);
}
}

Expand Down Expand Up @@ -1443,17 +1435,6 @@ private static void updatePaths(SegmentPrototype proto, Collection<InternalWorki
}
}

private static void notifyImpactedSegments(Collection<InternalWorkingMemory> wms, SegmentPrototype proto1, Set<SegmentMemoryPair> smemsToNotify) {
// any impacted segments must be notified for potential linking
for (InternalWorkingMemory wm : wms) {
Memory mem1 = wm.getNodeMemories().peekNodeMemory(proto1.getRootNode());
if (mem1 != null && mem1.getSegmentMemory() != null) {
// there was a split segment, both need notifying.
notifyImpactedSegments(mem1.getSegmentMemory(), wm, smemsToNotify);
}
}
}

private static void setNodeTypes(SegmentPrototype proto, LeftTupleNode[] protoNodes) {
int nodeTypesInSegment = 0;
for ( LeftTupleNode node : protoNodes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public void innerEval(PathMemory pmem,
}

boolean emptySrcTuples = srcTuples.isEmpty();
if ( !(NodeTypeEnums.isBetaNode(node) && ((BetaNode)node).isRightInputIsRiaNode() ) ) {
if ( !(NodeTypeEnums.isBetaNode(node) && node.isRightInputIsRiaNode() ) ) {
// The engine cannot skip a ria node, as the dirty might be several levels deep
if ( emptySrcTuples && smem.getDirtyNodeMask() == 0) {
// empty sources and segment is not dirty, skip to non empty src tuples or dirty segment.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,7 @@ public void removeTerminalNode(RuleRemovalContext context, TerminalNode tn, Coll

tn.visitLeftTupleNodes(n -> n.removeAssociatedTerminal(tn));

BaseNode node = (BaseNode) tn;
removeNodeAssociation(node, context.getRule(), new HashSet<>(), context);

removeNodeAssociation((BaseNode) tn, context.getRule(), new HashSet<>(), context);
resetMasks(removeNodes((AbstractTerminalNode)tn, workingMemories, context));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.junit.jupiter.api.Disabled;

@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesEvalTest extends AbstractAddRemoveGenerated2RulesTest {

public static Stream<ConstraintsPair> parameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.junit.jupiter.api.Disabled;

@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesIntegerTest extends AbstractAddRemoveGenerated2RulesTest {

public static Stream<ConstraintsPair> parameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.junit.jupiter.api.Disabled;

@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesMapContainsTest extends AbstractAddRemoveGenerated2RulesTest {

public static Stream<ConstraintsPair> parameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.junit.jupiter.api.Disabled;

@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesNotNotTest extends AbstractAddRemoveGenerated2RulesTest {

public static Stream<ConstraintsPair> parameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.junit.jupiter.api.Disabled;

@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesNotTest extends AbstractAddRemoveGenerated2RulesTest {

public static Stream<ConstraintsPair> parameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.junit.jupiter.api.Disabled;

@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesStringIntegerTest extends AbstractAddRemoveGenerated2RulesTest {

public static Stream<ConstraintsPair> parameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.junit.jupiter.api.Disabled;

@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesStringTest extends AbstractAddRemoveGenerated2RulesTest {

public static Stream<ConstraintsPair> parameters() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.drools.compiler.integrationtests.incrementalcompilation;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;

public class CasesFromGeneratedRulesTest {

@Test
@Timeout(40000)
public void testInsertFactsFireRulesRemoveRulesReinsertRulesRevertedRules() {
String rule1 = """
package com.rules;
global java.util.List list
rule R1
when
Integer()
then
list.add('R1');
end
""";

String rule2 = """
package com.rules;
global java.util.List list
rule R2
when
Integer()
exists(Integer() and Integer())
exists(Integer() and Integer())
then
list.add('R2');
end
""";

AddRemoveTestCases.insertFactsFireRulesRemoveRulesReinsertRules1(rule1, rule2, TestUtil.RULE1_NAME, TestUtil.RULE2_NAME, null, 1, 2);
}

@Test
@Timeout(40000)
public void testInsertFactsRemoveRulesFireRulesRemoveRules() {
String rule1 = """
package com.rules;
global java.util.List list
rule R1
when
exists(Integer())
not(Double() and Double())
Integer() not(Double() and Double())
then
list.add('R1');
end
""";

String rule2 = """
package com.rules;
global java.util.List list
rule R2
when
exists(Integer())
not(Double() and Double())
exists(Integer())
then
list.add('R2');
end
""";

AddRemoveTestCases.insertFactsRemoveRulesFireRulesRemoveRules2(rule1, rule2, TestUtil.RULE1_NAME, TestUtil.RULE2_NAME, null, 1);
}
}

0 comments on commit 14f5601

Please sign in to comment.