[pull] main from mobxjs:main#54
Merged
Merged
Conversation
* feat(mobx): make 2022.3 @observable decorator lazy Mirrors the lazy `@computed` treatment from #4639 for `@observable accessor`: defers `ObservableValue` construction to the first read/write/observe of the decorated accessor instead of allocating one per field at instance construction. Closes the wide-class / sparse-access waste described in #4616 for plain observable fields (the original issue covered both decorators). - `decorate_20223_` in `observableannotation.ts` registers a factory closure on `adm.lazyObservableKeys_` instead of building the `ObservableValue` and inserting it into `values_` from the accessor's `init` callback. - `ObservableObjectAdministration.materializeLazyObservable_(key)` lazily instantiates the `ObservableValue` and inserts it into `values_` on demand. It is called from `getObservablePropValue_` / `setObservablePropValue_` (so the existing accessor get/set Just Works) and from `getAtom` (so `observe(o, key, ...)` materialises before reading). - `isObservableProp` checks `lazyObservableKeys_` in addition to `values_` / `lazyComputedKeys_` so a decorated observable continues to report as observable before its first read. - Hot path is unchanged: `values_.get(key)` is the single lookup once the field has been read for the first time; the materialise branch only runs the first time and for the rare pre-read `getAtom` / `observe` callers. - The `initializedObjects` WeakSet workaround is gone — `init` now always registers the lazy factory, and `get` / `set` materialise via the admin. Regression tests in `stage3-decorators.ts`: - `4616 - @observable accessor should be lazy` — asserts `ObservableValue` is not created until first read while `isObservableProp` still returns `true`, and an untouched field stays in `lazyObservableKeys_`. - `4616 - observe on @observable accessor before first read materialises it` - `4616 - set on @observable accessor before first read materialises it` - `4616 - autorun reacts to @observable accessor that is lazy on entry` The existing `multiple inheritance should work` test asserted on `values_.keys()` directly; updated to union with `lazyObservableKeys_` since unread accessors now live there. * test(mobx): add @observable decorator perf benchmark Standalone perf benchmark for the lazy `@observable accessor` change. Mirrors `lazy-computed-decorator.ts`: 50k instances × 10 fields with two read patterns: - SPARSE: 1 of 10 fields read per instance (best case for lazy) - FULL: all 10 fields read per instance (worst case for lazy) Compiled into `__tests__/perf/compiled/` alongside the @computed benchmark via the existing `tsconfig.decorator.json`. Build with `npm run build` and run with `npm run perf-decorator` (or directly `node --expose-gc __tests__/perf/compiled/lazy-observable-decorator.js`). * test(mobx): clean up @observable perf benchmark framing Carry-over from the exploration version of this file: - Header still described it as exploration estimating an "upper bound" under an eager decorator. The lazy change ships in this PR, so the file is now a regression benchmark for the shipped impl, not an estimate. - A `Narrow` class (1 field) was being benchmarked but its output was never referenced by the summary — dead noise. - The summary block printed "construct heap → unchanged" between independent SPARSE / FULL runs, overstating precision. - "first-read 1× / 10×" labels in the summary were ambiguous (could read as repetitions rather than fields-per-instance). Rewrites the header to match `lazy-computed-decorator.ts`'s framing, collapses the two near-identical bench bodies into one parameterised `bench(readsPerInstance)`, drops the unused `Narrow` class, drops the misleading summary block, and adds a note that the file only measures the currently built mobx — comparing against eager requires checking out a pre-#4658 commit and rebuilding. * fix lazy observable perf --------- Co-authored-by: Gorbachev Egor <7gorbachevm@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )