diff --git a/.changeset/hip-seals-kiss.md b/.changeset/hip-seals-kiss.md deleted file mode 100644 index 9d0ea32e8..000000000 --- a/.changeset/hip-seals-kiss.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"mobx": patch ---- - -Fix regression from #4639 where isComputedProp returned false for lazy @computed properties before first read diff --git a/.changeset/lazy-computed-decorator.md b/.changeset/lazy-computed-decorator.md deleted file mode 100644 index 031f56a25..000000000 --- a/.changeset/lazy-computed-decorator.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"mobx": minor ---- - -feat(mobx): make the 2022.3 `@computed` decorator lazy. `ComputedValue` is now created on first read of the decorated getter rather than eagerly during instance construction, avoiding wasted allocations for getters that are never used. On a 50k-instance × 10-getter class with one read per instance this cuts construction heap by ~50% and construction time by ~25%; the steady-state read path is unchanged. Closes #4616. diff --git a/.changeset/npm-workspace-cli-resolution.md b/.changeset/npm-workspace-cli-resolution.md deleted file mode 100644 index f7b7ab570..000000000 --- a/.changeset/npm-workspace-cli-resolution.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"mobx-undecorate": patch ---- - -Fix CLI resolution of the bundled jscodeshift binary when dependencies are hoisted by npm workspaces. diff --git a/.changeset/observable-map-get-or-insert.md b/.changeset/observable-map-get-or-insert.md deleted file mode 100644 index d8e72c110..000000000 --- a/.changeset/observable-map-get-or-insert.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"mobx": patch ---- - -Add `getOrInsert` and `getOrInsertComputed` to `ObservableMap` for compatibility with ESNext `Map` typings. diff --git a/packages/mobx-undecorate/CHANGELOG.md b/packages/mobx-undecorate/CHANGELOG.md index f8c0d7d62..7119990b0 100644 --- a/packages/mobx-undecorate/CHANGELOG.md +++ b/packages/mobx-undecorate/CHANGELOG.md @@ -1,5 +1,11 @@ # mobx-undecorate +## 1.3.1 + +### Patch Changes + +- [`0e8fbd6947350e4318d3cf83b69ee58b4b839c25`](https://github.com/mobxjs/mobx/commit/0e8fbd6947350e4318d3cf83b69ee58b4b839c25) [#4646](https://github.com/mobxjs/mobx/pull/4646) Thanks [@kubk](https://github.com/kubk)! - Fix CLI resolution of the bundled jscodeshift binary when dependencies are hoisted by npm workspaces. + ## 1.3.0 ### Minor Changes diff --git a/packages/mobx-undecorate/package.json b/packages/mobx-undecorate/package.json index 4a546b14b..7d99517d2 100644 --- a/packages/mobx-undecorate/package.json +++ b/packages/mobx-undecorate/package.json @@ -1,6 +1,6 @@ { "name": "mobx-undecorate", - "version": "1.3.0", + "version": "1.3.1", "description": "Migrate MobX 4/5 to MobX 6", "bin": "cli.js", "repository": { diff --git a/packages/mobx/CHANGELOG.md b/packages/mobx/CHANGELOG.md index 5825d20f0..ee9e61343 100644 --- a/packages/mobx/CHANGELOG.md +++ b/packages/mobx/CHANGELOG.md @@ -1,5 +1,19 @@ # mobx +## 6.16.0 + +### Minor Changes + +- [`6b3fb0ee725c0521bbaf7ba901a30261472a0e71`](https://github.com/mobxjs/mobx/commit/6b3fb0ee725c0521bbaf7ba901a30261472a0e71) [#4639](https://github.com/mobxjs/mobx/pull/4639) Thanks [@ashishkr96](https://github.com/ashishkr96)! - feat(mobx): make the 2022.3 `@computed` decorator lazy. `ComputedValue` is now created on first read of the decorated getter rather than eagerly during instance construction, avoiding wasted allocations for getters that are never used. On a 50k-instance × 10-getter class with one read per instance this cuts construction heap by ~50% and construction time by ~25%; the steady-state read path is unchanged. Closes #4616. + +- [`f0c68749428fd4d3bba48e9685e44fd1ddbbee76`](https://github.com/mobxjs/mobx/commit/f0c68749428fd4d3bba48e9685e44fd1ddbbee76) [#4658](https://github.com/mobxjs/mobx/pull/4658) Thanks [@ashishkr96](https://github.com/ashishkr96)! - feat(mobx): make the 2022.3 `@observable accessor` decorator lazy. `ObservableValue` is now created on first read/write/observe of the decorated accessor rather than eagerly during instance construction, avoiding wasted allocations for fields that are never touched. On a 50k-instance × 10-field class with sparse access (1 of 10 fields read per instance), this cuts construction heap by ~82% and construction time by ~86% versus the eager path. Follow-up to #4639. + +### Patch Changes + +- [`7eb54418b16fb9b415c04c5b8e05779790dd74ed`](https://github.com/mobxjs/mobx/commit/7eb54418b16fb9b415c04c5b8e05779790dd74ed) [#4659](https://github.com/mobxjs/mobx/pull/4659) Thanks [@kubk](https://github.com/kubk)! - Fix regression from #4639 where isComputedProp returned false for lazy @computed properties before first read + +- [`c22b4b705447a4ccdce93473255f4beb170613f3`](https://github.com/mobxjs/mobx/commit/c22b4b705447a4ccdce93473255f4beb170613f3) [#4657](https://github.com/mobxjs/mobx/pull/4657) Thanks [@kubk](https://github.com/kubk)! - Add `getOrInsert` and `getOrInsertComputed` to `ObservableMap` for compatibility with ESNext `Map` typings. + ## 6.15.4 ### Patch Changes diff --git a/packages/mobx/__tests__/decorators_20223/stage3-decorators.ts b/packages/mobx/__tests__/decorators_20223/stage3-decorators.ts index 39dc348e8..9e75b77b9 100644 --- a/packages/mobx/__tests__/decorators_20223/stage3-decorators.ts +++ b/packages/mobx/__tests__/decorators_20223/stage3-decorators.ts @@ -1006,9 +1006,10 @@ test("multiple inheritance should work", () => { } } - const obsvKeys = [ - ...(mobx._getAdministration(new B()) as ObservableArrayAdministration).values_.keys() - ] + const adm = mobx._getAdministration(new B()) as any + // @observable accessor is lazy (#4616 follow-up), so unread fields live in + // `lazyObservableKeys_` until first read. Union with `values_` to see them all. + const obsvKeys = [...adm.values_.keys(), ...(adm.lazyObservableKeys_?.keys() ?? [])].sort() expect(obsvKeys).toEqual(["x", "y"]) }) @@ -1222,6 +1223,81 @@ test("4616 - observe on @computed before first read materialises it", () => { t.deepEqual(events, [8]) }) +test("4616 - @observable accessor should be lazy", () => { + class Wide { + @observable accessor unused: number = 1 + @observable accessor used: number = 2 + } + + const o = new Wide() + // Public API: both should report as observable props + t.equal(isObservableProp(o, "unused"), true) + t.equal(isObservableProp(o, "used"), true) + + // Internal check: ObservableValue is not yet allocated for either field + const adm: any = (o as any)[$mobx] + expect(adm.values_.has("unused")).toBe(false) + expect(adm.values_.has("used")).toBe(false) + expect(adm.lazyObservableKeys_.has("unused")).toBe(true) + expect(adm.lazyObservableKeys_.has("used")).toBe(true) + + // First access materialises the ObservableValue + t.equal(o.used, 2) + expect(adm.values_.has("used")).toBe(true) + expect(adm.lazyObservableKeys_.has("used")).toBe(false) + + // The unused field remains lazy + expect(adm.values_.has("unused")).toBe(false) + expect(adm.lazyObservableKeys_.has("unused")).toBe(true) +}) + +test("4616 - observe on @observable accessor before first read materialises it", () => { + class Counter { + @observable accessor count: number = 0 + } + + const o = new Counter() + const adm: any = (o as any)[$mobx] + expect(adm.values_.has("count")).toBe(false) + + const events: number[] = [] + observe(o, "count", ev => events.push((ev as any).newValue)) + // observe should have materialised the ObservableValue + expect(adm.values_.has("count")).toBe(true) + + o.count = 5 + o.count = 7 + t.deepEqual(events, [5, 7]) +}) + +test("4616 - set on @observable accessor before first read materialises it", () => { + class Counter { + @observable accessor count: number = 0 + } + + const o = new Counter() + const adm: any = (o as any)[$mobx] + expect(adm.values_.has("count")).toBe(false) + + o.count = 42 + expect(adm.values_.has("count")).toBe(true) + t.equal(o.count, 42) +}) + +test("4616 - autorun reacts to @observable accessor that is lazy on entry", () => { + class Counter { + @observable accessor count: number = 0 + } + + const o = new Counter() + const seen: number[] = [] + const dispose = autorun(() => seen.push(o.count)) + o.count = 1 + o.count = 2 + dispose() + t.deepEqual(seen, [0, 1, 2]) +}) + test(`decorated field can be inherited, but doesn't inherite the effect of decorator`, () => { class Store { @action diff --git a/packages/mobx/__tests__/perf/lazy-observable-decorator.ts b/packages/mobx/__tests__/perf/lazy-observable-decorator.ts new file mode 100644 index 000000000..2c2a86efa --- /dev/null +++ b/packages/mobx/__tests__/perf/lazy-observable-decorator.ts @@ -0,0 +1,236 @@ +// Benchmark for the lazy `@observable accessor` decorator (#4616 follow-up +// to #4639). +// +// Shape: many instances of wide stage-3-decorator classes with 10 +// `@observable accessor` fields, measured under sparse, full, and +// constructor-hydration patterns. This file intentionally does not call +// `makeObservable`; it measures only the 2022.3 decorator path affected by this +// PR. +// +// Run with `npm run perf-decorator` (requires a prior `npm run build`). + +/* eslint-disable @typescript-eslint/no-require-imports */ +import * as path from "path" +const distPath = path.resolve(__dirname, "..", "..", "..", "dist", "mobx.cjs.development.js") +const mobx = require(distPath) as { + observable: any +} +const { observable } = mobx + +const INSTANCES = 50_000 +const RUNS = 3 +const RE_READS = 5 + +const FIELDS = ["f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7", "f8", "f9"] as const +const READ_THREE = ["f0", "f1", "f2"] as const + +type Field = (typeof FIELDS)[number] +type WideCtor = new (seed: number) => any + +class DefaultsWide { + @observable accessor f0 = 0 + @observable accessor f1 = 1 + @observable accessor f2 = 2 + @observable accessor f3 = 3 + @observable accessor f4 = 4 + @observable accessor f5 = 5 + @observable accessor f6 = 6 + @observable accessor f7 = 7 + @observable accessor f8 = 8 + @observable accessor f9 = 9 +} + +class AssignedPartialWide { + @observable accessor f0 = 0 + @observable accessor f1 = 1 + @observable accessor f2 = 2 + @observable accessor f3 = 3 + @observable accessor f4 = 4 + @observable accessor f5 = 5 + @observable accessor f6 = 6 + @observable accessor f7 = 7 + @observable accessor f8 = 8 + @observable accessor f9 = 9 + + constructor(seed: number) { + this.f0 = seed + this.f1 = seed + 1 + this.f2 = seed + 2 + } +} + +class AssignedAllWide { + @observable accessor f0 = 0 + @observable accessor f1 = 1 + @observable accessor f2 = 2 + @observable accessor f3 = 3 + @observable accessor f4 = 4 + @observable accessor f5 = 5 + @observable accessor f6 = 6 + @observable accessor f7 = 7 + @observable accessor f8 = 8 + @observable accessor f9 = 9 + + constructor(seed: number) { + this.f0 = seed + this.f1 = seed + 1 + this.f2 = seed + 2 + this.f3 = seed + 3 + this.f4 = seed + 4 + this.f5 = seed + 5 + this.f6 = seed + 6 + this.f7 = seed + 7 + this.f8 = seed + 8 + this.f9 = seed + 9 + } +} + +type Sample = { + constructHeapMB: number + constructMs: number + firstReadMs: number + reReadMs: number + writeMs?: number +} + +function forceGc() { + if (typeof global.gc === "function") global.gc() +} + +function heapMB(): number { + return process.memoryUsage().heapUsed / (1024 * 1024) +} + +function readSparse(instances: any[]): number { + let sink = 0 + for (let i = 0; i < INSTANCES; i++) sink += instances[i][FIELDS[i % FIELDS.length]] + return sink +} + +function readFields(instances: any[], fields: readonly Field[]): number { + let sink = 0 + for (let i = 0; i < INSTANCES; i++) { + const inst = instances[i] + for (let f = 0; f < fields.length; f++) sink += inst[fields[f]] + } + return sink +} + +function writeField(instances: any[], field: Field): number { + let sink = 0 + for (let i = 0; i < INSTANCES; i++) { + instances[i][field] = i + sink += instances[i][field] + } + return sink +} + +function bench(Ctor: WideCtor, read: (instances: any[]) => number, writeFieldName?: Field): Sample { + forceGc() + const heapBefore = heapMB() + + const t0 = performance.now() + const instances: any[] = new Array(INSTANCES) + for (let i = 0; i < INSTANCES; i++) instances[i] = new Ctor(i) + const t1 = performance.now() + + forceGc() + const heapAfter = heapMB() + + const t2 = performance.now() + let sink = read(instances) + const t3 = performance.now() + + const t4 = performance.now() + for (let r = 0; r < RE_READS; r++) sink += read(instances) + const t5 = performance.now() + + let writeMs: number | undefined + if (writeFieldName) { + const t6 = performance.now() + sink += writeField(instances, writeFieldName) + const t7 = performance.now() + writeMs = t7 - t6 + } + + if (sink === Number.NEGATIVE_INFINITY) console.log("unreachable") + + return { + constructHeapMB: heapAfter - heapBefore, + constructMs: t1 - t0, + firstReadMs: t3 - t2, + reReadMs: t5 - t4, + writeMs + } +} + +function fmt(n: number | undefined, digits = 1): string { + return n === undefined ? " " : n.toFixed(digits).padStart(8) +} + +function runScenario( + label: string, + Ctor: WideCtor, + read: (instances: any[]) => number, + writeFieldName?: Field +) { + bench(Ctor, read, writeFieldName) // warmup + const samples: Sample[] = [] + for (let r = 0; r < RUNS; r++) samples.push(bench(Ctor, read, writeFieldName)) + + console.log(`\n${label}`) + console.log("run | construct heap MB | construct ms | first-read ms | re-read ms | write ms") + console.log("----+-------------------+--------------+---------------+------------+---------") + samples.forEach((s, i) => { + console.log( + ` ${i + 1} | ${fmt(s.constructHeapMB)} | ${fmt(s.constructMs)} | ${fmt( + s.firstReadMs + )} | ${fmt(s.reReadMs)} | ${fmt(s.writeMs)}` + ) + }) + const avg = (pick: (s: Sample) => number | undefined) => { + const values = samples.map(pick).filter((value): value is number => value !== undefined) + return values.length ? values.reduce((a, v) => a + v, 0) / values.length : undefined + } + console.log( + `avg | ${fmt(avg(s => s.constructHeapMB))} | ${fmt( + avg(s => s.constructMs) + )} | ${fmt(avg(s => s.firstReadMs))} | ${fmt( + avg(s => s.reReadMs) + )} | ${fmt(avg(s => s.writeMs))}` + ) +} + +function main() { + console.log(`\nLazy @observable benchmark — ${INSTANCES} instances, ${RE_READS} re-reads.`) + console.log(`Node ${process.version}, ${RUNS} timed runs after 1 warmup per scenario.`) + console.log(`Stage-3 decorators only; no makeObservable/makeAutoObservable.`) + + runScenario("DEFAULTS: sparse 1 of 10 fields read", DefaultsWide, readSparse) + runScenario( + "DEFAULTS: 3 of 10 fields read, then write unread f9", + DefaultsWide, + instances => readFields(instances, READ_THREE), + "f9" + ) + runScenario( + "HYDRATED PARTIAL: constructor assigns 3 of 10, read those 3, then write unread f9", + AssignedPartialWide, + instances => readFields(instances, READ_THREE), + "f9" + ) + runScenario( + "HYDRATED FULL: constructor assigns all 10, read 3, then write f9", + AssignedAllWide, + instances => readFields(instances, READ_THREE), + "f9" + ) + runScenario( + "HYDRATED FULL: constructor assigns all 10, read all 10, then write f9", + AssignedAllWide, + instances => readFields(instances, FIELDS), + "f9" + ) +} + +main() diff --git a/packages/mobx/__tests__/perf/tsconfig.decorator.json b/packages/mobx/__tests__/perf/tsconfig.decorator.json index aaf3616ec..34e10d676 100644 --- a/packages/mobx/__tests__/perf/tsconfig.decorator.json +++ b/packages/mobx/__tests__/perf/tsconfig.decorator.json @@ -11,5 +11,5 @@ "outDir": "./compiled", "rootDir": "./" }, - "include": ["./lazy-computed-decorator.ts"] + "include": ["./lazy-computed-decorator.ts", "./lazy-observable-decorator.ts"] } diff --git a/packages/mobx/package.json b/packages/mobx/package.json index 749cc03b1..9f3455de7 100644 --- a/packages/mobx/package.json +++ b/packages/mobx/package.json @@ -1,6 +1,6 @@ { "name": "mobx", - "version": "6.15.4", + "version": "6.16.0", "description": "Simple, scalable state management.", "source": "src/mobx.ts", "main": "dist/index.js", @@ -68,7 +68,7 @@ "perf": "scripts/perf.sh", "perf-legacy": "node --expose-gc ./__tests__/perf/index.js legacy", "perf-proxy": "node --expose-gc ./__tests__/perf/index.js proxy", - "perf-decorator": "tsc -p ./__tests__/perf/tsconfig.decorator.json && node --expose-gc ./__tests__/perf/compiled/lazy-computed-decorator.js", + "perf-decorator": "tsc -p ./__tests__/perf/tsconfig.decorator.json && node --expose-gc ./__tests__/perf/compiled/lazy-computed-decorator.js && node --expose-gc ./__tests__/perf/compiled/lazy-observable-decorator.js", "test:performance": "npm run perf -- proxy && npm run perf -- legacy && npm run perf-decorator", "test:mixed-versions": "npm test -- --testRegex mixed-versions", "test:types": "tsc --noEmit", diff --git a/packages/mobx/src/api/isobservable.ts b/packages/mobx/src/api/isobservable.ts index 46ba1b005..c4ec35cf7 100644 --- a/packages/mobx/src/api/isobservable.ts +++ b/packages/mobx/src/api/isobservable.ts @@ -22,7 +22,11 @@ function _isObservable(value, property?: PropertyKey): boolean { } if (isObservableObject(value)) { const adm = value[$mobx] - return adm.values_.has(property) || !!adm.lazyComputedKeys_?.has(property) + return ( + adm.values_.has(property) || + !!adm.lazyComputedKeys_?.has(property) || + !!adm.lazyObservableKeys_?.has(property) + ) } return false } diff --git a/packages/mobx/src/types/observableannotation.ts b/packages/mobx/src/types/observableannotation.ts index 69dee5775..b7ec2c1bf 100644 --- a/packages/mobx/src/types/observableannotation.ts +++ b/packages/mobx/src/types/observableannotation.ts @@ -64,51 +64,45 @@ function decorate_20223_( const ann = this const { kind, name } = context - // The laziness here is not ideal... It's a workaround to how 2022.3 Decorators are implemented: - // `addInitializer` callbacks are executed _before_ any accessors are defined (instead of the ideal-for-us right after each). - // This means that, if we were to do our stuff in an `addInitializer`, we'd attempt to read a private slot - // before it has been initialized. The runtime doesn't like that and throws a `Cannot read private member - // from an object whose class did not declare it` error. - // TODO: it seems that this will not be required anymore in the final version of the spec - // See TODO: link - const initializedObjects = new WeakSet() + if (kind !== "accessor") { + return + } - function initializeObservable(target, value) { + // Defer ObservableValue construction until first access. The factory is + // materialised by ObservableObjectAdministration on demand, so unused + // fields on wide classes never pay the per-instance allocation cost. + function registerLazy(target: any, value: any): ObservableObjectAdministration { const adm: ObservableObjectAdministration = asObservableObject(target)[$mobx] - const observable = new ObservableValue( - value, - ann.options_?.enhancer ?? deepEnhancer, - __DEV__ ? `${adm.name_}.${name.toString()}` : `ObservableObject.${name.toString()}`, - false + ;(adm.lazyObservableKeys_ ??= new Map()).set( + name, + () => + new ObservableValue( + value, + ann.options_?.enhancer ?? deepEnhancer, + __DEV__ + ? `${adm.name_}.${name.toString()}` + : `ObservableObject.${name.toString()}`, + false + ) ) - adm.values_.set(name, observable) - initializedObjects.add(target) + return adm } - if (kind == "accessor") { - return { - get() { - if (!initializedObjects.has(this)) { - initializeObservable(this, desc.get.call(this)) - } - return this[$mobx].getObservablePropValue_(name) - }, - set(value) { - if (!initializedObjects.has(this)) { - initializeObservable(this, value) - } - return this[$mobx].setObservablePropValue_(name, value) - }, - init(value) { - if (!initializedObjects.has(this)) { - initializeObservable(this, value) - } - return value - } + return { + get() { + const adm: ObservableObjectAdministration = + this[$mobx] ?? registerLazy(this, desc.get.call(this)) + return adm.getObservablePropValue_(name) + }, + set(value) { + const adm: ObservableObjectAdministration = this[$mobx] ?? registerLazy(this, value) + return adm.setObservablePropValue_(name, value) + }, + init(value) { + registerLazy(this, value) + return value } } - - return } function assertObservableDescriptor( diff --git a/packages/mobx/src/types/observableobject.ts b/packages/mobx/src/types/observableobject.ts index 810dffd0a..6e7e60027 100644 --- a/packages/mobx/src/types/observableobject.ts +++ b/packages/mobx/src/types/observableobject.ts @@ -99,6 +99,7 @@ export class ObservableObjectAdministration appliedAnnotations_?: object private pendingKeys_: undefined | Map> lazyComputedKeys_: undefined | Map ComputedValue> + lazyObservableKeys_: undefined | Map ObservableValue> constructor( public target_: any, @@ -120,8 +121,11 @@ export class ObservableObjectAdministration } getObservablePropValue_(key: PropertyKey): any { - // Hot path: single map lookup. Lazy computeds (rare) take the materialise branch. - const observable = this.values_.get(key) ?? this.materializeLazyComputed_(key) + // Hot path: single map lookup. Lazy entries (rare) take the materialise branch. + const observable = + this.values_.get(key) ?? + this.materializeLazyComputed_(key) ?? + this.materializeLazyObservable_(key) return observable!.get() } @@ -131,13 +135,33 @@ export class ObservableObjectAdministration return undefined } this.lazyComputedKeys_!.delete(key) + if (this.lazyComputedKeys_!.size === 0) { + this.lazyComputedKeys_ = undefined + } const computed = factory() this.values_.set(key, computed) return computed } + materializeLazyObservable_(key: PropertyKey): ObservableValue | undefined { + const factory = this.lazyObservableKeys_?.get(key) + if (!factory) { + return undefined + } + this.lazyObservableKeys_!.delete(key) + if (this.lazyObservableKeys_!.size === 0) { + this.lazyObservableKeys_ = undefined + } + const observable = factory() + this.values_.set(key, observable) + return observable + } + setObservablePropValue_(key: PropertyKey, newValue): boolean | null { - const observable = this.values_.get(key) ?? this.materializeLazyComputed_(key) + const observable = + this.values_.get(key) ?? + this.materializeLazyComputed_(key) ?? + this.materializeLazyObservable_(key) if (observable instanceof ComputedValue) { observable.set(newValue) return true diff --git a/packages/mobx/src/types/type-utils.ts b/packages/mobx/src/types/type-utils.ts index e9839908c..c48ddf26d 100644 --- a/packages/mobx/src/types/type-utils.ts +++ b/packages/mobx/src/types/type-utils.ts @@ -48,7 +48,10 @@ export function getAtom(thing: any, property?: PropertyKey): IDepTreeNode { return die(26) } const adm = (thing as any)[$mobx] - const observable = adm.values_.get(property) ?? adm.materializeLazyComputed_(property) + const observable = + adm.values_.get(property) ?? + adm.materializeLazyComputed_(property) ?? + adm.materializeLazyObservable_(property) if (!observable) { die(27, property, getDebugName(thing)) }