diff --git a/src/vs/base/common/observableImpl/autorun.ts b/src/vs/base/common/observableImpl/autorun.ts index bff1ad7e4bcf4..23718e12328c6 100644 --- a/src/vs/base/common/observableImpl/autorun.ts +++ b/src/vs/base/common/observableImpl/autorun.ts @@ -67,19 +67,8 @@ export class AutorunObserver implements IObserver, IReader, IDisposable { private state = AutorunState.stale; private updateCount = 0; private disposed = false; - - /** - * The actual dependencies. - */ - private _dependencies = new Set>(); - public get dependencies() { - return this._dependencies; - } - - /** - * Dependencies that have to be removed when {@link runFn} ran through. - */ - private staleDependencies = new Set>(); + private dependencies = new Set>(); + private dependenciesToBeRemoved = new Set>(); constructor( public readonly debugName: string, @@ -87,22 +76,47 @@ export class AutorunObserver implements IObserver, IReader, IDisposable { private readonly _handleChange: ((context: IChangeContext) => boolean) | undefined ) { getLogger()?.handleAutorunCreated(this); - this.runIfNeeded(); + this._runIfNeeded(); } - public readObservable(observable: IObservable): T { - // In case the run action disposes the autorun - if (this.disposed) { - return observable.get(); + public dispose(): void { + this.disposed = true; + for (const o of this.dependencies) { + o.removeObserver(this); } + this.dependencies.clear(); + } - observable.addObserver(this); - const value = observable.get(); - this._dependencies.add(observable); - this.staleDependencies.delete(observable); - return value; + private _runIfNeeded() { + if (this.state === AutorunState.upToDate) { + return; + } + + const emptySet = this.dependenciesToBeRemoved; + this.dependenciesToBeRemoved = this.dependencies; + this.dependencies = emptySet; + + this.state = AutorunState.upToDate; + + getLogger()?.handleAutorunTriggered(this); + + try { + this.runFn(this); + } finally { + // We don't want our observed observables to think that they are (not even temporarily) not being observed. + // Thus, we only unsubscribe from observables that are definitely not read anymore. + for (const o of this.dependenciesToBeRemoved) { + o.removeObserver(this); + } + this.dependenciesToBeRemoved.clear(); + } } + public toString(): string { + return `Autorun<${this.debugName}>`; + } + + // IObserver implementation public beginUpdate(): void { if (this.state === AutorunState.upToDate) { this.state = AutorunState.dependenciesMightHaveChanged; @@ -115,7 +129,7 @@ export class AutorunObserver implements IObserver, IReader, IDisposable { do { if (this.state === AutorunState.dependenciesMightHaveChanged) { this.state = AutorunState.upToDate; - for (const d of this._dependencies) { + for (const d of this.dependencies) { d.reportChanges(); if (this.state as AutorunState === AutorunState.stale) { // The other dependencies will refresh on demand @@ -124,7 +138,7 @@ export class AutorunObserver implements IObserver, IReader, IDisposable { } } - this.runIfNeeded(); + this._runIfNeeded(); } while (this.state !== AutorunState.upToDate); } this.updateCount--; @@ -149,42 +163,18 @@ export class AutorunObserver implements IObserver, IReader, IDisposable { } } - private runIfNeeded() { - if (this.state === AutorunState.upToDate) { - return; - } - - // Assert: this.staleDependencies is an empty set. - const emptySet = this.staleDependencies; - this.staleDependencies = this._dependencies; - this._dependencies = emptySet; - - this.state = AutorunState.upToDate; - - getLogger()?.handleAutorunTriggered(this); - - try { - this.runFn(this); - } finally { - // We don't want our observed observables to think that they are (not even temporarily) not being observed. - // Thus, we only unsubscribe from observables that are definitely not read anymore. - for (const o of this.staleDependencies) { - o.removeObserver(this); - } - this.staleDependencies.clear(); - } - } - - public dispose(): void { - this.disposed = true; - for (const o of this._dependencies) { - o.removeObserver(this); + // IReader implementation + public readObservable(observable: IObservable): T { + // In case the run action disposes the autorun + if (this.disposed) { + return observable.get(); } - this._dependencies.clear(); - } - public toString(): string { - return `Autorun<${this.debugName}>`; + observable.addObserver(this); + const value = observable.get(); + this.dependencies.add(observable); + this.dependenciesToBeRemoved.delete(observable); + return value; } } diff --git a/src/vs/base/common/observableImpl/base.ts b/src/vs/base/common/observableImpl/base.ts index 8003e3d498698..993afcb6742e6 100644 --- a/src/vs/base/common/observableImpl/base.ts +++ b/src/vs/base/common/observableImpl/base.ts @@ -8,71 +8,106 @@ import type { derived } from 'vs/base/common/observableImpl/derived'; import { getLogger } from 'vs/base/common/observableImpl/logging'; export interface IObservable { - readonly TChange: TChange; - /** - * Reads the current value. + * Returns the current value. * - * Must not be called from {@link IObserver.handleChange}. + * Calls {@link IObserver.handleChange} if the observable notices that the value changed. + * Must not be called from {@link IObserver.handleChange}! */ get(): T; + /** + * Forces the observable to check for and report changes. + * + * Has the same effect as calling {@link IObservable.get}, but does not force the observable + * to actually construct the value, e.g. if change deltas are used. + * Calls {@link IObserver.handleChange} if the observable notices that the value changed. + * Must not be called from {@link IObserver.handleChange}! + */ reportChanges(): void; + /** + * Adds the observer to the set of subscribed observers. + * This method is idempotent. + */ addObserver(observer: IObserver): void; + + /** + * Removes the observer from the set of subscribed observers. + * This method is idempotent. + */ removeObserver(observer: IObserver): void; /** - * Subscribes the reader to this observable and returns the current value of this observable. + * Reads the current value and subscribes to this observable. + * + * Just calls {@link IReader.readObservable} if a reader is given, otherwise {@link IObservable.get} + * (see {@link ConvenientObservable.read}). */ read(reader: IReader | undefined): T; + /** + * Creates a derived observable that depends on this observable. + * Use the reader to read other observables + * (see {@link ConvenientObservable.map}). + */ map(fn: (value: T, reader: IReader) => TNew): IObservable; + /** + * A human-readable name for debugging purposes. + */ readonly debugName: string; - /*get isPossiblyStale(): boolean; - - get isUpdating(): boolean;*/ + /** + * This property captures the type of the change object. Do not use it at runtime! + */ + readonly TChange: TChange; } export interface IReader { /** * Reads the value of an observable and subscribes to it. - * - * Is called by {@link IObservable.read}. */ readObservable(observable: IObservable): T; } +/** + * Represents an observer that can be subscribed to an observable. + * + * If an observer is subscribed to an observable and that observable didn't signal + * a change through one of the observer methods, the observer can assume that the + * observable didn't change. + * If an observable reported a possible change, {@link IObservable.reportChanges} forces + * the observable to report an actual change if there was one. + */ export interface IObserver { /** - * Indicates that calling {@link IObservable.get} might return a different value and the observable is in updating mode. - * Must not be called when the given observable has already been reported to be in updating mode. + * Signals that the given observable might have changed and a transaction potentially modifying that observable started. + * Before the given observable can call this method again, is must call {@link IObserver.endUpdate}. + * + * The method {@link IObservable.reportChanges} can be used to force the observable to report the changes. */ beginUpdate(observable: IObservable): void; /** - * Is called by a subscribed observable when it leaves updating mode and it doesn't expect changes anymore, - * i.e. when a transaction for that observable is over. - * - * Call {@link IObservable.reportChanges} to learn about possible changes (if they weren't reported yet). + * Signals that the transaction that potentially modified the given observable ended. */ endUpdate(observable: IObservable): void; + /** + * Signals that the given observable might have changed. + * The method {@link IObservable.reportChanges} can be used to force the observable to report the changes. + * + * Implementations must not call into other observables, as they might not have received this event yet! + * The change should be processed lazily or in {@link IObserver.endUpdate}. + */ handlePossibleChange(observable: IObservable): void; /** - * Is called by a subscribed observable immediately after it notices a change. - * - * When {@link IObservable.get} is called two times and no change was reported before the second call returns, - * there has been no change in between the two calls for that observable. - * - * If the update counter is zero for a subscribed observable and calling {@link IObservable.get} didn't trigger a change, - * subsequent calls to {@link IObservable.get} don't trigger a change either until the update counter is increased again. + * Signals that the given observable changed. * - * Implementations must not call into other observables! - * The change should be processed when all observed observables settled. + * Implementations must not call into other observables, as they might not have received this event yet! + * The change should be processed lazily or in {@link IObserver.endUpdate}. */ handleChange(observable: IObservable, change: TChange): void; } @@ -83,13 +118,10 @@ export interface ISettable { export interface ITransaction { /** - * Calls `Observer.beginUpdate` immediately - * and `Observer.endUpdate` when the transaction is complete. + * Calls {@link Observer.beginUpdate} immediately + * and {@link Observer.endUpdate} when the transaction ends. */ - updateObserver( - observer: IObserver, - observable: IObservable - ): void; + updateObserver(observer: IObserver, observable: IObservable): void; } let _derived: typeof derived; diff --git a/src/vs/base/common/observableImpl/derived.ts b/src/vs/base/common/observableImpl/derived.ts index 1145d9e2d485d..1aee7c6033e7e 100644 --- a/src/vs/base/common/observableImpl/derived.ts +++ b/src/vs/base/common/observableImpl/derived.ts @@ -24,8 +24,13 @@ const enum DerivedState { /** * A dependency changed and we need to recompute. + * After recomputation, we need to check the previous value to see if we changed as well. */ stale = 2, + + /** + * No change reported, our cached value is up to date. + */ upToDate = 3, } @@ -33,16 +38,8 @@ export class Derived extends BaseObservable implements IReader, IObs private state = DerivedState.initial; private value: T | undefined = undefined; private updateCount = 0; - - private _dependencies = new Set>(); - public get dependencies(): ReadonlySet> { - return this._dependencies; - } - - /** - * Dependencies that have to be removed when {@link runFn} ran through. - */ - private staleDependencies = new Set>(); + private dependencies = new Set>(); + private dependenciesToBeRemoved = new Set>(); public override get debugName(): string { return typeof this._debugName === 'function' ? this._debugName() : this._debugName; @@ -62,15 +59,15 @@ export class Derived extends BaseObservable implements IReader, IObs * We are not tracking changes anymore, thus we have to assume * that our cache is invalid. */ - this.state = DerivedState.stale; + this.state = DerivedState.initial; this.value = undefined; - for (const d of this._dependencies) { + for (const d of this.dependencies) { d.removeObserver(this); } - this._dependencies.clear(); + this.dependencies.clear(); } - public get(): T { + public override get(): T { if (this.observers.size === 0) { // Without observers, we don't know when to clean up stuff. // Thus, we don't cache anything to prevent memory leaks. @@ -85,7 +82,7 @@ export class Derived extends BaseObservable implements IReader, IObs // thus we also have to ask all our depedencies if they changed in this case. this.state = DerivedState.upToDate; - for (const d of this._dependencies) { + for (const d of this.dependencies) { /** might call {@link handleChange} indirectly, which could invalidate us */ d.reportChanges(); @@ -97,6 +94,7 @@ export class Derived extends BaseObservable implements IReader, IObs } this._recomputeIfNeeded(); + // In case recomputation changed one of our dependencies, we need to recompute again. } while (this.state !== DerivedState.upToDate); return this.value!; } @@ -106,9 +104,9 @@ export class Derived extends BaseObservable implements IReader, IObs if (this.state === DerivedState.upToDate) { return; } - const emptySet = this.staleDependencies; - this.staleDependencies = this._dependencies; - this._dependencies = emptySet; + const emptySet = this.dependenciesToBeRemoved; + this.dependenciesToBeRemoved = this.dependencies; + this.dependencies = emptySet; const hadValue = this.state !== DerivedState.initial; const oldValue = this.value; @@ -120,10 +118,10 @@ export class Derived extends BaseObservable implements IReader, IObs } finally { // We don't want our observed observables to think that they are (not even temporarily) not being observed. // Thus, we only unsubscribe from observables that are definitely not read anymore. - for (const o of this.staleDependencies) { + for (const o of this.dependenciesToBeRemoved) { o.removeObserver(this); } - this.staleDependencies.clear(); + this.dependenciesToBeRemoved.clear(); } const didChange = hadValue && oldValue !== this.value; @@ -142,22 +140,28 @@ export class Derived extends BaseObservable implements IReader, IObs } } + public override toString(): string { + return `LazyDerived<${this.debugName}>`; + } + // IObserver Implementation public beginUpdate(): void { - const prevState = this.state; + this.updateCount++; + const propagateBeginUpdate = this.updateCount === 1; if (this.state === DerivedState.upToDate) { this.state = DerivedState.dependenciesMightHaveChanged; - } - if (this.updateCount === 0) { - for (const r of this.observers) { - r.beginUpdate(this); + // If we propagate begin update, that will already signal a possible change. + if (!propagateBeginUpdate) { + for (const r of this.observers) { + r.handlePossibleChange(this); + } } - } else if (prevState === DerivedState.upToDate) { + } + if (propagateBeginUpdate) { for (const r of this.observers) { - r.handlePossibleChange(this); + r.beginUpdate(this); // This signals a possible change } } - this.updateCount++; } public endUpdate(): void { @@ -170,21 +174,20 @@ export class Derived extends BaseObservable implements IReader, IObs } public handlePossibleChange(observable: IObservable): void { - if (this.state === DerivedState.upToDate && this._dependencies.has(observable)) { - // In all other states, observers already know that we might have changed. + // In all other states, observers already know that we might have changed. + if (this.state === DerivedState.upToDate && this.dependencies.has(observable)) { + this.state = DerivedState.dependenciesMightHaveChanged; for (const r of this.observers) { r.handlePossibleChange(this); } - this.state = DerivedState.dependenciesMightHaveChanged; } } public handleChange(observable: IObservable, _change: TChange): void { - if ((this.state === DerivedState.dependenciesMightHaveChanged || this.state === DerivedState.upToDate) && this._dependencies.has(observable)) { - const prevState = this.state; + const isUpToDate = this.state === DerivedState.upToDate; + if ((this.state === DerivedState.dependenciesMightHaveChanged || isUpToDate) && this.dependencies.has(observable)) { this.state = DerivedState.stale; - - if (prevState === DerivedState.upToDate) { + if (isUpToDate) { for (const r of this.observers) { r.handlePossibleChange(this); } @@ -199,12 +202,8 @@ export class Derived extends BaseObservable implements IReader, IObs /** This might call {@link handleChange} indirectly, which could invalidate us */ const value = observable.get(); // Which is why we only add the observable to the dependencies now. - this._dependencies.add(observable); - this.staleDependencies.delete(observable); + this.dependencies.add(observable); + this.dependenciesToBeRemoved.delete(observable); return value; } - - override toString(): string { - return `LazyDerived<${this.debugName}>`; - } }