fix exponential blowup in diff algorith; wip

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:
+      assert i < a.len
-        if oldNode.kind notin {VNodeKind.component, VNodeKind.vthunk, VNodeKind.dthunk}:
+      assert j < b.len
-          assert current != nil
+      let r = if isSpecial:
-          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:
                 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:
+    # compute common prefix:
-      #  updateStyles(a[i], b[j])
-      #  a[i] = b[j]
-      #  action
-
     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
+    let pos = min(oldPos, newPos) + 1
-    for i in commonPrefix..pos-1:
+    # now the different children are in commonPrefix .. pos - 1:
-      if diff(newNode[i], oldNode[i], current, current.childNodes[i],
+    when false:
-              kxi) != different:
+      for i in commonPrefix..pos-1:
-        newNode[i] = oldNode[i]
+        detach(oldNode[i])
-      else:
+        kxi.addPatch(pkReplace, current, current.childNodes[i], newNode[i])
-        result = different
+    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..<n.len:
+      m = max(m, depth(n[i], total))
+    result = m + 1
+    inc total
 
 proc dodraw(kxi: KaraxInstance) =
   if kxi.renderer.isNil: return
@@ -391,7 +429,7 @@ proc dodraw(kxi: KaraxInstance) =
     #kout cstring"patch len ", patches.len
     apply(kxi)
     kxi.currentTree = newtree
-  #doAssert same(kxi.currentTree, document.getElementById(kxi.rootId))
+  doAssert same(kxi.currentTree, document.getElementById(kxi.rootId))
 
   if not kxi.postRenderCallback.isNil:
     kxi.postRenderCallback()
@@ -400,6 +438,10 @@ proc dodraw(kxi: KaraxInstance) =
   if kxi.toFocus != nil:
     kxi.toFocus.focus()
   kxi.renderId = 0
+  kxi.recursion = 0
+  when defined(stats):
+    var total = 0
+    echo "depth ", depth(kxi.currentTree, total), " total ", total
 
 proc reqFrame(callback: proc()): int {.importc: "window.requestAnimationFrame".}
 proc cancelFrame(id: int) {.importc: "window.cancelAnimationFrame".}

tests/difftest.nim

@@ -9,12 +9,27 @@ proc hasDom(n: Vnode) =
     doAssert n.dom != nil
     for i in 0..<n.len: hasDom(n[i])
 
-proc doDiff(a, b: VNode) =
+proc shortRepr(n: VNode): string =
-  var patches = newJSeq[Patch]()
+  if n == nil:
-  echo diff(b, a, nil, vnodeToDom(a, kxi), patches)
+    result = "nil"
-  for i in 0..<patches.len:
+  elif n.kind == VNodeKind.text:
-    echo patches[i]
+    result = $n.text
+  else:
+    result = $n.kind
+    for i in 0..<n.len:
+      result &= " " & shortRepr(n[i])
+
+proc doDiff(a, b: VNode; expected: varargs[string]) =
+  discard diff(b, a, nil, vnodeToDom(a, kxi), kxi)
+  for i in 0..<kxi.patchLen:
+    let p = $kxi.patches[i].k & " " & shortRepr(kxi.patches[i].n)
+    echo "got ", p
+    if i >= 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()