Skip to content

Commit

Permalink
Hydrate snapshotted views lazily to avoid excessive root boot costs
Browse files Browse the repository at this point in the history
[no-changelog-required]
  • Loading branch information
airhorns committed Jun 20, 2024
1 parent 0986578 commit 40b2cc9
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 29 deletions.
28 changes: 24 additions & 4 deletions spec/class-model-snapshotted-views.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { observable } from "mobx";
import { observable, runInAction } from "mobx";
import { ClassModel, action, snapshottedView, getSnapshot, register, types, onPatch } from "../src";
import { Apple } from "./fixtures/FruitAisle";
import { create } from "./helpers";
Expand Down Expand Up @@ -85,7 +85,9 @@ describe("class model snapshotted views", () => {
const instance = MyViewExample.create({ key: "1", name: "Test" });
onPatch(instance, fn);

observableArray.push("a");
runInAction(() => {
observableArray.push("a");
});
expect(fn).toMatchSnapshot();
});

Expand Down Expand Up @@ -141,8 +143,7 @@ describe("class model snapshotted views", () => {
@register
class HydrateExample extends ClassModel({ url: types.string }) {
@snapshottedView<URL>({
createReadOnly(value, snapshot, node) {
expect(snapshot).toBeDefined();
createReadOnly(value, node) {
expect(node).toBeDefined();
return value ? new URL(value) : undefined;
},
Expand Down Expand Up @@ -178,6 +179,25 @@ describe("class model snapshotted views", () => {
} as any);
expect(instance.withoutParams).toEqual(new URL("https://gadget.dev/blog/feature/extra"));
});

test("hydrators aren't called eagerly on readonly instances in case they are expensive", () => {
const fn = jest.fn().mockReturnValue("whatever");
@register
class HydrateExampleSpy extends ClassModel({}) {
@snapshottedView<URL>({
createReadOnly: fn,
})
get someView() {
return "view value";
}
}

const instance = HydrateExampleSpy.createReadOnly({ someView: "snapshot value" });
expect(fn).not.toHaveBeenCalled();

expect(instance.someView).toEqual("whatever");
expect(fn).toHaveBeenCalledTimes(1);
});
});

describe("references", () => {
Expand Down
4 changes: 2 additions & 2 deletions src/class-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type ActionMetadata = {
/** Options that configure a snapshotted view */
export interface SnapshottedViewOptions<V, T extends IAnyClassModelType> {
/** A function for converting a stored value in the snapshot back to the rich type for the view to return */
createReadOnly?: (value: V | undefined, snapshot: T["InputType"], node: Instance<T>) => V | undefined;
createReadOnly?: (value: V | undefined, node: Instance<T>) => V | undefined;

/** A function for converting the view value to a snapshot value */
createSnapshot?: (value: V) => any;
Expand Down Expand Up @@ -195,7 +195,7 @@ export function register<Instance, Klass extends { new (...args: any[]): Instanc
if (descriptor.get) {
Object.defineProperty(klass.prototype, property, {
...descriptor,
get: fastGetters.buildGetter(property, descriptor),
get: fastGetters.buildViewGetter(metadata, descriptor),
});
}

Expand Down
76 changes: 59 additions & 17 deletions src/fast-getter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { PropertyMetadata, ViewMetadata } from "./class-model";
import { snapshotProcessor } from "mobx-state-tree/dist/internal";
import type { PropertyMetadata, SnapshottedViewMetadata, ViewMetadata } from "./class-model";
import { getPropertyDescriptor } from "./class-model";
import { RegistrationError } from "./errors";
import { $notYetMemoized, $readOnly } from "./symbols";
Expand Down Expand Up @@ -28,6 +29,10 @@ export class FastGetBuilder {
return `mqt/${property}-memo`;
}

snapshottedViewInputSymbolName(property: string) {
return `mqt/${property}-svi-memo`;
}

outerClosureStatements(className: string) {
return this.memoizableProperties
.map(
Expand All @@ -39,30 +44,67 @@ export class FastGetBuilder {
.join("\n");
}

buildGetter(property: string, descriptor: PropertyDescriptor) {
buildViewGetter(metadata: ViewMetadata | SnapshottedViewMetadata, descriptor: PropertyDescriptor) {
const property = metadata.property;
const $memo = Symbol.for(this.memoSymbolName(property));
const source = `
(
function build({ $readOnly, $memo, $notYetMemoized, getValue }) {
return function get${property}(model, imports) {
if (!this[$readOnly]) return getValue.call(this);
let value = this[$memo];
if (value !== $notYetMemoized) {

let source;
let args;

if (metadata.type === "snapshotted-view" && metadata.options.createReadOnly) {
const $snapshotValue = Symbol.for(this.snapshottedViewInputSymbolName(property));

// this snapshotted view has a hydrator, so we need a special view function for readonly instances that lazily hydrates the snapshotted value
source = `
(
function build({ $readOnly, $memo, $notYetMemoized, $snapshotValue, getValue, hydrate }) {
return function get${property}(model, imports) {
if (!this[$readOnly]) return getValue.call(this);
let value = this[$memo];
if (value !== $notYetMemoized) {
return value;
}
const dehydratedValue = this[$snapshotValue];
if (typeof dehydratedValue !== "undefined") {
value = hydrate(dehydratedValue, this);
} else {
value = getValue.call(this);
}
this[$memo] = value;
return value;
}
}
)
//# sourceURL=mqt-eval/dynamic/${this.klass.name}-${property}-get.js
`;
args = { $readOnly, $memo, $snapshotValue, $notYetMemoized, hydrate: metadata.options.createReadOnly, getValue: descriptor.get };
} else {
source = `
(
function build({ $readOnly, $memo, $notYetMemoized, getValue }) {
return function get${property}(model, imports) {
if (!this[$readOnly]) return getValue.call(this);
let value = this[$memo];
if (value !== $notYetMemoized) {
return value;
}
value = getValue.call(this);
this[$memo] = value;
return value;
value = getValue.call(this);
this[$memo] = value;
return value;
}
}
}
)
//# sourceURL=mqt-eval/dynamic/${this.klass.name}-${property}-get.js
`;
)
//# sourceURL=mqt-eval/dynamic/${this.klass.name}-${property}-get.js
`;
args = { $readOnly, $memo, $notYetMemoized, getValue: descriptor.get };
}

try {
const builder = eval(source);
return builder({ $readOnly, $memo, $notYetMemoized, getValue: descriptor.get });
return builder(args);
} catch (error) {
console.error(`Error building getter for ${this.klass.name}#${property}`);
console.error(`Compiled source:\n${source}`);
Expand Down
15 changes: 9 additions & 6 deletions src/fast-instantiator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,21 +332,24 @@ export class InstantiatorBuilder<T extends IClassModelType<Record<string, IAnyTy
}`;
}

private assignSnapshottedViewExpression(snapshottedView: SnapshottedViewMetadata, index: number) {
private assignSnapshottedViewExpression(snapshottedView: SnapshottedViewMetadata) {
const varName = `view${snapshottedView.property}`;

let valueExpression = `snapshot?.["${snapshottedView.property}"]`;
let destinationProp;
if (snapshottedView.options.createReadOnly) {
const alias = this.alias(`snapshottedViews[${index}].options.createReadOnly`);
valueExpression = `${alias}(${valueExpression}, snapshot, this)`;
// we're using a hydrator, so we don't store it right at the memo, and instead stash it where we'll lazily hydrate it in the getter
destinationProp = this.alias(`Symbol.for("${this.getters.snapshottedViewInputSymbolName(snapshottedView.property)}")`);
} else {
// we're not using a hydrator, so we can stash the snapshotted value right into the memoized spot
destinationProp = this.alias(`Symbol.for("${this.getters.memoSymbolName(snapshottedView.property)}")`);
}
const memoSymbolAlias = this.alias(`Symbol.for("${this.getters.memoSymbolName(snapshottedView.property)}")`);

const valueExpression = `snapshot?.["${snapshottedView.property}"]`;
return `
// setup snapshotted view for ${snapshottedView.property}
const ${varName} = ${valueExpression};
if (typeof ${varName} != "undefined") {
this[${memoSymbolAlias}] = ${varName};
this[${destinationProp}] = ${varName};
}
`;
}
Expand Down

0 comments on commit 40b2cc9

Please sign in to comment.