Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
}
}
Loading