From 5d9f03f6ddfc8ed19d5ffef0275d8ed6cccee996 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Sun, 15 Sep 2019 17:02:11 -0400 Subject: [PATCH 1/2] Eliminate the concept of disposable Entry objects. --- src/entry.ts | 40 +++-------------------------- src/index.ts | 22 +--------------- src/tests/api.ts | 67 ------------------------------------------------ tsconfig.json | 1 + 4 files changed, 5 insertions(+), 125 deletions(-) diff --git a/src/entry.ts b/src/entry.ts index b123a9e..6f8a053 100644 --- a/src/entry.ts +++ b/src/entry.ts @@ -1,7 +1,6 @@ import { parentEntrySlot } from "./context"; import { OptimisticWrapOptions } from "./index"; -const reusableEmptyArray: AnyEntry[] = []; const emptySetPool: Set[] = []; const POOL_TARGET_SIZE = 100; @@ -51,7 +50,6 @@ export class Entry { public subscribe: OptimisticWrapOptions["subscribe"]; public unsubscribe?: () => any; - public reportOrphan?: (this: Entry) => any; public readonly parents = new Set(); public readonly childValues = new Map>(); @@ -80,14 +78,7 @@ export class Entry { // (3) valueGet(this.value) is usually returned without recomputation. public recompute(): TValue { assert(! this.recomputing, "already recomputing"); - - if (! rememberParent(this) && maybeReportOrphan(this)) { - // The recipient of the entry.reportOrphan callback decided to dispose - // of this orphan entry by calling entry.dispose(), so we don't need to - // (and should not) proceed with the recomputation. - return void 0 as any; - } - + rememberParent(this); return mightBeDirty(this) ? reallyRecompute(this) : valueGet(this.value); @@ -105,7 +96,7 @@ export class Entry { } public dispose() { - forgetChildren(this).forEach(maybeReportOrphan); + forgetChildren(this); maybeUnsubscribe(this); // Because this entry has been kicked out of the cache (in index.js), @@ -146,10 +137,7 @@ function rememberParent(child: AnyEntry) { } function reallyRecompute(entry: AnyEntry) { - // Since this recomputation is likely to re-remember some of this - // entry's children, we forget our children here but do not call - // maybeReportOrphan until after the recomputation finishes. - const originalChildren = forgetChildren(entry); + forgetChildren(entry); // Set entry as the parent entry while calling recomputeNewValue(entry). parentEntrySlot.withValue(entry, recomputeNewValue, [entry]); @@ -160,11 +148,6 @@ function reallyRecompute(entry: AnyEntry) { setClean(entry); } - // Now that we've had a chance to re-remember any children that were - // involved in the recomputation, we can safely report any orphan - // children that remain. - originalChildren.forEach(maybeReportOrphan); - return valueGet(entry.value); } @@ -264,35 +247,18 @@ function removeDirtyChild(parent: AnyEntry, child: AnyEntry) { } } -// If the given entry has a reportOrphan method, and no remaining parents, -// call entry.reportOrphan and return true iff it returns true. The -// reportOrphan function should return true to indicate entry.dispose() -// has been called, and the entry has been removed from any other caches -// (see index.js for the only current example). -function maybeReportOrphan(entry: AnyEntry) { - return entry.parents.size === 0 && - typeof entry.reportOrphan === "function" && - entry.reportOrphan() === true; -} - // Removes all children from this entry and returns an array of the // removed children. function forgetChildren(parent: AnyEntry) { - let children = reusableEmptyArray; - if (parent.childValues.size > 0) { - children = []; parent.childValues.forEach((_value, child) => { forgetChild(parent, child); - children.push(child); }); } // After we forget all our children, this.dirtyChildren must be empty // and therefore must have been reset to null. assert(parent.dirtyChildren === null); - - return children; } function forgetChild(parent: AnyEntry, child: AnyEntry) { diff --git a/src/index.ts b/src/index.ts index 465a0d5..85bc359 100644 --- a/src/index.ts +++ b/src/index.ts @@ -50,10 +50,6 @@ export type OptimisticWrapOptions = { // The maximum number of cache entries that should be retained before the // cache begins evicting the oldest ones. max?: number; - // If a wrapped function is "disposable," then its creator does not - // care about its return value, and it should be removed from the cache - // immediately when it no longer has any parents that depend on it. - disposable?: boolean; // The makeCacheKey function takes the same arguments that were passed to // the wrapper function and returns a single value that can be used as a key // in a Map to identify the cached result. @@ -77,19 +73,9 @@ export function wrap< entry => entry.dispose(), ); - const disposable = !! options.disposable; const makeCacheKey = options.makeCacheKey || defaultMakeCacheKey; function optimistic(): TResult { - if (disposable && ! parentEntrySlot.hasValue()) { - // If there's no current parent computation, and this wrapped - // function is disposable (meaning we don't care about entry.value, - // just dependency tracking), then we can short-cut everything else - // in this function, because entry.recompute() is going to recycle - // the entry object without recomputing anything, anyway. - return void 0 as any; - } - const key = makeCacheKey.apply(null, arguments as any); if (key === void 0) { return originalFunction.apply(null, arguments as any); @@ -104,9 +90,6 @@ export function wrap< entry = new Entry(originalFunction, args); cache.set(key, entry); entry.subscribe = options.subscribe; - if (disposable) { - entry.reportOrphan = () => cache.delete(key); - } } const value = entry.recompute(); @@ -125,10 +108,7 @@ export function wrap< caches.clear(); } - // If options.disposable is truthy, the caller of wrap is telling us - // they don't care about the result of entry.recompute(), so we should - // avoid returning the value, so it won't be accidentally used. - return disposable ? void 0 as any : value; + return value; } optimistic.dirty = function () { diff --git a/src/tests/api.ts b/src/tests/api.ts index ccc64a6..3376052 100644 --- a/src/tests/api.ts +++ b/src/tests/api.ts @@ -498,73 +498,6 @@ describe("optimism", function () { assert.strictEqual(child(), "parent"); }); - it("supports disposable wrapped functions", function () { - let dependCallCount = 0; - const depend = wrap(function (n?: number) { - return ++dependCallCount; - }, { - disposable: true - }); - - assert.strictEqual(typeof depend(), "undefined"); - assert.strictEqual(dependCallCount, 0); - - let parentCallCount = 0; - const parent = wrap(function () { - ++parentCallCount; - assert.strictEqual(typeof depend(1), "undefined"); - assert.strictEqual(typeof depend(2), "undefined"); - }); - - parent(); - assert.strictEqual(parentCallCount, 1); - assert.strictEqual(dependCallCount, 2); - - parent(); - assert.strictEqual(parentCallCount, 1); - assert.strictEqual(dependCallCount, 2); - - depend.dirty(1); - parent(); - assert.strictEqual(parentCallCount, 2); - assert.strictEqual(dependCallCount, 3); - - depend.dirty(2); - parent(); - assert.strictEqual(parentCallCount, 3); - assert.strictEqual(dependCallCount, 4); - - parent(); - assert.strictEqual(parentCallCount, 3); - assert.strictEqual(dependCallCount, 4); - - parent.dirty(); - parent(); - assert.strictEqual(parentCallCount, 4); - assert.strictEqual(dependCallCount, 4); - - depend.dirty(1); - depend(1); - // No change to dependCallCount because depend is called outside of - // any parent computation, and depend is disposable. - assert.strictEqual(dependCallCount, 4); - depend(2); - assert.strictEqual(dependCallCount, 4); - - depend.dirty(2); - depend(1); - // Again, no change because depend is disposable. - assert.strictEqual(dependCallCount, 4); - depend(2); - assert.strictEqual(dependCallCount, 4); - - parent(); - // Now, since both depend(1) and depend(2) are dirty, calling them in - // the context of the parent() computation results in two more - // increments of dependCallCount. - assert.strictEqual(dependCallCount, 6); - }); - it("is not confused by eviction during recomputation", function () { const fib: OptimisticWrapperFunction<[number], number> = wrap(function (n: number) { diff --git a/tsconfig.json b/tsconfig.json index cf23da9..3f0687f 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -11,6 +11,7 @@ "types": ["node", "mocha"], "strict": true, "noImplicitAny": true, + "noUnusedLocals": true, "esModuleInterop": true } } From 3e94cb2244ca1e56a33cbf6222c4ca3c6890a933 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 16 Sep 2019 12:43:56 -0400 Subject: [PATCH 2/2] Implement lightweight OptimisticDependencyFunction primitive. --- src/entry.ts | 26 ++++++++- src/index.ts | 30 ++++++++++ src/tests/deps.ts | 119 +++++++++++++++++++++++++++++++++++++++ src/tests/main.ts | 1 + src/tests/performance.ts | 19 ++++++- 5 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 src/tests/deps.ts diff --git a/src/entry.ts b/src/entry.ts index 6f8a053..a860290 100644 --- a/src/entry.ts +++ b/src/entry.ts @@ -1,7 +1,7 @@ import { parentEntrySlot } from "./context"; import { OptimisticWrapOptions } from "./index"; -const emptySetPool: Set[] = []; +const emptySetPool: Set[] = []; const POOL_TARGET_SIZE = 100; // Since this package might be used browsers, we should avoid using the @@ -89,6 +89,7 @@ export class Entry { this.dirty = true; this.value.length = 0; reportDirty(this); + forgetChildren(this); // We can go ahead and unsubscribe here, since any further dirty // notifications we receive will be redundant, and unsubscribing may // free up some resources, e.g. file watchers. @@ -115,6 +116,25 @@ export class Entry { forgetChild(parent, this); }); } + + private sets: Set> | null = null; + + public addToSet(entrySet: Set) { + entrySet.add(this); + if (! this.sets) { + this.sets = emptySetPool.pop() || new Set>(); + } + this.sets.add(entrySet); + } + + public removeFromSets() { + if (this.sets) { + this.sets.forEach(set => set.delete(this)); + this.sets.clear(); + emptySetPool.push(this.sets); + this.sets = null; + } + } } function rememberParent(child: AnyEntry) { @@ -256,6 +276,10 @@ function forgetChildren(parent: AnyEntry) { }); } + // Remove this parent Entry from any sets to which it was added by the + // addToSet method. + parent.removeFromSets(); + // After we forget all our children, this.dirtyChildren must be empty // and therefore must have been reset to null. assert(parent.dirtyChildren === null); diff --git a/src/index.ts b/src/index.ts index 85bc359..9c124f2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -46,6 +46,11 @@ export type OptimisticWrapperFunction< dirty: (...args: TArgs) => void; }; +type OptimisticDependencyFunction = + ((key: TKey) => void) & { + dirty: (key: TKey) => void; + }; + export type OptimisticWrapOptions = { // The maximum number of cache entries that should be retained before the // cache begins evicting the oldest ones. @@ -121,3 +126,28 @@ export function wrap< return optimistic as OptimisticWrapperFunction; } + +export function dep() { + const parentEntriesByKey = new Map>(); + + function depend(key: TKey) { + const parent = parentEntrySlot.getValue(); + if (parent) { + let parentEntrySet = parentEntriesByKey.get(key); + if (!parentEntrySet) { + parentEntriesByKey.set(key, parentEntrySet = new Set); + } + parent.addToSet(parentEntrySet); + } + } + + depend.dirty = function(key: TKey) { + const parentEntrySet = parentEntriesByKey.get(key); + if (parentEntrySet) { + parentEntrySet.forEach(entry => entry.setDirty()); + parentEntriesByKey.delete(key); + } + }; + + return depend as OptimisticDependencyFunction; +} diff --git a/src/tests/deps.ts b/src/tests/deps.ts new file mode 100644 index 0000000..9ea5fce --- /dev/null +++ b/src/tests/deps.ts @@ -0,0 +1,119 @@ +import * as assert from "assert"; +import { wrap, dep } from "../index"; + +describe("OptimisticDependencyFunction", () => { + it("can dirty OptimisticWrapperFunctions", () => { + const numberDep = dep(); + const stringDep = dep(); + let callCount = 0; + + const fn = wrap((n: number, s: string) => { + numberDep(n); + stringDep(s); + ++callCount; + return s.repeat(n); + }); + + assert.strictEqual(fn(0, "oyez"), ""); + assert.strictEqual(callCount, 1); + assert.strictEqual(fn(1, "oyez"), "oyez"); + assert.strictEqual(callCount, 2); + assert.strictEqual(fn(2, "oyez"), "oyezoyez"); + assert.strictEqual(callCount, 3); + + assert.strictEqual(fn(0, "oyez"), ""); + assert.strictEqual(fn(1, "oyez"), "oyez"); + assert.strictEqual(fn(2, "oyez"), "oyezoyez"); + assert.strictEqual(callCount, 3); + + numberDep.dirty(0); + assert.strictEqual(fn(0, "oyez"), ""); + assert.strictEqual(callCount, 4); + assert.strictEqual(fn(1, "oyez"), "oyez"); + assert.strictEqual(callCount, 4); + assert.strictEqual(fn(2, "oyez"), "oyezoyez"); + assert.strictEqual(callCount, 4); + + stringDep.dirty("mlem"); + assert.strictEqual(fn(0, "oyez"), ""); + assert.strictEqual(callCount, 4); + + stringDep.dirty("oyez"); + assert.strictEqual(fn(2, "oyez"), "oyezoyez"); + assert.strictEqual(callCount, 5); + assert.strictEqual(fn(1, "oyez"), "oyez"); + assert.strictEqual(callCount, 6); + assert.strictEqual(fn(0, "oyez"), ""); + assert.strictEqual(callCount, 7); + + assert.strictEqual(fn(0, "oyez"), ""); + assert.strictEqual(fn(1, "oyez"), "oyez"); + assert.strictEqual(fn(2, "oyez"), "oyezoyez"); + assert.strictEqual(callCount, 7); + }); + + it("should be forgotten when parent is recomputed", () => { + const d = dep(); + let callCount = 0; + let shouldDepend = true; + + const parent = wrap((id: string) => { + if (shouldDepend) d(id); + return ++callCount; + }); + + assert.strictEqual(parent("oyez"), 1); + assert.strictEqual(parent("oyez"), 1); + assert.strictEqual(parent("mlem"), 2); + assert.strictEqual(parent("mlem"), 2); + + d.dirty("mlem"); + assert.strictEqual(parent("oyez"), 1); + assert.strictEqual(parent("mlem"), 3); + + d.dirty("oyez"); + assert.strictEqual(parent("oyez"), 4); + assert.strictEqual(parent("mlem"), 3); + + parent.dirty("oyez"); + shouldDepend = false; + assert.strictEqual(parent("oyez"), 5); + assert.strictEqual(parent("mlem"), 3); + d.dirty("oyez"); + shouldDepend = true; + assert.strictEqual(parent("oyez"), 5); + assert.strictEqual(parent("mlem"), 3); + // This still has no effect because the previous call to parent("oyez") + // was cached. + d.dirty("oyez"); + assert.strictEqual(parent("oyez"), 5); + assert.strictEqual(parent("mlem"), 3); + parent.dirty("oyez"); + assert.strictEqual(parent("oyez"), 6); + assert.strictEqual(parent("mlem"), 3); + d.dirty("oyez"); + assert.strictEqual(parent("oyez"), 7); + assert.strictEqual(parent("mlem"), 3); + + parent.dirty("mlem"); + shouldDepend = false; + assert.strictEqual(parent("oyez"), 7); + assert.strictEqual(parent("mlem"), 8); + d.dirty("oyez"); + d.dirty("mlem"); + assert.strictEqual(parent("oyez"), 9); + assert.strictEqual(parent("mlem"), 8); + d.dirty("oyez"); + d.dirty("mlem"); + assert.strictEqual(parent("oyez"), 9); + assert.strictEqual(parent("mlem"), 8); + shouldDepend = true; + parent.dirty("mlem"); + assert.strictEqual(parent("oyez"), 9); + assert.strictEqual(parent("mlem"), 10); + d.dirty("oyez"); + d.dirty("mlem"); + assert.strictEqual(parent("oyez"), 9); + assert.strictEqual(parent("mlem"), 11); + }); +}); diff --git a/src/tests/main.ts b/src/tests/main.ts index 99b7981..241bcde 100644 --- a/src/tests/main.ts +++ b/src/tests/main.ts @@ -1,4 +1,5 @@ import "./api"; +import "./deps"; import "./cache"; import "./key-trie"; import "./context"; diff --git a/src/tests/performance.ts b/src/tests/performance.ts index 2ec96ac..609c6d6 100644 --- a/src/tests/performance.ts +++ b/src/tests/performance.ts @@ -1,5 +1,5 @@ import * as assert from "assert"; -import { wrap, KeyTrie } from "../index"; +import { wrap, dep, KeyTrie } from "../index"; describe("performance", function () { this.timeout(30000); @@ -17,6 +17,23 @@ describe("performance", function () { } }); + const keys: object[] = []; + for (let i = 0; i < 100000; ++i) { + keys.push({ i }); + } + + it("should be able to tolerate lots of deps", function () { + const d = dep(); + const parent = wrap((id: number) => { + keys.forEach(d); + return id; + }); + parent(1); + parent(2); + parent(3); + keys.forEach(key => d.dirty(key)); + }); + it("can speed up sorting with O(array.length) cache lookup", function () { let counter = 0; const trie = new KeyTrie(false);