Shrink ComputedValue using a bitfield

mobxjs · Jun 3, 2024 · ff1d3dd · ff1d3dd
1 parent 44a5fe0
commit ff1d3dd
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 41 deletions.
diff --git a/.changeset/dirty-turtles-marry.md b/.changeset/dirty-turtles-marry.md
@@ -0,0 +1,5 @@
+---
+"mobx": patch
+---
+
+Shrink ComputedValue using a bitfield
diff --git a/packages/mobx/__tests__/v5/base/errorhandling.js b/packages/mobx/__tests__/v5/base/errorhandling.js
@@ -4,7 +4,7 @@ const utils = require("../../v5/utils/test-utils")
 
 const { observable, computed, $mobx, autorun } = mobx
 
-const voidObserver = function () { }
+const voidObserver = function () {}
 
 function checkGlobalState() {
     const gs = mobx._getGlobalState()
@@ -148,7 +148,7 @@ test("deny state changes in views", function () {
 
     m.reaction(
         () => z.get(),
-        () => { }
+        () => {}
     )
     expect(
         utils.grabConsole(() => {
@@ -194,7 +194,7 @@ test("deny array change in view", function (done) {
     }).not.toThrow()
     m.reaction(
         () => z.length,
-        () => { }
+        () => {}
     )
 
     expect(
@@ -483,7 +483,7 @@ test("peeking inside erroring computed value doesn't bork (global) state", () =>
         b.get()
     }).toThrowError(/chocolademelk/)
 
-    expect(a.isPendingUnobservation_).toBe(false)
+    expect(a.isPendingUnobservation).toBe(false)
     expect(a.observers_.size).toBe(0)
     expect(a.diffValue_).toBe(0)
     expect(a.lowestObserverState_).toBe(-1)
@@ -493,15 +493,15 @@ test("peeking inside erroring computed value doesn't bork (global) state", () =>
     expect(b.dependenciesState_).toBe(-1) // NOT_TRACKING
     expect(b.observing_.length).toBe(0)
     expect(b.newObserving_).toBe(null)
-    expect(b.isPendingUnobservation_).toBe(false)
+    expect(b.isPendingUnobservation).toBe(false)
     expect(b.observers_.size).toBe(0)
     expect(b.diffValue_).toBe(0)
     expect(b.lowestObserverState_).toBe(0)
     expect(b.unboundDepsCount_).toBe(0)
     expect(() => {
         b.get()
     }).toThrowError(/chocolademelk/)
-    expect(b.isComputing_).toBe(false)
+    expect(b.isComputing).toBe(false)
 
     checkGlobalState()
 })
@@ -521,7 +521,7 @@ describe("peeking inside autorun doesn't bork (global) state", () => {
     expect(r).toBe(1)
 
     test("it should update correctly initially", () => {
-        expect(a.isPendingUnobservation_).toBe(false)
+        expect(a.isPendingUnobservation).toBe(false)
         expect(a.observers_.size).toBe(1)
         expect(a.diffValue_).toBe(0)
         expect(a.lowestObserverState_).toBe(-1)
@@ -531,13 +531,13 @@ describe("peeking inside autorun doesn't bork (global) state", () => {
         expect(b.dependenciesState_).toBe(0)
         expect(b.observing_.length).toBe(1)
         expect(b.newObserving_).toBe(null)
-        expect(b.isPendingUnobservation_).toBe(false)
+        expect(b.isPendingUnobservation).toBe(false)
         expect(b.observers_.size).toBe(1)
         expect(b.diffValue_).toBe(0)
         expect(b.lowestObserverState_).toBe(0)
         expect(b.unboundDepsCount_).toBe(1) // value is always the last bound amount of observers
         expect(b.value_).toBe(1)
-        expect(b.isComputing_).toBe(false)
+        expect(b.isComputing).toBe(false)
 
         expect(c.dependenciesState_).toBe(0)
         expect(c.observing_.length).toBe(1)
@@ -558,7 +558,7 @@ describe("peeking inside autorun doesn't bork (global) state", () => {
         }, /chocolademelk/)
         expect(r).toBe(2)
 
-        expect(a.isPendingUnobservation_).toBe(false)
+        expect(a.isPendingUnobservation).toBe(false)
         expect(a.observers_.size).toBe(1)
         expect(a.diffValue_).toBe(0)
         expect(a.lowestObserverState_).toBe(0)
@@ -568,12 +568,12 @@ describe("peeking inside autorun doesn't bork (global) state", () => {
         expect(b.dependenciesState_).toBe(0) // up to date (for what it's worth)
         expect(b.observing_.length).toBe(1)
         expect(b.newObserving_).toBe(null)
-        expect(b.isPendingUnobservation_).toBe(false)
+        expect(b.isPendingUnobservation).toBe(false)
         expect(b.observers_.size).toBe(1)
         expect(b.diffValue_).toBe(0)
         expect(b.lowestObserverState_).toBe(0)
         expect(b.unboundDepsCount_).toBe(1)
-        expect(b.isComputing_).toBe(false)
+        expect(b.isComputing).toBe(false)
         expect(() => b.get()).toThrowError(/chocolademelk/)
 
         expect(c.dependenciesState_).toBe(0)
@@ -594,7 +594,7 @@ describe("peeking inside autorun doesn't bork (global) state", () => {
         a.set(3)
         expect(r).toBe(3)
 
-        expect(a.isPendingUnobservation_).toBe(false)
+        expect(a.isPendingUnobservation).toBe(false)
         expect(a.observers_.size).toBe(1)
         expect(a.diffValue_).toBe(0)
         expect(a.lowestObserverState_).toBe(0)
@@ -604,13 +604,13 @@ describe("peeking inside autorun doesn't bork (global) state", () => {
         expect(b.dependenciesState_).toBe(0) // up to date
         expect(b.observing_.length).toBe(1)
         expect(b.newObserving_).toBe(null)
-        expect(b.isPendingUnobservation_).toBe(false)
+        expect(b.isPendingUnobservation).toBe(false)
         expect(b.observers_.size).toBe(1)
         expect(b.diffValue_).toBe(0)
         expect(b.lowestObserverState_).toBe(0)
         expect(b.unboundDepsCount_).toBe(1)
         expect(b.value_).toBe(3)
-        expect(b.isComputing_).toBe(false)
+        expect(b.isComputing).toBe(false)
 
         expect(c.dependenciesState_).toBe(0)
         expect(c.observing_.length).toBe(1)
@@ -628,7 +628,7 @@ describe("peeking inside autorun doesn't bork (global) state", () => {
     test("it should clean up correctly", () => {
         d()
 
-        expect(a.isPendingUnobservation_).toBe(false)
+        expect(a.isPendingUnobservation).toBe(false)
         expect(a.observers_.size).toBe(0)
         expect(a.diffValue_).toBe(0)
         expect(a.lowestObserverState_).toBe(0)
@@ -638,13 +638,13 @@ describe("peeking inside autorun doesn't bork (global) state", () => {
         expect(b.dependenciesState_).toBe(-1) // not tracking
         expect(b.observing_.length).toBe(0)
         expect(b.newObserving_).toBe(null)
-        expect(b.isPendingUnobservation_).toBe(false)
+        expect(b.isPendingUnobservation).toBe(false)
         expect(b.observers_.size).toBe(0)
         expect(b.diffValue_).toBe(0)
         expect(b.lowestObserverState_).toBe(0)
         expect(b.unboundDepsCount_).toBe(1)
         expect(b.value_).not.toBe(3)
-        expect(b.isComputing_).toBe(false)
+        expect(b.isComputing).toBe(false)
 
         expect(c.dependenciesState_).toBe(-1)
         expect(c.observing_.length).toBe(0)
@@ -866,4 +866,4 @@ describe("es5 compat warnings", () => {
     })
 })
 
-test("should throw when adding properties in ES5 compat mode", () => { })
+test("should throw when adding properties in ES5 compat mode", () => {})
diff --git a/packages/mobx/src/core/atom.ts b/packages/mobx/src/core/atom.ts
@@ -22,8 +22,8 @@ export interface IAtom extends IObservable {
 }
 
 export class Atom implements IAtom {
-    isPendingUnobservation_ = false // for effective unobserving. BaseAtom has true, for extra optimization, so its onBecomeUnobserved never gets called, because it's not needed
-    isBeingObserved_ = false
+    isPendingUnobservation = false // for effective unobserving. BaseAtom has true, for extra optimization, so its onBecomeUnobserved never gets called, because it's not needed
+    isBeingObserved = false
     observers_ = new Set<IDerivation>()
 
     diffValue_ = 0

diff --git a/packages/mobx/src/core/computedvalue.ts b/packages/mobx/src/core/computedvalue.ts
@@ -56,6 +56,18 @@ export type IComputedDidChange<T = any> = {
     oldValue: T | undefined
 }
 
+function getFlag(flags: number, mask: number) {
+    return !!(flags & mask)
+}
+function setFlag(flags: number, mask: number, newValue: boolean): number {
+    if (newValue) {
+        flags |= mask
+    } else {
+        flags &= ~mask
+    }
+    return flags
+}
+
 /**
  * A node in the state dependency root that observes other nodes, and can be observed itself.
  *
@@ -79,8 +91,6 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
     dependenciesState_ = IDerivationState_.NOT_TRACKING_
     observing_: IObservable[] = [] // nodes we are looking at. Our value depends on these nodes
     newObserving_ = null // during tracking it's an array with new observed observers
-    isBeingObserved_ = false
-    isPendingUnobservation_: boolean = false
     observers_ = new Set<IDerivation>()
     diffValue_ = 0
     runId_ = 0
@@ -90,8 +100,13 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
     protected value_: T | undefined | CaughtException = new CaughtException(null)
     name_: string
     triggeredBy_?: string
-    isComputing_: boolean = false // to check for cycles
-    isRunningSetter_: boolean = false
+
+    private static readonly isComputingMask_ = 0b0001
+    private static readonly isRunningSetterMask_ = 0b0010
+    private static readonly isBeingObservedMask_ = 0b0100
+    private static readonly isPendingUnobservationMask_ = 0b1000
+    private flags_ = 0b0000
+
     derivation: () => T // N.B: unminified as it is used by MST
     setter_?: (value: T) => void
     isTracing_: TraceMode = TraceMode.NONE
@@ -153,12 +168,41 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
         }
     }
 
+    // to check for cycles
+    private get isComputing(): boolean {
+        return getFlag(this.flags_, ComputedValue.isComputingMask_)
+    }
+    private set isComputing(newValue: boolean) {
+        this.flags_ = setFlag(this.flags_, ComputedValue.isComputingMask_, newValue)
+    }
+
+    private get isRunningSetter(): boolean {
+        return getFlag(this.flags_, ComputedValue.isRunningSetterMask_)
+    }
+    private set isRunningSetter(newValue: boolean) {
+        this.flags_ = setFlag(this.flags_, ComputedValue.isRunningSetterMask_, newValue)
+    }
+
+    get isBeingObserved(): boolean {
+        return getFlag(this.flags_, ComputedValue.isBeingObservedMask_)
+    }
+    set isBeingObserved(newValue: boolean) {
+        this.flags_ = setFlag(this.flags_, ComputedValue.isBeingObservedMask_, newValue)
+    }
+
+    get isPendingUnobservation(): boolean {
+        return getFlag(this.flags_, ComputedValue.isPendingUnobservationMask_)
+    }
+    set isPendingUnobservation(newValue: boolean) {
+        this.flags_ = setFlag(this.flags_, ComputedValue.isPendingUnobservationMask_, newValue)
+    }
+
     /**
      * Returns the current value of this computed value.
      * Will evaluate its computation first if needed.
      */
     public get(): T {
-        if (this.isComputing_) {
+        if (this.isComputing) {
             die(32, this.name_, this.derivation)
         }
         if (
@@ -196,14 +240,14 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
 
     public set(value: T) {
         if (this.setter_) {
-            if (this.isRunningSetter_) {
+            if (this.isRunningSetter) {
                 die(33, this.name_)
             }
-            this.isRunningSetter_ = true
+            this.isRunningSetter = true
             try {
                 this.setter_.call(this.scope_, value)
             } finally {
-                this.isRunningSetter_ = false
+                this.isRunningSetter = false
             }
         } else {
             die(34, this.name_)
@@ -242,7 +286,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
     }
 
     computeValue_(track: boolean) {
-        this.isComputing_ = true
+        this.isComputing = true
         // don't allow state changes during computation
         const prev = allowStateChangesStart(false)
         let res: T | CaughtException
@@ -260,7 +304,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
             }
         }
         allowStateChangesEnd(prev)
-        this.isComputing_ = false
+        this.isComputing = false
         return res
     }
 

diff --git a/packages/mobx/src/core/observable.ts b/packages/mobx/src/core/observable.ts
@@ -24,10 +24,10 @@ export interface IObservable extends IDepTreeNode {
      * the dependency is already established
      */
     lastAccessedBy_: number
-    isBeingObserved_: boolean
+    isBeingObserved: boolean
 
     lowestObserverState_: IDerivationState_ // Used to avoid redundant propagations
-    isPendingUnobservation_: boolean // Used to push itself to global.pendingUnobservations at most once per batch.
+    isPendingUnobservation: boolean // Used to push itself to global.pendingUnobservations at most once per batch.
 
     observers_: Set<IDerivation>
 
@@ -91,9 +91,9 @@ export function removeObserver(observable: IObservable, node: IDerivation) {
 }
 
 export function queueForUnobservation(observable: IObservable) {
-    if (observable.isPendingUnobservation_ === false) {
+    if (observable.isPendingUnobservation === false) {
         // invariant(observable._observers.length === 0, "INTERNAL ERROR, should only queue for unobservation unobserved observables");
-        observable.isPendingUnobservation_ = true
+        observable.isPendingUnobservation = true
         globalState.pendingUnobservations.push(observable)
     }
 }
@@ -114,11 +114,11 @@ export function endBatch() {
         const list = globalState.pendingUnobservations
         for (let i = 0; i < list.length; i++) {
             const observable = list[i]
-            observable.isPendingUnobservation_ = false
+            observable.isPendingUnobservation = false
             if (observable.observers_.size === 0) {
-                if (observable.isBeingObserved_) {
+                if (observable.isBeingObserved) {
                     // if this observable had reactive observers, trigger the hooks
-                    observable.isBeingObserved_ = false
+                    observable.isBeingObserved = false
                     observable.onBUO()
                 }
                 if (observable instanceof ComputedValue) {
@@ -146,12 +146,12 @@ export function reportObserved(observable: IObservable): boolean {
             observable.lastAccessedBy_ = derivation.runId_
             // Tried storing newObserving, or observing, or both as Set, but performance didn't come close...
             derivation.newObserving_![derivation.unboundDepsCount_++] = observable
-            if (!observable.isBeingObserved_ && globalState.trackingContext) {
-                observable.isBeingObserved_ = true
+            if (!observable.isBeingObserved && globalState.trackingContext) {
+                observable.isBeingObserved = true
                 observable.onBO()
             }
         }
-        return observable.isBeingObserved_
+        return observable.isBeingObserved
     } else if (observable.observers_.size === 0 && globalState.inBatch > 0) {
         queueForUnobservation(observable)
     }