Optimize $indexFor by caching index locations in a weak map

src/FirebaseArray.js: added a cache of record indices
test/unit/firebase.spec.js: Fix broken test case
test/mocks/mock.utils.js: unused parameter

Author: katowulf

src/FirebaseArray.js

@@ -63,6 +63,13 @@
         this._promise = readyPromise;
         this._destroyFn = destroyFn;
 
+        // indexCache is a weak hashmap (a lazy list) of keys to array indices,
+        // items are not guaranteed to stay up to date in this list (since the list
+        // can be manually edited without calling the $ methods) and it should
+        // always be used with skepticism regarding whether it is accurate
+        // (see $indexFor() below for proper usage)
+        this._indexCache = {};
+
         // Array.isArray will not work on objects which extend the Array class.
         // So instead of extending the Array class, we just return an actual array.
         // However, it's still possible to extend FirebaseArray and have the public methods
@@ -175,8 +182,16 @@
          */
         $indexFor: function(key) {
           var self = this;
-          // todo optimize and/or cache these? they wouldn't need to be perfect
-          return this.$list.findIndex(function(rec) { return self._getKey(rec) === key; });
+          var cache = self._indexCache;
+          // evaluate whether our key is cached and, if so, whether it is up to date
+          if( !cache.hasOwnProperty(key) || self.$keyAt(cache[key]) !== key ) {
+            // update the hashmap
+            var pos = self.$list.findIndex(function(rec) { return self._getKey(rec) === key; });
+            if( pos !== -1 ) {
+              cache[key] = pos;
+            }
+          }
+          return cache.hasOwnProperty(key)? cache[key] : -1;
         },
 
         /**
@@ -349,7 +364,7 @@
          * @returns {string||null}
          * @private
          */
-        _getKey: function(rec) {
+        _getKey: function(rec) { //todo rename this to $$getId
           return angular.isObject(rec)? rec.$id : null;
         },
 
@@ -366,13 +381,13 @@
         $$process: function(event, rec, prevChild) {
           var key = this._getKey(rec);
           var changed = false;
-          var pos;
+          var curPos;
           switch(event) {
             case 'child_added':
-              pos = this.$indexFor(key);
+              curPos = this.$indexFor(key);
               break;
             case 'child_moved':
-              pos = this.$indexFor(key);
+              curPos = this.$indexFor(key);
               this._spliceOut(key);
               break;
             case 'child_removed':
@@ -385,9 +400,9 @@
             default:
               throw new Error('Invalid event type ' + event);
           }
-          if( angular.isDefined(pos) ) {
+          if( angular.isDefined(curPos) ) {
             // add it to the array
-            changed = this._addAfter(rec, prevChild) !== pos;
+            changed = this._addAfter(rec, prevChild) !== curPos;
           }
           if( changed ) {
             // send notifications to anybody monitoring $watch
@@ -433,6 +448,7 @@
             if( i === 0 ) { i = this.$list.length; }
           }
           this.$list.splice(i, 0, rec);
+          this._indexCache[this._getKey(rec)] = i;
           return i;
         },
 
@@ -447,6 +463,7 @@
         _spliceOut: function(key) {
           var i = this.$indexFor(key);
           if( i > -1 ) {
+            delete this._indexCache[key];
             return this.$list.splice(i, 1)[0];
           }
           return null;
@@ -466,12 +483,13 @@
             return list[indexOrItem];
           }
           else if( angular.isObject(indexOrItem) ) {
-            var i = list.length;
-            while(i--) {
-              if( list[i] === indexOrItem ) {
-                return indexOrItem;
-              }
-            }
+            // it must be an item in this array; it's not sufficient for it just to have
+            // a $id or even a $id that is in the array, it must be an actual record
+            // the fastest way to determine this is to use $getRecord (to avoid iterating all recs)
+            // and compare the two
+            var key = this._getKey(indexOrItem);
+            var rec = this.$getRecord(key);
+            return rec === indexOrItem? rec : null;
           }
           return null;
         },
@@ -530,4 +548,4 @@
       return FirebaseArray;
     }
   ]);
-})();
+})();

tests/mocks/mock.utils.js

@@ -17,7 +17,7 @@ angular.module('mock.utils', [])
       $delegate[method]._super = origMethod;
     }
 
-    $provide.decorator('$firebaseUtils', function($delegate, $timeout) {
+    $provide.decorator('$firebaseUtils', function($delegate) {
       spyOnCallback($delegate, 'compile');
       spyOnCallback($delegate, 'wait');
       return $delegate;

tests/unit/firebase.spec.js

@@ -712,6 +712,7 @@ describe('$firebase', function () {
     });
 
     it('should batch requests', function() {
+      $fb.$asObject(); // creates the listeners
       flushAll();
       $utils.wait.completed.calls.reset();
       var ref = $fb.$ref();