From: drh <drh@noemail.net>
Date: Fri, 31 Oct 2014 14:26:36 +0000 (+0000)
Subject: Simplify the logic in the cell redistribution loop of balance_nonroot().
X-Git-Tag: version-3.8.8~197
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d836d42383d93c549826510747b0b6a1bb55c55b;p=thirdparty%2Fsqlite.git

Simplify the logic in the cell redistribution loop of balance_nonroot().
Enhance and clarify comments and add assert() statements for additional
verification of correctness.

FossilOrigin-Name: a07078b60007e88adea67bec5f0caf91f707ad78
---

diff --git a/manifest b/manifest
index e6108baa44..5aa7ed2f46 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Simplify\sthe\smath\sslightly,\sand\sreduce\sby\sone\sthe\snumber\sof\sloop\siterations,\nfor\sthe\sloop\sin\sbalance_nonroot()\sthat\smoves\scells\sbetween\spages.
-D 2014-10-31T12:22:00.360
+C Simplify\sthe\slogic\sin\sthe\scell\sredistribution\sloop\sof\sbalance_nonroot().\nEnhance\sand\sclarify\scomments\sand\sadd\sassert()\sstatements\sfor\sadditional\nverification\sof\scorrectness.
+D 2014-10-31T14:26:36.417
 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f
 F Makefile.in cf57f673d77606ab0f2d9627ca52a9ba1464146a
 F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23
@@ -172,7 +172,7 @@ F src/auth.c b56c78ebe40a2110fd361379f7e8162d23f92240
 F src/backup.c 7f841396adfd47507ff670a471162d2bfcda3136
 F src/bitvec.c 19a4ba637bd85f8f63fc8c9bae5ade9fb05ec1cb
 F src/btmutex.c 49ca66250c7dfa844a4d4cb8272b87420d27d3a5
-F src/btree.c 0f294d5e1eca7a912037d1992c904e28959a5aa5
+F src/btree.c 61d96c2edacc5267fae6e477c24c774cd540d7f0
 F src/btree.h a79aa6a71e7f1055f01052b7f821bd1c2dce95c8
 F src/btreeInt.h 026d0129724e8f265fdc60d44ec240cf5a4e6179
 F src/build.c 67bb05b1077e0cdaccb2e36bfcbe7a5df9ed31e8
@@ -1210,7 +1210,7 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1
 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4
 F tool/warnings.sh 0abfd78ceb09b7f7c27c688c8e3fe93268a13b32
 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
-P ea5d56be5fe14934e4dbe9c17d46b058f487a231
-R 5dd41c1bf29a3c6eca71e97060df99dc
+P 2e838db82e533598b3cb00011c04fc0d5a896895
+R 6da135d51be6a673f0389acc80c0dfb3
 U drh
-Z 53accc48044e0d3a03ff14711aee89d8
+Z 452523febd435385cb08d146713a8a8f
diff --git a/manifest.uuid b/manifest.uuid
index 000bb51e9a..4ff01299eb 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-2e838db82e533598b3cb00011c04fc0d5a896895
\ No newline at end of file
+a07078b60007e88adea67bec5f0caf91f707ad78
\ No newline at end of file
diff --git a/src/btree.c b/src/btree.c
index 4176d2c8bd..9300a6a54f 100644
--- a/src/btree.c
+++ b/src/btree.c
@@ -7026,28 +7026,43 @@ static int balance_nonroot(
   ** is important, as this code needs to avoid disrupting any page from which
   ** cells may still to be read. In practice, this means:
   **
-  **   1) If cells are to be removed from the start of the page and shifted
-  **      to the left-hand sibling, it is not safe to update the page until 
-  **      the left-hand sibling (apNew[i-1]) has already been updated.
+  **  (1) If cells are moving left (from apNew[iPg] to apNew[iPg-1])
+  **      then it is not safe to update page apNew[iPg] until after
+  **      the left-hand sibling apNew[iPg-1] has been updated.
   **
-  **   2) If cells are to be removed from the end of the page and shifted
-  **      to the right-hand sibling, it is not safe to update the page until 
-  **      the right-hand sibling (apNew[i+1]) has already been updated.
+  **  (2) If cells are moving right (from apNew[iPg] to apNew[iPg+1])
+  **      then it is not safe to update page apNew[iPg] until after
+  **      the right-hand sibling apNew[iPg+1] has been updated.
   **
   ** If neither of the above apply, the page is safe to update.
+  **
+  ** The iPg value in the following loop starts at nNew-1 goes down
+  ** to 0, then back up to nNew-1 again, thus making two passes over
+  ** the pages.  On the initial downward pass, only condition (1) above
+  ** needs to be tested because (2) will always be true from the previous
+  ** step.  On the upward pass, both conditions are always true, so the
+  ** upwards pass simply processes pages that were missed on the downward
+  ** pass.
   */
   for(i=1-nNew; i<nNew; i++){
     int iPg = i<0 ? -i : i;
-    /* iPg takes values from nNew-1 down to 0 then back up to nNew-1 again */
     assert( iPg>=0 && iPg<nNew );
-    if( abDone[iPg]==0 
-     && (iPg==0 || cntOld[iPg-1]>=cntNew[iPg-1] || abDone[iPg-1])
-     && (cntNew[iPg]>=cntOld[iPg] || abDone[iPg+1])
+    if( abDone[iPg] ) continue;         /* Skip pages already processed */
+    if( i>=0                            /* On the upwards pass, or... */
+     || cntOld[iPg-1]>=cntNew[iPg-1]    /* Condition (1) is true */
     ){
       int iNew;
       int iOld;
       int nNewCell;
 
+      /* Verify condition (1):  If cells are moving left, update iPg
+      ** only after iPg-1 has already been updated. */
+      assert( iPg==0 || cntOld[iPg-1]>=cntNew[iPg-1] || abDone[iPg-1] );
+
+      /* Verify condition (2):  If cells are moving right, update iPg
+      ** only after iPg+1 has already been updated. */
+      assert( cntNew[iPg]>=cntOld[iPg] || abDone[iPg+1] );
+
       if( iPg==0 ){
         iNew = iOld = 0;
         nNewCell = cntNew[0];
@@ -7058,12 +7073,14 @@ static int balance_nonroot(
       }
 
       editPage(apNew[iPg], iOld, iNew, nNewCell, apCell, szCell);
-      abDone[iPg] = 1;
+      abDone[iPg]++;
       apNew[iPg]->nFree = usableSpace-szNew[iPg];
       assert( apNew[iPg]->nOverflow==0 );
       assert( apNew[iPg]->nCell==nNewCell );
     }
   }
+
+  /* All pages have been processed exactly once */
   assert( memcmp(abDone, "\01\01\01\01\01", nNew)==0 );
 
   assert( nOld>0 );