Lazy eventhandlers (mobxjs#1160)

* improve performance by creating EventHandlers object only when needed

* small refactor

* update change log

changelog.md

@@ -1,3 +1,4 @@
+-   Performance improvement for event handlers so they consume less RAM through [#1160](https://github.com/mobxjs/mobx-state-tree/pull/1160) by [@xaviergonz](https://github.com/xaviergonz)
 -   Make liveliness errors give more info to trace their cause [#1142](https://github.com/mobxjs/mobx-state-tree/issues/1142) through [#1147](https://github.com/mobxjs/mobx-state-tree/pull/1147) by [@xaviergonz](https://github.com/xaviergonz)
 
 # 3.10.2

packages/mobx-state-tree/src/core/node/BaseNode.ts

@@ -5,10 +5,19 @@ import {
     escapeJsonPath,
     EventHandlers,
     IType,
-    IAnyType
+    IAnyType,
+    IDisposer
 } from "../../internal"
 import { createAtom, IAtom } from "mobx"
 
+type HookSubscribers = {
+    [Hook.afterAttach]: (node: AnyNode, hook: Hook) => void
+    [Hook.afterCreate]: (node: AnyNode, hook: Hook) => void
+    [Hook.afterCreationFinalization]: (node: AnyNode, hook: Hook) => void
+    [Hook.beforeDestroy]: (node: AnyNode, hook: Hook) => void
+    [Hook.beforeDetach]: (node: AnyNode, hook: Hook) => void
+}
+
 /**
  * @internal
  * @hidden
@@ -50,13 +59,23 @@ export abstract class BaseNode<C, S, T> {
     }
 
     readonly type: IAnyType
-    readonly hookSubscribers = new EventHandlers<{
-        [Hook.afterAttach]: (node: AnyNode, hook: Hook) => void
-        [Hook.afterCreate]: (node: AnyNode, hook: Hook) => void
-        [Hook.afterCreationFinalization]: (node: AnyNode, hook: Hook) => void
-        [Hook.beforeDestroy]: (node: AnyNode, hook: Hook) => void
-        [Hook.beforeDetach]: (node: AnyNode, hook: Hook) => void
-    }>()
+
+    private _hookSubscribers?: EventHandlers<HookSubscribers>
+
+    protected abstract fireHook(name: Hook): void
+
+    protected fireInternalHook(name: Hook) {
+        if (this._hookSubscribers) {
+            this._hookSubscribers.emit(name, this, name)
+        }
+    }
+
+    registerHook<H extends Hook>(hook: H, hookHandler: HookSubscribers[H]): IDisposer {
+        if (!this._hookSubscribers) {
+            this._hookSubscribers = new EventHandlers()
+        }
+        return this._hookSubscribers.register(hook, hookHandler)
+    }
 
     environment: any = undefined
 
@@ -116,12 +135,6 @@ export abstract class BaseNode<C, S, T> {
 
     abstract setParent(newParent: AnyObjectNode | null, subpath: string | null): void
 
-    protected abstract fireHook(name: Hook): void
-
-    protected fireInternalHook(name: Hook) {
-        this.hookSubscribers.emit(name, this, name)
-    }
-
     snapshot!: S
     abstract getSnapshot(): S
 
@@ -163,7 +176,9 @@ export abstract class BaseNode<C, S, T> {
     abstract finalizeDeath(): void
 
     protected baseFinalizeDeath() {
-        this.hookSubscribers.clearAll()
+        if (this._hookSubscribers) {
+            this._hookSubscribers.clearAll()
+        }
 
         this._subpathUponDeath = this._subpath
         this._pathUponDeath = this.getEscapedPath(false)

packages/mobx-state-tree/src/core/node/object-node.ts

@@ -34,7 +34,8 @@ import {
     getPath,
     warnError,
     AnyNode,
-    IStateTreeNode
+    IStateTreeNode,
+    ArgumentTypes
 } from "../../internal"
 
 let nextNodeId = 1
@@ -68,6 +69,12 @@ const snapshotReactionOptions = {
     }
 }
 
+type InternalEventHandlers<S> = {
+    [InternalEvents.Dispose]: () => void
+    [InternalEvents.Patch]: (patch: IJsonPatch, reversePatch: IJsonPatch) => void
+    [InternalEvents.Snapshot]: (snapshot: S) => void
+}
+
 /**
  * @internal
  * @hidden
@@ -103,12 +110,6 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
     _isRunningAction = false // only relevant for root
     private _hasSnapshotReaction = false
 
-    private readonly _internalEvents = new EventHandlers<{
-        [InternalEvents.Dispose]: () => void
-        [InternalEvents.Patch]: (patch: IJsonPatch, reversePatch: IJsonPatch) => void
-        [InternalEvents.Snapshot]: (snapshot: S) => void
-    }>()
-
     private _observableInstanceState = ObservableInstanceLifecycle.UNINITIALIZED
     private _childNodes: IChildNodesMap
     private _initialSnapshot: C
@@ -509,8 +510,8 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
         // a disposer added with addDisposer at this stage (beforeDestroy) is actually never released
         this.baseAboutToDie()
 
-        this._internalEvents.emit(InternalEvents.Dispose)
-        this._internalEvents.clear(InternalEvents.Dispose)
+        this._internalEventsEmit(InternalEvents.Dispose)
+        this._internalEventsClear(InternalEvents.Dispose)
     }
 
     private unregisterIdentifiers(): void {
@@ -534,53 +535,52 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
         const snapshot = this.snapshot
         this._snapshotUponDeath = snapshot
 
-        this._internalEvents.clearAll()
+        this._internalEventsClearAll()
 
         this.baseFinalizeDeath()
     }
 
     onSnapshot(onChange: (snapshot: S) => void): IDisposer {
         this._addSnapshotReaction()
-        return this._internalEvents.register(InternalEvents.Snapshot, onChange)
+        return this._internalEventsRegister(InternalEvents.Snapshot, onChange)
     }
 
     protected emitSnapshot(snapshot: S): void {
-        this._internalEvents.emit(InternalEvents.Snapshot, snapshot)
+        this._internalEventsEmit(InternalEvents.Snapshot, snapshot)
     }
 
     onPatch(handler: (patch: IJsonPatch, reversePatch: IJsonPatch) => void): IDisposer {
-        return this._internalEvents.register(InternalEvents.Patch, handler)
+        return this._internalEventsRegister(InternalEvents.Patch, handler)
     }
 
     emitPatch(basePatch: IReversibleJsonPatch, source: AnyNode): void {
-        const intEvs = this._internalEvents
-        if (intEvs.hasSubscribers(InternalEvents.Patch)) {
+        if (this._internalEventsHasSubscribers(InternalEvents.Patch)) {
             const localizedPatch: IReversibleJsonPatch = extend({}, basePatch, {
                 path: source.path.substr(this.path.length) + "/" + basePatch.path // calculate the relative path of the patch
             })
             const [patch, reversePatch] = splitPatch(localizedPatch)
-            intEvs.emit(InternalEvents.Patch, patch, reversePatch)
+            this._internalEventsEmit(InternalEvents.Patch, patch, reversePatch)
         }
         if (this.parent) this.parent.emitPatch(basePatch, source)
     }
 
     hasDisposer(disposer: () => void): boolean {
-        return this._internalEvents.has(InternalEvents.Dispose, disposer)
+        return this._internalEventsHas(InternalEvents.Dispose, disposer)
     }
 
     addDisposer(disposer: () => void): void {
         if (!this.hasDisposer(disposer)) {
-            this._internalEvents.register(InternalEvents.Dispose, disposer, true)
+            this._internalEventsRegister(InternalEvents.Dispose, disposer, true)
             return
         }
         throw fail("cannot add a disposer when it is already registered for execution")
     }
 
     removeDisposer(disposer: () => void): void {
-        if (!this._internalEvents.has(InternalEvents.Dispose, disposer)) {
+        if (!this._internalEventsHas(InternalEvents.Dispose, disposer)) {
             throw fail("cannot remove a disposer which was never registered for execution")
         }
-        this._internalEvents.unregister(InternalEvents.Dispose, disposer)
+        this._internalEventsUnregister(InternalEvents.Dispose, disposer)
     }
 
     removeMiddleware(handler: IMiddlewareHandler): void {
@@ -616,6 +616,66 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
             this._hasSnapshotReaction = true
         }
     }
+
+    // #region internal event handling
+
+    private _internalEvents?: EventHandlers<InternalEventHandlers<S>>
+
+    // we proxy the methods to avoid creating an EventHandlers instance when it is not needed
+
+    private _internalEventsHasSubscribers(event: InternalEvents): boolean {
+        return !!this._internalEvents && this._internalEvents.hasSubscribers(event)
+    }
+
+    private _internalEventsRegister<IE extends InternalEvents>(
+        event: IE,
+        eventHandler: InternalEventHandlers<S>[IE],
+        atTheBeginning = false
+    ): IDisposer {
+        if (!this._internalEvents) {
+            this._internalEvents = new EventHandlers()
+        }
+        return this._internalEvents.register(event, eventHandler, atTheBeginning)
+    }
+
+    private _internalEventsHas<IE extends InternalEvents>(
+        event: IE,
+        eventHandler: InternalEventHandlers<S>[IE]
+    ): boolean {
+        return !!this._internalEvents && this._internalEvents.has(event, eventHandler)
+    }
+
+    private _internalEventsUnregister<IE extends InternalEvents>(
+        event: IE,
+        eventHandler: InternalEventHandlers<S>[IE]
+    ): void {
+        if (this._internalEvents) {
+            this._internalEvents.unregister(event, eventHandler)
+        }
+    }
+
+    private _internalEventsEmit<IE extends InternalEvents>(
+        event: IE,
+        ...args: ArgumentTypes<InternalEventHandlers<S>[IE]>
+    ): void {
+        if (this._internalEvents) {
+            this._internalEvents.emit(event, ...args)
+        }
+    }
+
+    private _internalEventsClear(event: InternalEvents): void {
+        if (this._internalEvents) {
+            this._internalEvents.clear(event)
+        }
+    }
+
+    private _internalEventsClearAll(): void {
+        if (this._internalEvents) {
+            this._internalEvents.clearAll()
+        }
+    }
+
+    // #endregion
 }
 
 /**

packages/mobx-state-tree/src/types/utility-types/reference.ts

@@ -225,11 +225,11 @@ export abstract class BaseReferenceType<IT extends IAnyComplexType> extends Type
             this.fireInvalidated(cause, storedRefNode, referenceId, refTargetNode)
         }
 
-        const refTargetDetachHookDisposer = refTargetNode.hookSubscribers.register(
+        const refTargetDetachHookDisposer = refTargetNode.registerHook(
             Hook.beforeDetach,
             hookHandler
         )
-        const refTargetDestroyHookDisposer = refTargetNode.hookSubscribers.register(
+        const refTargetDestroyHookDisposer = refTargetNode.registerHook(
             Hook.beforeDestroy,
             hookHandler
         )
@@ -253,7 +253,7 @@ export abstract class BaseReferenceType<IT extends IAnyComplexType> extends Type
 
         // get rid of the watcher hook when the stored ref node is destroyed
         // detached is ignored since scalar nodes (where the reference resides) cannot be detached
-        storedRefNode.hookSubscribers.register(Hook.beforeDestroy, () => {
+        storedRefNode.registerHook(Hook.beforeDestroy, () => {
             if (onRefTargetDestroyedHookDisposer) {
                 onRefTargetDestroyedHookDisposer()
             }
@@ -308,15 +308,15 @@ export abstract class BaseReferenceType<IT extends IAnyComplexType> extends Type
         } else {
             if (!storedRefNode.isRoot) {
                 // start watching once the whole tree is ready
-                storedRefNode.root.hookSubscribers.register(Hook.afterCreationFinalization, () => {
+                storedRefNode.root.registerHook(Hook.afterCreationFinalization, () => {
                     // make sure to attach it so it can start listening
                     if (storedRefNode.parent) {
                         storedRefNode.parent.createObservableInstanceIfNeeded()
                     }
                 })
             }
             // start watching once the node is attached somewhere / parent changes
-            storedRefNode.hookSubscribers.register(Hook.afterAttach, () => {
+            storedRefNode.registerHook(Hook.afterAttach, () => {
                 startWatching(false)
             })
         }

packages/mobx-state-tree/src/utils.ts

@@ -219,7 +219,11 @@ export function addHiddenWritableProp(object: any, propName: string, value: any)
     })
 }
 
-type ArgumentTypes<F extends Function> = F extends (...args: infer A) => any ? A : never
+/**
+ * @internal
+ * @hidden
+ */
+export type ArgumentTypes<F extends Function> = F extends (...args: infer A) => any ? A : never
 
 /**
  * @internal

tslint.json

@@ -12,7 +12,6 @@
         "no-duplicate-variable": true,
         "no-eval": true,
         "no-internal-module": true,
-        "no-trailing-whitespace": true,
         "no-shadowed-variable": true,
         "no-switch-case-fall-through": true,
         "no-unused-expression": true,