Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor getter caching based on keypath state #223

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 41 additions & 2 deletions src/getter.js
Expand Up @@ -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 {
Expand All @@ -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}
Expand All @@ -88,7 +127,6 @@ function getStoreDeps(getter) {
}

const storeDeps = getFlattenedDeps(getter)
.map(keyPath => keyPath.first())
.filter(x => !!x)


Expand All @@ -106,6 +144,7 @@ export default {
isGetter,
getComputeFn,
getFlattenedDeps,
getCanonicalKeypathDeps,
getStoreDeps,
getDeps,
fromKeyPath,
Expand Down
12 changes: 0 additions & 12 deletions src/key-path.js
Expand Up @@ -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)
}
90 changes: 47 additions & 43 deletions src/reactor.js
@@ -1,15 +1,15 @@
import Immutable from 'immutable'
import { Map, is, Set } from 'immutable'
import createReactMixin from './create-react-mixin'
import * as fns from './reactor/fns'
import { DefaultCache } from './reactor/cache'
import { 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 ObserverState from './reactor/observer-state'
import {
ReactorState,
ObserverState,
DEBUG_OPTIONS,
PROD_OPTIONS,
} from './reactor/records'
Expand Down Expand Up @@ -60,8 +60,22 @@ class Reactor {
* @return {*}
*/
evaluate(keyPathOrGetter) {
let { result, reactorState } = fns.evaluate(this.reactorState, keyPathOrGetter)
this.reactorState = reactorState
let result

this.reactorState = this.reactorState.withMutations(reactorState => {
if (!isKeyPath(keyPathOrGetter)) {
// look through the keypathStates and see if any of the getters dependencies are dirty, if so resolve
// against the previous reactor state
const maxCacheDepth = fns.getOption(reactorState, 'maxCacheDepth')
fns.resolveDirtyKeypathStates(
this.prevReactorState,
reactorState,
getCanonicalKeypathDeps(keyPathOrGetter, maxCacheDepth)
)
}
result = fns.evaluate(reactorState, keyPathOrGetter)
})

return result
}

Expand Down Expand Up @@ -95,10 +109,9 @@ class Reactor {
handler = getter
getter = []
}
let { observerState, entry } = fns.addObserver(this.observerState, getter, handler)
this.observerState = observerState
const entry = this.observerState.addObserver(this.reactorState, getter, handler)
return () => {
this.observerState = fns.removeObserverByEntry(this.observerState, entry)
this.observerState.removeObserverByEntry(this.reactorState, entry)
}
}

Expand All @@ -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.removeObserver(this.reactorState, getter, handler)
}

/**
Expand All @@ -130,6 +143,7 @@ class Reactor {
}

try {
this.prevReactorState = this.reactorState
this.reactorState = fns.dispatch(this.reactorState, actionType, payload)
} catch (e) {
this.__isDispatching = false
Expand Down Expand Up @@ -171,6 +185,7 @@ class Reactor {
* @param {Object} stores
*/
registerStores(stores) {
this.prevReactorState = this.reactorState
this.reactorState = fns.registerStores(this.reactorState, stores)
this.__notify()
}
Expand All @@ -196,6 +211,7 @@ class Reactor {
* @param {Object} state
*/
loadState(state) {
this.prevReactorState = this.reactorState
this.reactorState = fns.loadState(this.reactorState, state)
this.__notify()
}
Expand All @@ -220,59 +236,47 @@ class Reactor {
return
}

const dirtyStores = this.reactorState.get('dirtyStores')
if (dirtyStores.size === 0) {
return
}
this.prevReactorState = this.prevReactorState.asMutable()
this.reactorState = this.reactorState.asMutable()

fns.getLoggerFunction(this.reactorState, 'notifyStart')(this.reactorState, this.observerState)

let observerIdsToNotify = Immutable.Set().withMutations(set => {
// notify all observers
set.union(this.observerState.get('any'))
const keypathsToResolve = this.observerState.getTrackedKeypaths()
const changedKeypaths = fns.resolveDirtyKeypathStates(
this.prevReactorState,
this.reactorState,
keypathsToResolve,
true // increment all dirty states (this should leave no unknown state in the keypath tracker map):
)

dirtyStores.forEach(id => {
const entries = this.observerState.getIn(['stores', id])
if (!entries) {
return
}
set.union(entries)
})
})
// get observers to notify based on the keypaths that changed
const observersToNotify = this.observerState.getObserversToNotify(changedKeypaths)

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.hasObserver(observer)) {
// the observer was removed in a hander function
return
}
let didCall = false

const getter = entry.get('getter')
const handler = entry.get('handler')
const getter = observer.get('getter')
const handler = observer.get('handler')

fns.getLoggerFunction(this.reactorState, 'notifyEvaluateStart')(this.reactorState, getter)

const prevEvaluateResult = fns.evaluate(this.prevReactorState, getter)
const currEvaluateResult = fns.evaluate(this.reactorState, getter)

this.prevReactorState = prevEvaluateResult.reactorState
this.reactorState = currEvaluateResult.reactorState
const prevValue = fns.evaluate(this.prevReactorState, getter)
const currValue = fns.evaluate(this.reactorState, getter)

const prevValue = prevEvaluateResult.result
const currValue = currEvaluateResult.result

if (!Immutable.is(prevValue, currValue)) {
// TODO(jordan) pull some comparator function out of the reactorState
if (!is(prevValue, currValue)) {
handler.call(null, currValue)
didCall = true
}
fns.getLoggerFunction(this.reactorState, 'notifyEvaluateEnd')(this.reactorState, getter, didCall, currValue)
})

const nextReactorState = fns.resetDirtyStores(this.reactorState)

this.prevReactorState = nextReactorState
this.reactorState = nextReactorState
this.prevReactorState = this.prevReactorState.asImmutable()
this.reactorState = this.reactorState.asImmutable()

fns.getLoggerFunction(this.reactorState, 'notifyEnd')(this.reactorState, this.observerState)
}
Expand Down
37 changes: 31 additions & 6 deletions src/reactor/cache.js
Expand Up @@ -2,7 +2,7 @@ import { Map, OrderedSet, Record } from 'immutable'

export const CacheEntry = Record({
value: null,
storeStates: Map(),
states: Map(),
dispatchId: null,
})

Expand Down Expand Up @@ -93,6 +93,25 @@ export class BasicCache {
evict(item) {
return new BasicCache(this.cache.remove(item))
}

/**
* Removes entry from cache
* @param {Iterable} items
* @return {BasicCache}
*/
evictMany(items) {
const newCache = this.cache.withMutations(c => {
items.forEach(item => {
c.remove(item)
})
})

return new BasicCache(newCache)
}

empty() {
return new BasicCache()
}
}

const DEFAULT_LRU_LIMIT = 1000
Expand Down Expand Up @@ -173,15 +192,12 @@ export class LRUCache {
)
}

const cache = (this.lru
.take(this.evictCount)
.reduce((c, evictItem) => c.evict(evictItem), this.cache)
.miss(item, entry))
const itemsToRemove = this.lru.take(this.evictCount)

lruCache = new LRUCache(
this.limit,
this.evictCount,
cache,
this.cache.evictMany(itemsToRemove).miss(item, entry),
this.lru.skip(this.evictCount).add(item)
)
} else {
Expand Down Expand Up @@ -212,6 +228,15 @@ export class LRUCache {
this.lru.remove(item)
)
}

empty() {
return new LRUCache(
this.limit,
this.evictCount,
this.cache.empty(),
OrderedSet()
)
}
}

/**
Expand Down