Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves observable logging #189355

Merged
merged 1 commit into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/vs/base/common/observableInternal/autorun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export class AutorunObserver<TChangeSummary = any> implements IObserver, IReader
this.runFn(this, changeSummary);
}
} finally {
getLogger()?.handleAutorunFinished(this);
// 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) {
Expand Down
21 changes: 16 additions & 5 deletions src/vs/base/common/observableInternal/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,17 @@ export abstract class ConvenientObservable<T, TChange> implements IObservable<T,
(reader) => fn(this.read(reader), reader),
() => {
const name = getFunctionName(fn);
return name !== undefined ? name : `${this.debugName} (mapped)`;
if (name !== undefined) {
return name;
}

// regexp to match `x => x.y` where x and y can be arbitrary identifiers (uses backref):
const regexp = /^\s*\(?\s*([a-zA-Z_$][a-zA-Z_$0-9]*)\s*\)?\s*=>\s*\1\.([a-zA-Z_$][a-zA-Z_$0-9]*)\s*$/;
const match = regexp.exec(fn.toString());
if (match) {
return `${this.debugName}.${match[2]}`;
}
return `${this.debugName} (mapped)`;
},
);
}
Expand Down Expand Up @@ -198,11 +208,9 @@ export abstract class BaseObservable<T, TChange = void> extends ConvenientObserv
export function transaction(fn: (tx: ITransaction) => void, getDebugName?: () => string): void {
const tx = new TransactionImpl(fn, getDebugName);
try {
getLogger()?.handleBeginTransaction(tx);
fn(tx);
} finally {
tx.finish();
getLogger()?.handleEndTransaction();
}
}

Expand All @@ -217,7 +225,9 @@ export function subtransaction(tx: ITransaction | undefined, fn: (tx: ITransacti
export class TransactionImpl implements ITransaction {
private updatingObservers: { observer: IObserver; observable: IObservable<any> }[] | null = [];

constructor(private readonly fn: Function, private readonly _getDebugName?: () => string) { }
constructor(private readonly fn: Function, private readonly _getDebugName?: () => string) {
getLogger()?.handleBeginTransaction(this);
}

public getDebugName(): string | undefined {
if (this._getDebugName) {
Expand All @@ -238,6 +248,7 @@ export class TransactionImpl implements ITransaction {
for (const { observer, observable } of updatingObservers) {
observer.endUpdate(observable);
}
getLogger()?.handleEndTransaction();
}
}

Expand Down Expand Up @@ -287,7 +298,7 @@ export class ObservableValue<T, TChange = void>
try {
const oldValue = this._value;
this._setValue(value);
getLogger()?.handleObservableChanged(this, { oldValue, newValue: value, change, didChange: true });
getLogger()?.handleObservableChanged(this, { oldValue, newValue: value, change, didChange: true, hadValue: true });

for (const observer of this.observers) {
tx.updateObserver(observer, this);
Expand Down
3 changes: 2 additions & 1 deletion src/vs/base/common/observableInternal/derived.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ export class Derived<T, TChangeSummary = any> extends BaseObservable<T, void> im
oldValue,
newValue: this.value,
change: undefined,
didChange
didChange,
hadValue,
});

if (didChange) {
Expand Down
46 changes: 31 additions & 15 deletions src/vs/base/common/observableInternal/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@ interface IChangeInformation {
newValue: unknown;
change: unknown;
didChange: boolean;
hadValue: boolean;
}

export interface IObservableLogger {
handleObservableChanged(observable: ObservableValue<unknown, unknown>, info: IChangeInformation): void;
handleObservableChanged(observable: ObservableValue<any, any>, info: IChangeInformation): void;
handleFromEventObservableTriggered(observable: FromEventObservable<any, any>, info: IChangeInformation): void;

handleAutorunCreated(autorun: AutorunObserver): void;
handleAutorunTriggered(autorun: AutorunObserver): void;
handleAutorunFinished(autorun: AutorunObserver): void;

handleDerivedCreated(observable: Derived<unknown>): void;
handleDerivedRecomputed(observable: Derived<unknown>, info: IChangeInformation): void;
handleDerivedCreated(observable: Derived<any>): void;
handleDerivedRecomputed(observable: Derived<any>, info: IChangeInformation): void;

handleBeginTransaction(transaction: TransactionImpl): void;
handleEndTransaction(): void;
Expand All @@ -50,6 +52,15 @@ export class ConsoleObservableLogger implements IObservableLogger {
}

private formatInfo(info: IChangeInformation): ConsoleText[] {
if (!info.hadValue) {
return [
normalText(` `),
styled(formatValue(info.newValue, 60), {
color: 'green',
}),
normalText(` (initial)`),
];
}
return info.didChange
? [
normalText(` `),
Expand Down Expand Up @@ -102,7 +113,8 @@ export class ConsoleObservableLogger implements IObservableLogger {
formatKind('derived recomputed'),
styled(derived.debugName, { color: 'BlueViolet' }),
...this.formatInfo(info),
this.formatChanges(changedObservables)
this.formatChanges(changedObservables),
{ data: [{ fn: derived['computeFn'] }] }
]));
changedObservables.clear();
}
Expand All @@ -112,6 +124,7 @@ export class ConsoleObservableLogger implements IObservableLogger {
formatKind('observable from event triggered'),
styled(observable.debugName, { color: 'BlueViolet' }),
...this.formatInfo(info),
{ data: [{ fn: observable['getValue'] }] }
]));
}

Expand All @@ -129,9 +142,15 @@ export class ConsoleObservableLogger implements IObservableLogger {
console.log(...this.textToConsoleArgs([
formatKind('autorun'),
styled(autorun.debugName, { color: 'BlueViolet' }),
this.formatChanges(changedObservables)
this.formatChanges(changedObservables),
{ data: [{ fn: autorun['runFn'] }] }
]));
changedObservables.clear();
this.indentation++;
}

handleAutorunFinished(autorun: AutorunObserver): void {
this.indentation--;
}

handleBeginTransaction(transaction: TransactionImpl): void {
Expand All @@ -142,6 +161,7 @@ export class ConsoleObservableLogger implements IObservableLogger {
console.log(...this.textToConsoleArgs([
formatKind('transaction'),
styled(transactionName, { color: 'BlueViolet' }),
{ data: [{ fn: transaction['fn'] }] }
]));
this.indentation++;
}
Expand All @@ -153,13 +173,12 @@ export class ConsoleObservableLogger implements IObservableLogger {

type ConsoleText =
| (ConsoleText | undefined)[]
| { text: string; style: string; data?: Record<string, unknown> }
| { data: Record<string, unknown> };
| { text: string; style: string; data?: unknown[] }
| { data: unknown[] };

function consoleTextToArgs(text: ConsoleText): unknown[] {
const styles = new Array<any>();
const initial = {};
const data = initial;
const data: unknown[] = [];
let firstArg = '';

function process(t: ConsoleText): void {
Expand All @@ -173,20 +192,17 @@ function consoleTextToArgs(text: ConsoleText): unknown[] {
firstArg += `%c${t.text}`;
styles.push(t.style);
if (t.data) {
Object.assign(data, t.data);
data.push(...t.data);
}
} else if ('data' in t) {
Object.assign(data, t.data);
data.push(...t.data);
}
}

process(text);

const result = [firstArg, ...styles];
if (Object.keys(data).length > 0) {
result.push(data);
}

result.push(...data);
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion src/vs/base/common/observableInternal/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class FromEventObservable<TArgs, T> extends BaseObservable<T> {

const didChange = !this.hasValue || this.value !== newValue;

getLogger()?.handleFromEventObservableTriggered(this, { oldValue: this.value, newValue, change: undefined, didChange });
getLogger()?.handleFromEventObservableTriggered(this, { oldValue: this.value, newValue, change: undefined, didChange, hadValue: this.hasValue });

if (didChange) {
this.value = newValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ export class ViewZoneManager extends Disposable {


this._register(autorun(reader => {
/** @description update */
/** @description update editor top offsets */
const m = this._diffModel.read(reader)?.syncedMovedTexts.read(reader);

let deltaOrigToMod = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class MovedBlocksLinesPart extends Disposable {
this._rootElement.appendChild(element);

this._register(autorun(reader => {
/** @description update */
/** @description update moved blocks lines positioning */
const info = this._originalEditorLayoutInfo.read(reader);
const info2 = this._modifiedEditorLayoutInfo.read(reader);
if (!info || !info2) {
Expand All @@ -45,8 +45,14 @@ export class MovedBlocksLinesPart extends Disposable {
const viewZonesChanged = observableSignalFromEvent('onDidChangeViewZones', this._editors.modified.onDidChangeViewZones);

this._register(autorun(reader => {
/** @description update */
element.replaceChildren();

/** @description update moved blocks lines */
const moves = this._diffModel.read(reader)?.diff.read(reader)?.movedTexts;
if (!moves) {
return;
}

viewZonesChanged.read(reader);

const info = this._originalEditorLayoutInfo.read(reader);
Expand All @@ -56,11 +62,6 @@ export class MovedBlocksLinesPart extends Disposable {
}
const width = info.verticalScrollbarWidth + info.contentLeft - MovedBlocksLinesPart.movedCodeBlockPadding;

const moves = this._diffModel.read(reader)?.diff.read(reader)?.movedTexts;
if (!moves) {
return;
}

let idx = 0;
for (const m of moves) {
function computeLineStart(range: LineRange, editor: ICodeEditor) {
Expand Down
1 change: 1 addition & 0 deletions src/vs/editor/browser/widget/diffEditorWidget2/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export class ObservableElementSizeObserver extends Disposable {
this._height = observableValue('height', this.elementSizeObserver.getHeight());

this._register(this.elementSizeObserver.onDidChange(e => transaction(tx => {
/** @description Set width/height from elementSizeObserver */
this._width.set(this.elementSizeObserver.getWidth(), tx);
this._height.set(this.elementSizeObserver.getHeight(), tx);
})));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export class InlineCompletionsSource extends Disposable {

this._updateOperation.clear();
transaction(tx => {
/** @description Update completions with provider result */
target.set(completions, tx);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export class SuggestWidgetAdaptor extends Disposable {
this._currentSuggestItemInfo = newInlineCompletion;

transaction(tx => {
/** @description Update state from suggest widget */
this.checkModelVersion(tx);
this._selectedItem.set(this._isActive ? this._currentSuggestItemInfo : undefined, tx);
});
Expand Down