Refactor getter caching based on keypath state

The current version of NuclearJS uses a cache key consisting of store
states (monotomically incresing ID per store).  This has the
disadvantage of allowing only a single level of depth when figuring out
if a cache entry is stale.  This leads to poor performance when the
shape of a Reactor's state is more deep than wide, ie a store having
multiple responsibilities for state tracking.

The implementation is as follows:

- Consumer can set the maxCacheDepth when instantiating a reactor
- Getters are broken down into the canonical set of keypaths based on
the maxCacheDepth
- Add a keypath tracker abstraction to maintain the state value of all
tracked keypaths
- After any state change (`Reactor.__notify`) dirty keypaths are
resolved and then based on which keypaths have changed any dependent
observers are called

Author: jordangarcia

src/getter.js

@@ -54,7 +54,7 @@ function getFlattenedDeps(getter, existing) {
 
     getDeps(getter).forEach(dep => {
       if (isKeyPath(dep)) {
-        set.add(List(dep))
+        set.add(Immutable.List(dep))
       } else if (isGetter(dep)) {
         set.union(getFlattenedDeps(dep))
       } else {
@@ -66,6 +66,45 @@ function getFlattenedDeps(getter, existing) {
   return existing.union(toAdd)
 }
 
+/**
+ * Returns a set of deps that have been flattened and expanded
+ * expanded ex: ['store1', 'key1'] => [['store1'], ['store1', 'key1']]
+ *
+ * Note: returns a keypath as an Immutable.List(['store1', 'key1')
+ * @param {Getter} getter
+ * @param {Number} maxDepth
+ * @return {Immutable.Set}
+ */
+function getCanonicalKeypathDeps(getter, maxDepth) {
+  if (maxDepth === undefined) {
+    throw new Error('Must supply maxDepth argument')
+  }
+
+  const cacheKey = `__storeDeps_${maxDepth}`
+  if (getter.hasOwnProperty(cacheKey)) {
+    return getter[cacheKey]
+  }
+
+  const deps = Immutable.Set().withMutations(set => {
+    getFlattenedDeps(getter).forEach(keypath => {
+      if (keypath.size <= maxDepth) {
+        set.add(keypath)
+      } else {
+        set.add(keypath.slice(0, maxDepth))
+      }
+    })
+  })
+
+  Object.defineProperty(getter, cacheKey, {
+    enumerable: false,
+    configurable: false,
+    writable: false,
+    value: deps,
+  })
+
+  return deps
+}
+
 /**
  * @param {KeyPath}
  * @return {Getter}
@@ -88,7 +127,6 @@ function getStoreDeps(getter) {
   }
 
   const storeDeps = getFlattenedDeps(getter)
-    .map(keyPath => keyPath.first())
     .filter(x => !!x)
 
 
@@ -106,6 +144,7 @@ export default {
   isGetter,
   getComputeFn,
   getFlattenedDeps,
+  getCanonicalKeypathDeps,
   getStoreDeps,
   getDeps,
   fromKeyPath,

src/key-path.js

@@ -13,15 +13,3 @@ export function isKeyPath(toTest) {
   )
 }
 
-/**
- * Checks if two keypaths are equal by value
- * @param {KeyPath} a
- * @param {KeyPath} a
- * @return {Boolean}
- */
-export function isEqual(a, b) {
-  const iA = Immutable.List(a)
-  const iB = Immutable.List(b)
-
-  return Immutable.is(iA, iB)
-}

src/reactor.js

@@ -4,7 +4,7 @@ import * as fns from './reactor/fns'
 import { DefaultCache } from './reactor/cache'
 import { NoopLogger, ConsoleGroupLogger } from './logging'
 import { isKeyPath } from './key-path'
-import { isGetter } from './getter'
+import { isGetter, getCanonicalKeypathDeps } from './getter'
 import { toJS } from './immutable-helpers'
 import { extend, toFactory } from './utils'
 import {
@@ -60,7 +60,20 @@ class Reactor {
    * @return {*}
    */
   evaluate(keyPathOrGetter) {
-    let { result, reactorState } = fns.evaluate(this.reactorState, keyPathOrGetter)
+    // look through the keypathStates and see if any of the getters dependencies are dirty, if so resolve
+    // against the previous reactor state
+    let updatedReactorState = this.reactorState
+    if (!isKeyPath(keyPathOrGetter)) {
+      const maxCacheDepth = fns.getOption(updatedReactorState, 'maxCacheDepth')
+      let res = fns.resolveDirtyKeypathStates(
+        this.prevReactorState,
+        this.reactorState,
+        getCanonicalKeypathDeps(keyPathOrGetter, maxCacheDepth)
+      )
+      updatedReactorState = res.reactorState
+    }
+
+    let { result, reactorState } = fns.evaluate(updatedReactorState, keyPathOrGetter)
     this.reactorState = reactorState
     return result
   }
@@ -95,10 +108,10 @@ class Reactor {
       handler = getter
       getter = []
     }
-    let { observerState, entry } = fns.addObserver(this.observerState, getter, handler)
+    let { observerState, entry } = fns.addObserver(this.reactorState, this.observerState, getter, handler)
     this.observerState = observerState
     return () => {
-      this.observerState = fns.removeObserverByEntry(this.observerState, entry)
+      this.observerState = fns.removeObserverByEntry(this.reactorState, this.observerState, entry)
     }
   }
 
@@ -110,7 +123,7 @@ class Reactor {
       throw new Error('Must call unobserve with a Getter')
     }
 
-    this.observerState = fns.removeObserver(this.observerState, getter, handler)
+    this.observerState = fns.removeObserver(this.reactorState, this.observerState, getter, handler)
   }
 
   /**
@@ -130,6 +143,7 @@ class Reactor {
     }
 
     try {
+      this.prevReactorState = this.reactorState
       this.reactorState = fns.dispatch(this.reactorState, actionType, payload)
     } catch (e) {
       this.__isDispatching = false
@@ -171,6 +185,7 @@ class Reactor {
    * @param {Object} stores
    */
   registerStores(stores) {
+    this.prevReactorState = this.reactorState
     this.reactorState = fns.registerStores(this.reactorState, stores)
     this.__notify()
   }
@@ -196,6 +211,7 @@ class Reactor {
    * @param {Object} state
    */
   loadState(state) {
+    this.prevReactorState = this.reactorState
     this.reactorState = fns.loadState(this.reactorState, state)
     this.__notify()
   }
@@ -210,6 +226,15 @@ class Reactor {
     this.observerState = new ObserverState()
   }
 
+  /**
+   * Denotes a new state, via a store registration, dispatch or some other method
+   * Resolves any outstanding keypath states and sets a new reactorState
+   * @private
+   */
+  __nextState(newState) {
+    // TODO(jordan): determine if this is actually needed
+  }
+
   /**
    * Notifies all change observers with the current state
    * @private
@@ -220,33 +245,32 @@ class Reactor {
       return
     }
 
-    const dirtyStores = this.reactorState.get('dirtyStores')
-    if (dirtyStores.size === 0) {
-      return
-    }
-
-    let observerIdsToNotify = Immutable.Set().withMutations(set => {
-      // notify all observers
-      set.union(this.observerState.get('any'))
+    const keypathsToResolve = this.observerState.get('trackedKeypaths')
+    const { reactorState, changedKeypaths } = fns.resolveDirtyKeypathStates(
+      this.prevReactorState,
+      this.reactorState,
+      keypathsToResolve,
+      true // increment all dirty states (this should leave no unknown state in the keypath tracker map):
+    )
+    this.reactorState = reactorState
 
-      dirtyStores.forEach(id => {
-        const entries = this.observerState.getIn(['stores', id])
-        if (!entries) {
-          return
+    // get observers to notify based on the keypaths that changed
+    let observersToNotify = Immutable.Set().withMutations(set => {
+      changedKeypaths.forEach(keypath => {
+        const entries = this.observerState.getIn(['keypathToEntries', keypath])
+        if (entries && entries.size > 0) {
+          set.union(entries)
         }
-        set.union(entries)
       })
     })
 
-    observerIdsToNotify.forEach((observerId) => {
-      const entry = this.observerState.getIn(['observersMap', observerId])
-      if (!entry) {
-        // don't notify here in the case a handler called unobserve on another observer
+    observersToNotify.forEach((observer) => {
+      if (!this.observerState.get('observers').has(observer)) {
+        // the observer was removed in a hander function
         return
       }
-
-      const getter = entry.get('getter')
-      const handler = entry.get('handler')
+      const getter = observer.get('getter')
+      const handler = observer.get('handler')
 
       const prevEvaluateResult = fns.evaluate(this.prevReactorState, getter)
       const currEvaluateResult = fns.evaluate(this.reactorState, getter)
@@ -257,15 +281,11 @@ class Reactor {
       const prevValue = prevEvaluateResult.result
       const currValue = currEvaluateResult.result
 
+      // TODO pull some comparator function out of the reactorState
       if (!Immutable.is(prevValue, currValue)) {
         handler.call(null, currValue)
       }
     })
-
-    const nextReactorState = fns.resetDirtyStores(this.reactorState)
-
-    this.prevReactorState = nextReactorState
-    this.reactorState = nextReactorState
   }
 
   /**

src/reactor/cache.js

@@ -2,7 +2,7 @@ import { Map, OrderedSet, Record } from 'immutable'
 
 export const CacheEntry = Record({
   value: null,
-  storeStates: Map(),
+  states: Map(),
   dispatchId: null,
 })