From f61175dfb1d09b7723f130bf74a03f504596a5d5 Mon Sep 17 00:00:00 2001 From: Michael Ilseman Date: Sat, 22 Jul 2023 07:31:46 -0700 Subject: [PATCH 1/4] wip: trivial save points --- .../Engine/Backtracking.swift | 44 +++++++++---------- .../_StringProcessing/Engine/MECapture.swift | 8 ++++ .../_StringProcessing/Engine/Processor.swift | 36 ++++++++++----- 3 files changed, 54 insertions(+), 34 deletions(-) diff --git a/Sources/_StringProcessing/Engine/Backtracking.swift b/Sources/_StringProcessing/Engine/Backtracking.swift index 48470ce91..ff099f35e 100644 --- a/Sources/_StringProcessing/Engine/Backtracking.swift +++ b/Sources/_StringProcessing/Engine/Backtracking.swift @@ -28,25 +28,15 @@ extension Processor { // save point stack var stackEnd: CallStackAddress - // FIXME: Save minimal info (e.g. stack position and - // perhaps current start) - var captureEnds: [_StoredCapture] - - // The int registers store values that can be relevant to - // backtracking, such as the number of trips in a quantification. - var intRegisters: [Int] - // Same with position registers - var posRegisters: [Input.Index] + var savedStateIndex: Array.Index var destructure: ( pc: InstructionAddress, pos: Position?, stackEnd: CallStackAddress, - captureEnds: [_StoredCapture], - intRegisters: [Int], - PositionRegister: [Input.Index] + savedStateIndex: Array.Index ) { - return (pc, pos, stackEnd, captureEnds, intRegisters, posRegisters) + return (pc, pos, stackEnd, savedStateIndex) } var rangeIsEmpty: Bool { rangeEnd == nil } @@ -82,36 +72,44 @@ extension Processor { } } - func makeSavePoint( + mutating func makeSavePoint( _ pc: InstructionAddress, addressOnly: Bool = false ) -> SavePoint { - SavePoint( + // TODO: Only push back if there's been a change in state + savedState.append(.init( + captureEnds: storedCaptures, + intRegisters: registers.ints, + posRegisters: registers.positions)) + + return SavePoint( pc: pc, pos: addressOnly ? nil : currentPosition, rangeStart: nil, rangeEnd: nil, isScalarSemantics: false, // FIXME: refactor away stackEnd: .init(callStack.count), - captureEnds: storedCaptures, - intRegisters: registers.ints, - posRegisters: registers.positions) + savedStateIndex: savedState.index(before: savedState.endIndex)) } - func startQuantifierSavePoint( + mutating func startQuantifierSavePoint( isScalarSemantics: Bool ) -> SavePoint { + // TODO: Only push back if there's been a change in state + savedState.append(.init( + captureEnds: storedCaptures, + intRegisters: registers.ints, + posRegisters: registers.positions)) + // Restores to the instruction AFTER the current quantifier instruction - SavePoint( + return SavePoint( pc: controller.pc + 1, pos: nil, rangeStart: nil, rangeEnd: nil, isScalarSemantics: isScalarSemantics, stackEnd: .init(callStack.count), - captureEnds: storedCaptures, - intRegisters: registers.ints, - posRegisters: registers.positions) + savedStateIndex: savedState.index(before: savedState.endIndex)) } } diff --git a/Sources/_StringProcessing/Engine/MECapture.swift b/Sources/_StringProcessing/Engine/MECapture.swift index 4bea21133..e9354e43e 100644 --- a/Sources/_StringProcessing/Engine/MECapture.swift +++ b/Sources/_StringProcessing/Engine/MECapture.swift @@ -31,6 +31,14 @@ extension Processor { + + struct SavedState { + var captureEnds: [_StoredCapture] + var intRegisters: [Int] + var posRegisters: [Input.Index] + } + + struct _StoredCapture { var range: Range? = nil diff --git a/Sources/_StringProcessing/Engine/Processor.swift b/Sources/_StringProcessing/Engine/Processor.swift index 0350a37db..d718d8c93 100644 --- a/Sources/_StringProcessing/Engine/Processor.swift +++ b/Sources/_StringProcessing/Engine/Processor.swift @@ -77,6 +77,8 @@ struct Processor { var callStack: [InstructionAddress] = [] + var savedState: [SavedState] = [] + var storedCaptures: Array<_StoredCapture> var wordIndexCache: Set? = nil @@ -324,13 +326,11 @@ extension Processor { state = .fail return } - let (pc, pos, stackEnd, capEnds, intRegisters, posRegisters): ( + let (pc, pos, stackEnd, savedStateIndex): ( pc: InstructionAddress, pos: Position?, stackEnd: CallStackAddress, - captureEnds: [_StoredCapture], - intRegisters: [Int], - PositionRegister: [Input.Index] + savedStateIndex: Array.Index ) let idx = savePoints.index(before: savePoints.endIndex) @@ -338,22 +338,36 @@ extension Processor { if !savePoints[idx].rangeIsEmpty { savePoints[idx].takePositionFromRange(input) } + + // TODO: clean up the quantifier save point logic some + let shouldRemoveSavePoint = savePoints[idx].rangeIsEmpty + // If we have a normal save point or an empty quantifier save point, remove it - if savePoints[idx].rangeIsEmpty { - (pc, pos, stackEnd, capEnds, intRegisters, posRegisters) = savePoints.removeLast().destructure + if shouldRemoveSavePoint { + (pc, pos, stackEnd, savedStateIndex) = savePoints.removeLast().destructure + + + } else { - (pc, pos, stackEnd, capEnds, intRegisters, posRegisters) = savePoints[idx].destructure + (pc, pos, stackEnd, savedStateIndex) = savePoints[idx].destructure } assert(stackEnd.rawValue <= callStack.count) - assert(capEnds.count == storedCaptures.count) + assert(savedState[savedStateIndex].captureEnds.count == storedCaptures.count) controller.pc = pc currentPosition = pos ?? currentPosition callStack.removeLast(callStack.count - stackEnd.rawValue) - storedCaptures = capEnds - registers.ints = intRegisters - registers.positions = posRegisters + storedCaptures = savedState[savedStateIndex].captureEnds + registers.ints = savedState[savedStateIndex].intRegisters + registers.positions = savedState[savedStateIndex].posRegisters + + if shouldRemoveSavePoint { + // TODO: Have save points share saved state when identical, and in that + // case add a check to see whether we should pop or not + assert(savedStateIndex == savedState.index(before: savedState.endIndex)) + _ = savedState.removeLast() + } metrics.addBacktrack() } From 0df040ea2b7d1c9a1b460a9ee43d58754ff705e9 Mon Sep 17 00:00:00 2001 From: Michael Ilseman Date: Sat, 22 Jul 2023 08:00:27 -0700 Subject: [PATCH 2/4] wip: broken assertion, broken assumptions --- Sources/_StringProcessing/Engine/Processor.swift | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Sources/_StringProcessing/Engine/Processor.swift b/Sources/_StringProcessing/Engine/Processor.swift index d718d8c93..dd76fd791 100644 --- a/Sources/_StringProcessing/Engine/Processor.swift +++ b/Sources/_StringProcessing/Engine/Processor.swift @@ -365,7 +365,15 @@ extension Processor { if shouldRemoveSavePoint { // TODO: Have save points share saved state when identical, and in that // case add a check to see whether we should pop or not - assert(savedStateIndex == savedState.index(before: savedState.endIndex)) + if !(savedStateIndex == savedState.index(before: savedState.endIndex)) { +// print(savedState) +// print(savedState.count) +// print(savedStateIndex) +// assert(false) + + // FIXME: This assertion gets hit, where the saved index is _not_ the + // FIXME: last saved state. Yet, the tests pass... Fix this. + } _ = savedState.removeLast() } From 12d5b725765cafffceced5ef79de1a2a39ba02f7 Mon Sep 17 00:00:00 2001 From: Michael Ilseman Date: Sat, 22 Jul 2023 09:42:59 -0700 Subject: [PATCH 3/4] wip: hammer out just a little more, still hitting assertion --- .../_StringProcessing/Engine/MEQuantify.swift | 4 +++ .../_StringProcessing/Engine/Processor.swift | 29 +++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Sources/_StringProcessing/Engine/MEQuantify.swift b/Sources/_StringProcessing/Engine/MEQuantify.swift index 1ff734ccd..493cebbb6 100644 --- a/Sources/_StringProcessing/Engine/MEQuantify.swift +++ b/Sources/_StringProcessing/Engine/MEQuantify.swift @@ -80,6 +80,10 @@ extension Processor { if !savePoint.rangeIsEmpty { savePoints.append(savePoint) + } else { + // TODO: Refactor to make logic smoother... + // TODO: Skip this unnecessary saved state traffic + rectifySaveState(savePoint.savedStateIndex) } return true } diff --git a/Sources/_StringProcessing/Engine/Processor.swift b/Sources/_StringProcessing/Engine/Processor.swift index dd76fd791..1eec82924 100644 --- a/Sources/_StringProcessing/Engine/Processor.swift +++ b/Sources/_StringProcessing/Engine/Processor.swift @@ -137,6 +137,7 @@ extension Processor { self.registers.reset(sentinel: searchBounds.upperBound) self.savePoints.removeAll(keepingCapacity: true) + self.savedState.removeAll(keepingCapacity: true) self.callStack.removeAll(keepingCapacity: true) for idx in storedCaptures.indices { @@ -321,6 +322,14 @@ extension Processor { return true } + // TODO: better name / design, come up with something applicable for both + // signalFailure and clearThrough + mutating func rectifySaveState(_ idx: Array.Index) { + // FIXME: invariant should hold, even though tests pass... +// assert(idx == savedState.index(before: savedState.endIndex)) + _ = savedState.removeLast() + } + mutating func signalFailure() { guard !savePoints.isEmpty else { state = .fail @@ -345,9 +354,6 @@ extension Processor { // If we have a normal save point or an empty quantifier save point, remove it if shouldRemoveSavePoint { (pc, pos, stackEnd, savedStateIndex) = savePoints.removeLast().destructure - - - } else { (pc, pos, stackEnd, savedStateIndex) = savePoints[idx].destructure } @@ -363,18 +369,7 @@ extension Processor { registers.positions = savedState[savedStateIndex].posRegisters if shouldRemoveSavePoint { - // TODO: Have save points share saved state when identical, and in that - // case add a check to see whether we should pop or not - if !(savedStateIndex == savedState.index(before: savedState.endIndex)) { -// print(savedState) -// print(savedState.count) -// print(savedStateIndex) -// assert(false) - - // FIXME: This assertion gets hit, where the saved index is _not_ the - // FIXME: last saved state. Yet, the tests pass... Fix this. - } - _ = savedState.removeLast() + rectifySaveState(savedStateIndex) } metrics.addBacktrack() @@ -403,6 +398,7 @@ extension Processor { mutating func clearThrough(_ address: InstructionAddress) { while let sp = savePoints.popLast() { + rectifySaveState(sp.savedStateIndex) if sp.pc == address { controller.step() return @@ -472,7 +468,8 @@ extension Processor { controller.pc = nextPC case .clear: - if let _ = savePoints.popLast() { + if let sp = savePoints.popLast() { + rectifySaveState(sp.savedStateIndex) controller.step() } else { // TODO: What should we do here? From 3122ca65bc9739c6baff4e8cced3187039f716f4 Mon Sep 17 00:00:00 2001 From: Michael Ilseman Date: Sat, 22 Jul 2023 09:56:29 -0700 Subject: [PATCH 4/4] finish up trivial save points, at least this experiment --- .../_StringProcessing/Engine/MEQuantify.swift | 16 ++++++++++++++++ Sources/_StringProcessing/Engine/Processor.swift | 15 ++++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/Sources/_StringProcessing/Engine/MEQuantify.swift b/Sources/_StringProcessing/Engine/MEQuantify.swift index 493cebbb6..b2daacec3 100644 --- a/Sources/_StringProcessing/Engine/MEQuantify.swift +++ b/Sources/_StringProcessing/Engine/MEQuantify.swift @@ -74,6 +74,10 @@ extension Processor { } if trips < payload.minTrips { + // TODO: Refactor to make logic smoother... + // TODO: Skip this unnecessary saved state traffic + rectifySaveState(savePoint.savedStateIndex) + signalFailure() return false } @@ -108,6 +112,10 @@ extension Processor { savePoint.shrinkRange(input) if !savePoint.rangeIsEmpty { savePoints.append(savePoint) + } else { + // TODO: Refactor to make logic smoother... + // TODO: Skip this unnecessary saved state traffic + rectifySaveState(savePoint.savedStateIndex) } return true } @@ -128,6 +136,10 @@ extension Processor { } if savePoint.rangeIsEmpty { + // TODO: Refactor to make logic smoother... + // TODO: Skip this unnecessary saved state traffic + rectifySaveState(savePoint.savedStateIndex) + signalFailure() return false } @@ -135,6 +147,10 @@ extension Processor { savePoint.shrinkRange(input) if !savePoint.rangeIsEmpty { savePoints.append(savePoint) + } else { + // TODO: Refactor to make logic smoother... + // TODO: Skip this unnecessary saved state traffic + rectifySaveState(savePoint.savedStateIndex) } return true } diff --git a/Sources/_StringProcessing/Engine/Processor.swift b/Sources/_StringProcessing/Engine/Processor.swift index 1eec82924..cf11ad04a 100644 --- a/Sources/_StringProcessing/Engine/Processor.swift +++ b/Sources/_StringProcessing/Engine/Processor.swift @@ -325,8 +325,7 @@ extension Processor { // TODO: better name / design, come up with something applicable for both // signalFailure and clearThrough mutating func rectifySaveState(_ idx: Array.Index) { - // FIXME: invariant should hold, even though tests pass... -// assert(idx == savedState.index(before: savedState.endIndex)) + assert(idx == savedState.index(before: savedState.endIndex)) _ = savedState.removeLast() } @@ -649,9 +648,15 @@ extension Processor { let (val, cap) = payload.pairedValueCapture let value = registers[val] let capNum = Int(asserting: cap.rawValue) - let sp = makeSavePoint(self.currentPC) - storedCaptures[capNum].registerValue( - value, overwriteInitial: sp) + + // FIXME: Why is this overwriteInitial in here? It's not used inside + // FIXME: registerValue at all + + // let sp = makeSavePoint(self.currentPC) + // storedCaptures[capNum].registerValue( + // value, overwriteInitial: sp) + + storedCaptures[capNum].registerValue(value) controller.step() } }