From 43ee8903561b0d06adb26c1d88f6d8ecb4cd7929 Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Mon, 3 Jul 2017 23:03:30 +0200 Subject: [PATCH] fix exponential blowup in diff algorith; wip --- src/karax.nim | 96 +++++++++++++++++++++++++++++++++------------- tests/difftest.nim | 37 +++++++++++++----- 2 files changed, 96 insertions(+), 37 deletions(-) diff --git a/src/karax.nim b/src/karax.nim index 3c41d7e..0be777a 100644 --- a/src/karax.nim +++ b/src/karax.nim @@ -34,6 +34,7 @@ type renderId: int patches: seq[Patch] # we reuse this to save allocations patchLen: int + recursion: int var @@ -161,6 +162,9 @@ proc vnodeToDom(n: VNode; kxi: KaraxInstance): Node = proc same(n: VNode, e: Node): bool = if n.kind == VNodeKind.component: result = same(VComponent(n).expanded, e) + elif n.kind == VNodeKind.vthunk or n.kind == VNodeKind.dthunk: + # we don't check these for now: + result = true elif toTag[n.kind] == e.nodename: result = true if n.kind != VNodeKind.text: @@ -178,7 +182,7 @@ proc replaceById(id: cstring; newTree: Node) = type EqResult = enum - changed, different, similar, identical + changed, different, similar, identical, usenewNode proc eq(a, b: VNode): EqResult = if a.kind != b.kind: return different @@ -274,6 +278,13 @@ proc apply(kxi: KaraxInstance) = kxi.patchLen = 0 proc diff(newNode, oldNode: VNode; parent, current: Node; kxi: KaraxInstance): EqResult = + if kxi.recursion > 100: + echo "newNode ", newNode.kind, " oldNode ", oldNode.kind, " eq ", eq(newNode, oldNode) + if oldNode.kind == VNodeKind.text: + echo oldNode.text + return + #doAssert false, "overflow!" + inc kxi.recursion result = eq(newNode, oldNode) case result of identical, similar: @@ -293,68 +304,85 @@ proc diff(newNode, oldNode: VNode; parent, current: Node; kxi: KaraxInstance): E assert oldNode.kind == newNode.kind var commonPrefix = 0 + let isSpecial = oldNode.kind == VNodeKind.component or + oldNode.kind == VNodeKind.vthunk or + oldNode.kind == VNodeKind.dthunk template eqAndUpdate(a: VNode; i: int; b: VNode; j: int; info, action: untyped) = let oldLen = kxi.patchLen - when false: - if oldNode.kind notin {VNodeKind.component, VNodeKind.vthunk, VNodeKind.dthunk}: - assert current != nil - assert current.childNodes[j] != nil, $info - assert oldNode.len == current.len - - let r = if oldNode.kind == VNodeKind.component or oldNode.kind == VNodeKind.vthunk or - oldNode.kind == VNodeKind.dthunk: + assert i < a.len + assert j < b.len + let r = if isSpecial: diff(a[i], b[j], parent, current, kxi) else: - diff(a[i], b[j], current, current.childNodes[j], kxi) + (assert j < current.len; + diff(a[i], b[j], current, current.childNodes[j], kxi)) case r of identical, changed, similar: a[i] = b[j] action + of usenewNode: + b[j] = a[i] + action of different: # undo what 'diff' would have done: kxi.patchLen = oldLen - if result != different: result = r + #if result != different: result = r break - #of similar: - # updateStyles(a[i], b[j]) - # a[i] = b[j] - # action - + # compute common prefix: while commonPrefix < minLength: eqAndUpdate(newNode, commonPrefix, oldNode, commonPrefix, cstring"prefix"): inc commonPrefix + # compute common suffix: var oldPos = oldLength - 1 var newPos = newLength - 1 while oldPos >= commonPrefix and newPos >= commonPrefix: eqAndUpdate(newNode, newPos, oldNode, oldPos, cstring"suffix"): dec oldPos dec newPos + echo "came here" - var pos = min(oldPos, newPos) + 1 - for i in commonPrefix..pos-1: - if diff(newNode[i], oldNode[i], current, current.childNodes[i], - kxi) != different: - newNode[i] = oldNode[i] - else: - result = different + let pos = min(oldPos, newPos) + 1 + # now the different children are in commonPrefix .. pos - 1: + when false: + for i in commonPrefix..pos-1: + detach(oldNode[i]) + kxi.addPatch(pkReplace, current, current.childNodes[i], newNode[i]) + when true: + for i in commonPrefix..pos-1: + let r = diff(newNode[i], oldNode[i], current, current.childNodes[i], + kxi) + if r == usenewNode: + oldNode[i] = newNode[i] + elif r != different: + newNode[i] = oldNode[i] + #else: + # result = different if oldPos + 1 == oldLength: for i in pos..newPos: kxi.addPatch(pkAppend, current, nil, newNode[i]) - result = different + result = usenewNode + #result = different else: let before = current.childNodes[oldPos + 1] for i in pos..newPos: kxi.addPatch(pkInsertBefore, current, before, newNode[i]) - result = different + result = usenewNode + #result = different # XXX call 'attach' here? for i in pos..oldPos: detach(oldNode[i]) #doAssert i < current.childNodes.len kxi.addPatch(pkRemove, current, current.childNodes[i], nil) - result = different + result = usenewNode #different + # after the applied patch, conceptually the nodes are identical, so + # no further search is required. 'changed' needs to be propagated + # for the component system to work. 'similar' was transformed into + # identical too: + #if result == different or result == similar: + # result = identical of changed: assert oldNode.kind == VNodeKind.component @@ -375,6 +403,16 @@ proc diff(newNode, oldNode: VNode; parent, current: Node; kxi: KaraxInstance): E of different: detach(oldNode) kxi.addPatch(pkReplace, parent, current, newNode) + of usenewNode: doAssert(false, "eq returned usenewNode") + dec kxi.recursion + +when defined(stats): + proc depth(n: VNode; total: var int): int = + var m = 0 + for i in 0..= expected.len: + echo "patches differ; expected nothing but got: ", p + elif p != expected[i]: + echo "patches differ; expected ", expected[i], " but got: ", p #hasDom(kxi.currentTree) + kxi.patchLen = 0 proc testAppend() = let a = buildHtml(tdiv): @@ -26,7 +41,7 @@ proc testAppend() = li: text "A" li: text "B" li: text "C" - doDiff(a, b) + doDiff(a, b, "pkAppend li C") proc testInsert() = let a = buildHtml(tdiv): @@ -38,7 +53,7 @@ proc testInsert() = li: text "A" li: text "B" li: text "C" - doDiff(a, b) + doDiff(a, b, "pkInsert li B") proc testDelete() = let a = buildHtml(tdiv): @@ -49,10 +64,12 @@ proc testDelete() = let b = buildHtml(tdiv): ul: discard - doDiff(a, b) + doDiff(a, b, "pkDetach li A", "pkRemove nil", + "pkDetach li B", "pkRemove nil", + "pkDetach li C", "pkRemove nil") kxi = KaraxInstance(rootId: cstring"ROOT", renderer: proc (): VNode = discard) -testAppend() +#testAppend() testInsert() -testDelete() +#testDelete()