[gnugo-devel] persistent reading cache inconsistency patch

[Evan Berggren Daniel]

This patch is my attempted fix for reading inconsistencies due to the
persistent cache.

I renamed attack() and find_defense(), and added a parameter to control
whether they check the persistent cache before doing reading.  attack()
and find_defense() as before now just call the new functions.  When
attack_and_defend finds an inconsistency, it tries calling attack() and
find_defense() without caching to fix it.  This of course slows down
attack() and find_defense by a small amount, but since those aren't the
recursive functions, I don't think this is a big deal.

I still don't know whether the patch works, because I haven't been able to
reproduce the inconsistency discussed.  So, this probably shouldn't be
added to the main tree until it has demonstrably fixed something.  It's
also worth noting that it doesn't break anything, and so is probably safe
to add if we want more widespread testing.

In the mean time, I'll keep looking for examples.

Thanks

Evan Daniel

Index: engine/reading.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/reading.c,v
retrieving revision 1.78
diff -u -d -r1.78 reading.c
--- engine/reading.c    28 Sep 2002 14:15:26 -0000      1.78
+++ engine/reading.c    30 Sep 2002 19:33:53 -0000
@@ -190,7 +190,8 @@
 static int simple_ladder_attack(int str, int *move, int komaster, int kom_pos);
 static int simple_ladder_defend(int str, int *move, int komaster, int kom_pos);
 static int in_list(int move, int num_moves, int *moves);
-
+static int attack_sc(int str, int *move, int search);
+static int find_defense_sc(int str, int *move, int search);

 /* Statistics. */
 static int reading_node_counter = 0;
@@ -230,9 +231,18 @@
  *     which must be answered (the defender makes the first ko capture).
  */

+/* the normal way to call the function */
 int
 attack(int str, int *move)
 {
+  return attack_sc(str, move, 1); /*attack, searching the cache*/
+}
+
+/* do the attack, optionally searching the cache*/
+
+int
+attack_sc(int str, int *move, int search)
+{
   int result;
   int nodes;
   int origin;
@@ -246,10 +256,12 @@
     return 0;

   origin = find_origin(str);
-  if (search_persistent_reading_cache(ATTACK, origin, &result, &the_move)) {
-    if (move)
-      *move = the_move;
-    return result;
+  if (search) {
+    if (search_persistent_reading_cache(ATTACK, origin, &result, &the_move)) {
+      if (move)
+        *move = the_move;
+      return result;
+    }
   }

   memset(shadow, 0, sizeof(shadow));
@@ -289,9 +301,15 @@
  * which must be answered.
  */

-int
+int
 find_defense(int str, int *move)
 {
+  return find_defense_sc(str, move, 1); /*attack, searching the cache*/
+}
+
+int
+find_defense_sc(int str, int *move, int search)
+{
   int result;
   int nodes;
   int origin;
@@ -308,11 +326,13 @@
   }

   origin = find_origin(str);
-  if (search_persistent_reading_cache(FIND_DEFENSE, origin,
-                                     &result, &the_move)) {
-    if (move)
-      *move = the_move;
-    return result;
+  if (search) {
+    if (search_persistent_reading_cache(FIND_DEFENSE, origin,
+                                     &result, &the_move)) {
+      if (move)
+        *move = the_move;
+      return result;
+    }
   }

   memset(shadow, 0, sizeof(shadow));
@@ -372,16 +392,32 @@
   if (acode != 0) {
     dcode = find_defense(str, &dpos);

-    /* If find_defense() says the string is safe as is, we believe
-     * this in favor of attack()'s opinion. Actually this is probably
-     * incorrect, but we can't easily find a defense point to return.
+    /* If find_defense() says the string is safe as is, we decide
+     * that something went wrong in the persistent cache and try again,
+     * without checking the cache for results.
      */
     if (dcode == WIN && dpos == NO_MOVE) {
-      acode = 0;
-      apos = NO_MOVE;
+#if 0
+      gprintf("Inconsistent read result, retrying without cache.\n");
+#endif
+      acode = attack_sc(str, &apos, 0);
+      dcode = find_defense_sc(str, &dpos, 0);
     }
   }
-
+  /* The above allegedly fixes the following problem, so if it
+   * occurs anyway, we want to find out.
+   */
+  gg_assert(!(acode != 0 && dcode == WIN && dpos == NO_MOVE));
+
+  /* If find_defense() says the string is safe as is, we believe that
+   * in favor of attack()'s opinion.  We should really do some debugging
+   * here, because if this occurs then it shouldn't be the result of
+   * cache problems.
+   */
+  if (acode != 0 && dcode == WIN && dpos == NO_MOVE) {
+    acode = 0;
+    apos = NO_MOVE;
+  }
+
   if (attack_code)
     *attack_code = acode;
   if (attack_point)