Improve performance when saving a number of variables that don't have space allocated in the EEPROM. This avoids scanning the entire EEPROM every time a variable that has not yet had space allocated is written.

git-svn-id: https://arducopter.googlecode.com/svn/trunk@1534 f9c3cf11-9bcb-44bc-f272-b75c42450872

libraries/AP_Common/AP_Var.cpp

@@ -421,6 +421,23 @@ bool AP_Var::_EEPROM_scan(void)
         eeprom_address += sizeof(var_header) + var_header.size + 1;
     }
 
+    // Mark any variables that weren't assigned addresses as not-allocated,
+    // so that we don't waste time looking for them again later.
+    //
+    // Note that this isn't done when the header is not found on an empty EEPROM.
+    // The first variable written on an empty EEPROM falls out as soon as the
+    // header is not found.  The second will scan and find one variable, then
+    // mark all the rest as not allocated.
+    //
+    vp = _variables;
+    while (vp) {
+        if (vp->_key & k_key_not_located) {
+            vp->_key |= k_key_not_allocated;
+            log("key %u not allocated", vp->key());
+        }
+        vp = vp->_link;
+    }
+
     // Scanning is complete
     log("scan done");
     _tail_sentinel = eeprom_address;
@@ -448,12 +465,11 @@ bool AP_Var::_EEPROM_locate(bool allocate)
         return true;                // it has
     }
 
-    // We don't know where this variable belongs; try scanning the EEPROM
-    //
-    // XXX this is going to *suck* for mass-save operations.  Do we need
-    //     a flag/key bit that indicates that a variable was known during
-    //     but not located by a scan?
+    // We don't know where this variable belongs.  If the variable isn't
+    // marked as already having been looked for and not found in EEPROM,
+    // try scanning to see if we can locate it.
     //
+    if (!(_key & k_key_not_allocated)) {
         log("need scan");
         _EEPROM_scan();
 
@@ -462,6 +478,8 @@ bool AP_Var::_EEPROM_locate(bool allocate)
         if (!(_key & k_key_not_located)) {
             return true;                // it has
         }
+    }
+
     // If not located and not permitted to allocate, we have failed.
     //
     if (!allocate) {
@@ -508,7 +526,7 @@ bool AP_Var::_EEPROM_locate(bool allocate)
     // tail sentinel location.
     //
     new_location = _tail_sentinel;
-    _tail_sentinel += sizeof(Var_header) + size;
+    _tail_sentinel += sizeof(var_header) + size;
     log("allocated %u/%u for key %u new sentinel %u", new_location, size, key(), _tail_sentinel);
 
     // Write the new sentinel first.  If we are interrupted during this operation
@@ -522,7 +540,7 @@ bool AP_Var::_EEPROM_locate(bool allocate)
     //
     var_header.key = key();
     var_header.size = size - 1;
-    eeprom_write_block(&var_header, (void *)new_location, sizeof(Var_header));
+    eeprom_write_block(&var_header, (void *)new_location, sizeof(var_header));
 
     // We have successfully allocated space and thus located the variable.
     // Update _key to point to the space allocated for it.

libraries/AP_Common/AP_Var.h

@@ -108,6 +108,12 @@ public:
     ///
     static const Key k_key_not_located = (Key)1 << 15;
 
+    /// A key that has this bit set was not found during a scan of the EEPROM.
+    /// If this bit is set in the key, it's not useful to scan the EEPROM again
+    /// in an attempt to find it.
+    ///
+    static const Key k_key_not_allocated = (Key)1 << 14;
+
     /// Key assigned to the terminal entry in EEPROM.
     ///
     static const Key k_key_sentinel = 0xff;
@@ -115,7 +121,7 @@ public:
     /// A bitmask that removes any control bits from a key giving just the
     /// value.
     ///
-    static const Key k_key_mask = ~(k_key_not_located);
+    static const Key k_key_mask = ~(k_key_not_located | k_key_not_allocated);
 
     /// The largest variable that will be saved to EEPROM.
     /// This affects the amount of stack space that is required by the ::save, ::load,

libraries/AP_Common/examples/AP_Var/AP_Var.pde

@@ -307,13 +307,16 @@ setup(void)
 
         AP_Float    f1(10.0, 1);
         AP_Float    f2(123.0, 2);
+        AP_Int8     i(17, 3);
 
         REQUIRE(true == AP_Var::save_all());
         f1 = 0;
         f2 = 0;
+        i = 0;
         REQUIRE(true == AP_Var::load_all());
         REQUIRE(f1 == 10.0);
         REQUIRE(f2 == 123.0);
+        REQUIRE(i == 17);
 
         AP_Var::erase_all();
     }
@@ -339,57 +342,6 @@ setup(void)
         AP_Var::erase_all();
     }
 
-#if SAVE
-    // AP_Var: load and save
-    {
-        TEST(var_load_save);
-
-        AP_Float    f1(10, 4);
-        AP_Float    f2(0, 4);
-
-        f2.save();
-        f2 = 1.0;
-        f2.load();
-        REQUIRE(f2 == 0);
-
-        f1.save();
-        f2.load();
-        REQUIRE(f2 == 10);
-    }
-
-    // AP_Var: group load/save
-    {
-        TEST(var_group_loadsave);
-
-        AP_Var_group	group(PSTR("group_"), 4);
-        AP_Float    	f1(10.0, 8);
-        AP_Float    	f2(1.0, 4, PSTR("var"), &group);
-
-        f1.save();
-        f1.load();
-        REQUIRE(f1 == 10);
-
-        f2.save();
-        f2.load();
-        REQUIRE(f2 == 1);
-
-        f1.load();
-        REQUIRE(f1 == 1);
-    }
-
-    // AP_Var: derived types
-    {
-        TEST(var_derived);
-
-        AP_Float16	f(10.0, 20);
-
-        f.save();
-        f = 0;
-        REQUIRE(f == 0);
-        f.load();
-        REQUIRE(f = 10.0);
-    }
-#endif
 
     Test::report();
 }