From eb1e646cd487023b663c46f6d7d13ef85aad232e Mon Sep 17 00:00:00 2001 From: Grigorii Kirgizov Date: Tue, 15 Dec 2020 19:36:41 +0300 Subject: [PATCH] Minor refactoring: move OccurrenceContractObserver from ConstraintsProcessing into ProcessingStrategy Rename NonIncrementalProcessing -> DefaultProcessing Add GroundProcessing as common superclass to handle OccurrenceContractObserver Add another test for this occurrence contract logic. --- .../core/internal/ConstraintsProcessing.kt | 25 ++----- .../core/internal/EvaluationSessionImpl.kt | 4 +- .../internal/OccurrenceContractObserver.kt | 6 +- .../core/internal/ProcessingStrategy.kt | 71 ++++++++++++------- reactor/Test/test/TestIncrementalProgram.kt | 56 ++++++++++++++- 5 files changed, 112 insertions(+), 50 deletions(-) diff --git a/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ConstraintsProcessing.kt b/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ConstraintsProcessing.kt index 60c0afc4..45ff9234 100644 --- a/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ConstraintsProcessing.kt +++ b/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ConstraintsProcessing.kt @@ -46,12 +46,7 @@ internal class ConstraintsProcessing( ) : StoreAwareJournalImpl(journal, logicalState), IncrSpecHolder { - private var incrementalProcessing: ProcessingStrategy = NonIncrementalProcessing() - - // todo: move to IncrementalProcessing - private val occurrenceContractObserver: OccurrenceContractObserver? = - if (ispec.assertLevel().assertContracts()) OccurrenceContractObserver(logicalState, ispec) else null - + private var incrementalProcessing: ProcessingStrategy = DefaultProcessing() fun setStrategy(strategy: ProcessingStrategy) { this.incrementalProcessing = strategy } @@ -108,9 +103,7 @@ internal class ConstraintsProcessing( active.stored = true logActivation(active) active.revive(logicalState) - - // fixme: move to incremental facade - occurrenceContractObserver?.onActivated(active) + incrementalProcessing.processActivated(active, logicalState) } assert(active.alive) @@ -194,7 +187,8 @@ internal class ConstraintsProcessing( profiler.profile("terminateOccurrence") { - occ.clearLogicalState(logicalState) + occ.terminate(logicalState) + incrementalProcessing.processDiscarded(occ, logicalState) } @@ -204,18 +198,11 @@ internal class ConstraintsProcessing( } } - private fun Occurrence.clearLogicalState(observable: LogicalStateObservable) { - terminate(observable) - if (occurrenceContractObserver != null && this.isPrincipal) { - occurrenceContractObserver.onDiscarded(this) - } - } - - inner class ProgramStateCleaner{ fun erase(occurrence: Occurrence) { dispatchingFront = dispatchingFront.forget(occurrence) - occurrence.clearLogicalState(logicalState) + occurrence.terminate(logicalState) + incrementalProcessing.processDiscarded(occurrence, logicalState) } fun erase(match: RuleMatchEx) { diff --git a/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/EvaluationSessionImpl.kt b/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/EvaluationSessionImpl.kt index 54ea9e40..bfb22478 100644 --- a/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/EvaluationSessionImpl.kt +++ b/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/EvaluationSessionImpl.kt @@ -114,7 +114,7 @@ internal class EvaluationSessionImpl private constructor ( val logicalState = LogicalState() val dispatchingFront = Dispatcher(ruleIndex).front() - val processingStrategy = NonIncrementalProcessing() + val processingStrategy = GroundProcessing(incrementality) val processing = ConstraintsProcessing(dispatchingFront, journal, logicalState, incrementality, trace, profiler) processing.setStrategy(processingStrategy) @@ -168,7 +168,6 @@ internal class EvaluationSessionImpl private constructor ( override fun endSession(session: SessionParts): SessionToken = with(session) { val histView = journal.view() processing.resetStore() // clear observers - // todo: need clearing occurrenceContractObservers? (MPSCR-66) val principalState = sessionState(frontState).resetLookup() return SessionTokenImpl(histView, emptyList(), ruleIndex.toRules(), principalState, logicalState, ruleIndex) } @@ -213,7 +212,6 @@ internal class EvaluationSessionImpl private constructor ( outputOccurrences.forEach{ it.terminate(logicalState) } processing.resetStore() // clear observers - // todo: need clearing occurrenceContractObservers? (MPSCR-66) logicalState.reset() val rules = ruleIndex.toRules().filter(preambleInfo::inPreamble) diff --git a/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/OccurrenceContractObserver.kt b/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/OccurrenceContractObserver.kt index 5cf008ab..506af991 100644 --- a/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/OccurrenceContractObserver.kt +++ b/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/OccurrenceContractObserver.kt @@ -33,16 +33,16 @@ import kotlin.collections.HashSet * must be immutable, that is, logicals can't be unified * with either ground or free other logicals. */ -internal class OccurrenceContractObserver(private val observable: LogicalStateObservable, override val ispec: IncrementalSpec): IncrSpecHolder { +internal class OccurrenceContractObserver(override val ispec: IncrementalSpec): IncrSpecHolder { private val observers: HashMap = hashMapOf() - fun onActivated(occ: Occurrence) { + fun onActivated(occ: Occurrence, observable: LogicalStateObservable) { if (occ.isPrincipal) { observers[occ.identity] = UnmodifiableLogicalObserver(occ, observable) } } - fun onDiscarded(occ: Occurrence) { + fun onDiscarded(occ: Occurrence, observable: LogicalStateObservable) { // NB: works between incremental sessions only if this instance is preserved between them // observers.remove(occ.identity)?.removeObservers(observable) diff --git a/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ProcessingStrategy.kt b/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ProcessingStrategy.kt index 77085598..891e4ae2 100644 --- a/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ProcessingStrategy.kt +++ b/reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ProcessingStrategy.kt @@ -54,13 +54,26 @@ internal interface ProcessingStrategy { * Should return a filtered [matches] list. */ fun processOccurrenceMatches(active: Occurrence, matches: List): List + + /** + * Called on new activated [Occurrence]. Allows to handle program's + * logical state, e.g. add processing-specific observers with [observable]. + */ + fun processActivated(active: Occurrence, observable: LogicalStateObservable): Unit + + /** + * Called for each replaced [Occurrence] in match's head. + * It's a pair method for [processActivated] to discharge its effects, + * e.g. for clearing program logical state. + */ + fun processDiscarded(occ: Occurrence, observable: LogicalStateObservable): Unit } /** * Default non-incremental processing with stubs. */ -internal class NonIncrementalProcessing: ProcessingStrategy { +internal open class DefaultProcessing: ProcessingStrategy { override fun invalidatedFeedback(): FeedbackKeySet = emptySet() @@ -75,6 +88,31 @@ internal class NonIncrementalProcessing: ProcessingStrategy { override fun processMatch(match: RuleMatchEx) {} override fun processOccurrenceMatches(active: Occurrence, matches: List): List = matches + + override fun processActivated(active: Occurrence, observable: LogicalStateObservable) {} + + override fun processDiscarded(occ: Occurrence, observable: LogicalStateObservable) {} +} + + +/** + * Processing strategy that observes logical vars and ensures basic incremental contract. + */ +internal open class GroundProcessing(override val ispec: IncrementalSpec): DefaultProcessing(), IncrSpecHolder { + + private val occurrenceContractObserver: OccurrenceContractObserver? = + if (ispec.assertLevel().assertContracts()) OccurrenceContractObserver(ispec) else null + + override fun processActivated(active: Occurrence, observable: LogicalStateObservable) { + occurrenceContractObserver?.onActivated(active, observable) + } + + override fun processDiscarded(occ: Occurrence, observable: LogicalStateObservable) { + if (occ.isPrincipal) { + occurrenceContractObserver?.onDiscarded(occ, observable) + } + } + } @@ -97,14 +135,14 @@ internal class NonIncrementalProcessing: ProcessingStrategy { * in [IncrementalProcessing]. */ internal class IncrementalProcessing( - override val ispec: IncrementalSpec, + ispec: IncrementalSpec, val journal: MatchJournal, newRules: Iterable, droppedRules: Iterable, stateCleaner: ConstraintsProcessing.ProgramStateCleaner, ruleIndex: RuleIndex, trace: EvaluationTrace -): ProcessingStrategy, IncrSpecHolder { +): GroundProcessing(ispec) { private val journalIndex = journal.index() private val ruleOrdering = RuleOrdering(ruleIndex) @@ -127,6 +165,7 @@ internal class IncrementalProcessing( override fun processOccurrenceMatches(active: Occurrence, matches: List) = postponeFutureMatchesImpl(active, matches) + override fun run(processing: ConstraintsProcessing, controller: Controller, main: Constraint): FeedbackStatus { var status: FeedbackStatus = FeedbackStatus.NORMAL() val cursor = journal.cursor @@ -188,15 +227,15 @@ internal class IncrementalProcessing( * * Journal invalidation and injected intermediate processing with * [processMatch] & [processOccurrenceMatches] are not needed for this. - * So this strategy is very close to the default [NonIncrementalProcessing]. + * So this strategy is very close to the default [DefaultProcessing]. */ internal class PreambleProcessing( - override val ispec: IncrementalSpec, + ispec: IncrementalSpec, val journal: MatchJournal, newRules: Iterable, ruleIndex: RuleIndex, trace: EvaluationTrace -): ProcessingStrategy, IncrSpecHolder { +): GroundProcessing(ispec) { private val journalIndex = journal.index() private val ruleOrdering = RuleOrdering(ruleIndex) @@ -204,16 +243,6 @@ internal class PreambleProcessing( private val continuator = ContinueOccurrencesStage(ispec, journalIndex) private val adder = AdditionStage(ispec, newRules, continuator, ruleOrdering, ruleIndex, trace) - - override fun invalidatedFeedback(): FeedbackKeySet = emptySet() - - override fun invalidatedRules(): List = emptyList() - - override fun processMatch(match: RuleMatchEx) = Unit - - override fun processOccurrenceMatches(active: Occurrence, matches: List) = matches - - override fun run(processing: ConstraintsProcessing, controller: Controller, main: Constraint): FeedbackStatus { var status: FeedbackStatus = FeedbackStatus.NORMAL() val cursor = journal.cursor @@ -243,21 +272,15 @@ internal class PreambleProcessing( * Importantly, one ensured by [OccurrenceContractObserver]. */ internal class CachedOccurrencesProcessing( - override val ispec: IncrementalSpec, + ispec: IncrementalSpec, private val occurrences: OccurrenceStore -): ProcessingStrategy, IncrSpecHolder { +): GroundProcessing(ispec) { private var inPreamble = true private val postponedMatches: MutableList>> = mutableListOf() - override fun invalidatedFeedback(): FeedbackKeySet = emptySet() - - override fun invalidatedRules(): List = emptyList() - - override fun processMatch(match: RuleMatchEx) = Unit - override fun processOccurrenceMatches(active: Occurrence, matches: List) = if (inPreamble && matches.isNotEmpty()) { postponedMatches.add(active to matches) diff --git a/reactor/Test/test/TestIncrementalProgram.kt b/reactor/Test/test/TestIncrementalProgram.kt index 5f11d7a6..33e761a6 100644 --- a/reactor/Test/test/TestIncrementalProgram.kt +++ b/reactor/Test/test/TestIncrementalProgram.kt @@ -1574,11 +1574,65 @@ class TestIncrementalProgram { )) ).launch("violate contract assertion", progSpec) { result -> - result.storeView().constraintSymbols() shouldBe setOf(sym1("foo"), sym1("hasBound")) + //NB: this check isn't supposed to be even run because of exception + //result.storeView().constraintSymbols() shouldBe setOf(sym1("foo"), sym1("hasBound")) } } + @Test(expected = EvaluationFailureException::class) + fun violatePrincipalLogicalContractOnRelaunch() { + val progSpec = MockIncrProgSpec( + setOf("main", "produceBound"), + setOf(sym1("foo")) + ).withContractChecks() + + val (X, Y) = metaLogical("X", "Y") + + val fooMatch = + rule("produceBound", + headKept( + pconstraint("foo", X) + ), + body( + statement({ x, y -> eq(x,y) }, X, Y), + constraint("hasBound", Y) + )) + + programWithRules( + rule("main", + headReplaced( + constraint("main") + ), + body( + pconstraint("foo", X) + )), + + // place for inserting 'fooMatch' rule + + rule("bindVar", + headKept( + constraint("hasBound", Y) + ), + body( + statement({ y -> y.set(42) }, Y) + )) + + ).launch("normal run", progSpec) { result -> + + result.storeView().constraintSymbols() shouldBe setOf(sym1("foo")) + + }.let { (builder, evalRes) -> + builder + .insertRulesAt(1, fooMatch) + .relaunch("violate contract assertion", progSpec, evalRes.token()) { result -> + + //NB: this check isn't supposed to be even run because of exception + //result.storeView().constraintSymbols() shouldBe setOf(sym1("foo"), sym1("hasBound")) + } + } + } + @Test fun expectTypeGenericNonprincipal() {