From 6e93da1b71539f9f86c55b1b7eadedc602671d98 Mon Sep 17 00:00:00 2001 From: huangyanyan Date: Thu, 13 Apr 2023 09:21:37 +0800 Subject: [PATCH 1/3] Improve problem view performance Signed-off-by: huangyanyan --- .../problem-composite-tree-node.spec.ts | 50 ++++++--- .../problem/problem-composite-tree-node.ts | 59 ++++++---- .../problem/problem-decorations-provider.ts | 13 ++- .../src/browser/problem/problem-tree-model.ts | 21 +++- .../browser/monaco-diagnostic-collection.ts | 101 ------------------ .../monaco/src/browser/monaco-languages.ts | 43 +++----- .../src/browser/monaco-marker-collection.ts | 83 ++++++++++++++ 7 files changed, 196 insertions(+), 174 deletions(-) delete mode 100644 packages/monaco/src/browser/monaco-diagnostic-collection.ts create mode 100644 packages/monaco/src/browser/monaco-marker-collection.ts diff --git a/packages/markers/src/browser/problem/problem-composite-tree-node.spec.ts b/packages/markers/src/browser/problem/problem-composite-tree-node.spec.ts index 2b75550e88d88..5c3b9f5a4be47 100644 --- a/packages/markers/src/browser/problem/problem-composite-tree-node.spec.ts +++ b/packages/markers/src/browser/problem/problem-composite-tree-node.spec.ts @@ -75,12 +75,16 @@ describe('problem-composite-tree-node', () => { const lowMarkerNode = createMarkerInfo('2', new URI('b'), [lowNode]); const highFirstRoot = getRootNode('highFirstRoot'); - ProblemCompositeTreeNode.addChild(highFirstRoot, highMarkerNode, [highMarker]); - ProblemCompositeTreeNode.addChild(highFirstRoot, lowMarkerNode, [lowMarker]); + ProblemCompositeTreeNode.addChildren(highFirstRoot, new Map([ + [highMarkerNode.id, { node: highMarkerNode, markers: [highMarker] }], + [lowMarkerNode.id, { node: lowMarkerNode, markers: [lowMarker] }], + ])); expectCorrectOrdering(highFirstRoot); const lowFirstRoot = getRootNode('lowFirstRoot'); - ProblemCompositeTreeNode.addChild(lowFirstRoot, lowMarkerNode, [lowMarker]); - ProblemCompositeTreeNode.addChild(lowFirstRoot, highMarkerNode, [highMarker]); + ProblemCompositeTreeNode.addChildren(lowFirstRoot, new Map([ + [lowMarkerNode.id, { node: lowMarkerNode, markers: [lowMarker] }], + [highMarkerNode.id, { node: highMarkerNode, markers: [highMarker] }], + ])); expectCorrectOrdering(lowFirstRoot); function expectCorrectOrdering(root: MarkerRootNode): void { @@ -124,8 +128,10 @@ describe('problem-composite-tree-node', () => { const nodeB = createMockMarkerNode(markerB); const markerInfoNodeA = createMarkerInfo('1', new URI('a'), [nodeA]); const markerInfoNodeB = createMarkerInfo('2', new URI('b'), [nodeB]); - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeA, [markerA]); - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeB, [markerB]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }], + [markerInfoNodeB.id, { node: markerInfoNodeB, markers: [markerB] }], + ])); expect(rootNode.children.length).to.equal(2); expect(rootNode.children[0]).to.equal(markerInfoNodeA); @@ -138,10 +144,14 @@ describe('problem-composite-tree-node', () => { const markerA = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Error); const nodeA = createMockMarkerNode(markerA); const markerInfoNodeA = createMarkerInfo('1', new URI('a'), [nodeA]); - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeA, [markerA]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] + ])); markerA.data.severity = DiagnosticSeverity.Hint; - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeA, [markerA]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] + ])); expect(rootNode.children.length).to.equal(1); expect(rootNode.children[0]).to.equal(markerInfoNodeA); @@ -151,15 +161,21 @@ describe('problem-composite-tree-node', () => { const markerA = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Error); const nodeA = createMockMarkerNode(markerA); const markerInfoNodeA = createMarkerInfo('1', new URI('a'), [nodeA]); - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeA, [markerA]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] + ])); const markerB = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Error); const nodeB = createMockMarkerNode(markerB); const markerInfoNodeB = createMarkerInfo('2', new URI('b'), [nodeB]); - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeB, [markerB]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeB.id, { node: markerInfoNodeB, markers: [markerB] }] + ])); markerA.data.severity = DiagnosticSeverity.Hint; - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeA, [markerA]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] + ])); expect(rootNode.children.length).to.equal(2); expect(rootNode.children[0]).to.equal(markerInfoNodeB); @@ -172,15 +188,21 @@ describe('problem-composite-tree-node', () => { const markerA = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Hint); const nodeA = createMockMarkerNode(markerA); const markerInfoNodeA = createMarkerInfo('1', new URI('a'), [nodeA]); - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeA, [markerA]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] + ])); const markerB = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Error); const nodeB = createMockMarkerNode(markerB); const markerInfoNodeB = createMarkerInfo('2', new URI('b'), [nodeB]); - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeB, [markerB]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeB.id, { node: markerInfoNodeB, markers: [markerB] }] + ])); markerA.data.severity = DiagnosticSeverity.Error; - ProblemCompositeTreeNode.addChild(rootNode, markerInfoNodeA, [markerA]); + ProblemCompositeTreeNode.addChildren(rootNode, new Map([ + [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] + ])); expect(rootNode.children.length).to.equal(2); expect(rootNode.children[0]).to.equal(markerInfoNodeA); diff --git a/packages/markers/src/browser/problem/problem-composite-tree-node.ts b/packages/markers/src/browser/problem/problem-composite-tree-node.ts index c74fd99359a1e..95bc08780904d 100644 --- a/packages/markers/src/browser/problem/problem-composite-tree-node.ts +++ b/packages/markers/src/browser/problem/problem-composite-tree-node.ts @@ -33,33 +33,46 @@ export namespace ProblemCompositeTreeNode { parent.severity = maxSeverity; }; - export function addChild(parent: CompositeTreeNode, child: MarkerInfoNode, markers: Marker[]): CompositeTreeNode { - ProblemCompositeTreeNode.setSeverity(child, markers); + export function addChildren(parent: CompositeTreeNode, insertChildren: Map[] }>): void { + for (const { node, markers } of insertChildren.values()) { + ProblemCompositeTreeNode.setSeverity(node, markers); + } + + const sortedInsertChildren = new Map([...insertChildren.entries()].sort( + ([, a], [, b]) => (ProblemUtils.severityCompare(a.node.severity, b.node.severity) || compareURI(a.node.uri, b.node.uri)) + )); + + let startIndex = 0; const children = parent.children as MarkerInfoNode[]; - const index = children.findIndex(value => value.id === child.id); - if (index !== -1) { - CompositeTreeNode.removeChild(parent, child); - } if (children.length === 0) { - children.push(child); - CompositeTreeNode.setParent(child, 0, parent); - } else { - let inserted = false; - for (let i = 0; i < children.length; i++) { - // sort by severity, equal severity => sort by URI - if (ProblemUtils.severityCompare(child.severity, children[i].severity) < 0 - || (ProblemUtils.severityCompare(child.severity, children[i].severity) === 0 && compareURI(child.uri, children[i].uri) < 0)) { - children.splice(i, 0, child); - inserted = true; - CompositeTreeNode.setParent(child, i, parent); - break; - }; + for (const { node } of sortedInsertChildren.values()) { + const index = children.findIndex(value => value.id === node.id); + if (index !== -1) { + CompositeTreeNode.removeChild(parent, node); } - if (inserted === false) { - children.push(child); - CompositeTreeNode.setParent(child, children.length - 1, parent); + if (children.length === 0) { + children.push(node); + startIndex = 1; + CompositeTreeNode.setParent(node, 0, parent); + } else { + let inserted = false; + for (let i = startIndex; i < children.length; i++) { + // sort by severity, equal severity => sort by URI + if (ProblemUtils.severityCompare(node.severity, children[i].severity) < 0 + || (ProblemUtils.severityCompare(node.severity, children[i].severity) === 0 && compareURI(node.uri, children[i].uri) < 0)) { + children.splice(i, 0, node); + inserted = true; + startIndex = i + 1; + CompositeTreeNode.setParent(node, i, parent); + break; + }; + } + if (inserted === false) { + children.push(node); + startIndex = children.length; + CompositeTreeNode.setParent(node, children.length - 1, parent); + } } } - return parent; } const compareURI = (uri1: URI, uri2: URI): number => diff --git a/packages/markers/src/browser/problem/problem-decorations-provider.ts b/packages/markers/src/browser/problem/problem-decorations-provider.ts index 0ed8804f8a6ea..c3eeee175f704 100644 --- a/packages/markers/src/browser/problem/problem-decorations-provider.ts +++ b/packages/markers/src/browser/problem/problem-decorations-provider.ts @@ -21,6 +21,7 @@ import { ProblemManager } from './problem-manager'; import { ProblemUtils } from './problem-utils'; import { FrontendApplicationContribution } from '@theia/core/lib/browser'; import { CancellationToken, Emitter, Event, nls } from '@theia/core'; +import debounce = require('lodash.debounce'); @injectable() export class ProblemDecorationsProvider implements DecorationsProvider { @@ -35,13 +36,15 @@ export class ProblemDecorationsProvider implements DecorationsProvider { @postConstruct() protected init(): void { - this.problemManager.onDidChangeMarkers(() => { - const newUris = Array.from(this.problemManager.getUris(), stringified => new URI(stringified)); - this.onDidChangeEmitter.fire(newUris.concat(this.currentUris)); - this.currentUris = newUris; - }); + this.problemManager.onDidChangeMarkers(() => this.fireDidDecorationsChanged()); } + private fireDidDecorationsChanged = debounce(() => { + const newUris = Array.from(this.problemManager.getUris(), stringified => new URI(stringified)); + this.onDidChangeEmitter.fire(newUris.concat(this.currentUris)); + this.currentUris = newUris; + }, 10); + provideDecorations(uri: URI, token: CancellationToken): Decoration | Promise | undefined { const markers = this.problemManager.findMarkers({ uri }).filter(ProblemUtils.filterMarker).sort(ProblemUtils.severityCompareMarker); if (markers.length) { diff --git a/packages/markers/src/browser/problem/problem-tree-model.ts b/packages/markers/src/browser/problem/problem-tree-model.ts index d66618c73019f..9f1ed0aa0a8ff 100644 --- a/packages/markers/src/browser/problem/problem-tree-model.ts +++ b/packages/markers/src/browser/problem/problem-tree-model.ts @@ -17,16 +17,18 @@ import { ProblemMarker } from '../../common/problem-marker'; import { ProblemManager } from './problem-manager'; import { ProblemCompositeTreeNode } from './problem-composite-tree-node'; -import { MarkerNode, MarkerTree, MarkerOptions, MarkerInfoNode } from '../marker-tree'; +import { MarkerNode, MarkerTree, MarkerOptions, MarkerInfoNode, MarkerRootNode } from '../marker-tree'; import { MarkerTreeModel } from '../marker-tree-model'; import { injectable, inject } from '@theia/core/shared/inversify'; import { OpenerOptions, TreeNode } from '@theia/core/lib/browser'; import { Marker } from '../../common/marker'; import { Diagnostic } from '@theia/core/shared/vscode-languageserver-protocol'; import { ProblemUtils } from './problem-utils'; +import debounce = require('lodash.debounce'); @injectable() export class ProblemTree extends MarkerTree { + private _markers = new Map[] }>(); constructor( @inject(ProblemManager) markerManager: ProblemManager, @@ -77,12 +79,21 @@ export class ProblemTree extends MarkerTree { } protected override insertNodeWithMarkers(node: MarkerInfoNode, markers: Marker[]): void { - ProblemCompositeTreeNode.addChild(node.parent, node, markers); - const children = this.getMarkerNodes(node, markers); - node.numberOfMarkers = markers.length; - this.setChildren(node, children); + this._markers.set(node.id, { node, markers }); + this.doInsertNodesWithMarkers(); } + private doInsertNodesWithMarkers = debounce(() => { + ProblemCompositeTreeNode.addChildren(this.root as MarkerRootNode, this._markers); + + for (const { node, markers } of this._markers.values()) { + const children = this.getMarkerNodes(node, markers); + node.numberOfMarkers = markers.length; + this.setChildren(node, children); + } + + this._markers.clear(); + }, 10); } @injectable() diff --git a/packages/monaco/src/browser/monaco-diagnostic-collection.ts b/packages/monaco/src/browser/monaco-diagnostic-collection.ts deleted file mode 100644 index 27d8ecef95b39..0000000000000 --- a/packages/monaco/src/browser/monaco-diagnostic-collection.ts +++ /dev/null @@ -1,101 +0,0 @@ -// ***************************************************************************** -// Copyright (C) 2020 TypeFox and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// http://www.eclipse.org/legal/epl-2.0. -// -// This Source Code may also be made available under the following Secondary -// Licenses when the conditions for such availability set forth in the Eclipse -// Public License v. 2.0 are satisfied: GNU General Public License, version 2 -// with the GNU Classpath Exception which is available at -// https://www.gnu.org/software/classpath/license.html. -// -// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 -// ***************************************************************************** - -import { Diagnostic } from '@theia/core/shared/vscode-languageserver-protocol'; -import { DisposableCollection, Disposable } from '@theia/core/lib/common/disposable'; -import { ProtocolToMonacoConverter } from './protocol-to-monaco-converter'; -import * as monaco from '@theia/monaco-editor-core'; - -export class MonacoDiagnosticCollection implements Disposable { - - protected readonly diagnostics = new Map(); - protected readonly toDispose = new DisposableCollection(); - - constructor( - protected readonly name: string, - protected readonly p2m: ProtocolToMonacoConverter) { - } - - dispose(): void { - this.toDispose.dispose(); - } - - get(uri: string): Diagnostic[] { - const diagnostics = this.diagnostics.get(uri); - return !!diagnostics ? diagnostics.diagnostics : []; - } - - set(uri: string, diagnostics: Diagnostic[]): void { - const existing = this.diagnostics.get(uri); - if (existing) { - existing.diagnostics = diagnostics; - } else { - const modelDiagnostics = new MonacoModelDiagnostics(uri, diagnostics, this.name, this.p2m); - this.diagnostics.set(uri, modelDiagnostics); - this.toDispose.push(Disposable.create(() => { - this.diagnostics.delete(uri); - modelDiagnostics.dispose(); - })); - } - } - -} - -export class MonacoModelDiagnostics implements Disposable { - readonly uri: monaco.Uri; - protected _markers: monaco.editor.IMarkerData[] = []; - protected _diagnostics: Diagnostic[] = []; - constructor( - uri: string, - diagnostics: Diagnostic[], - readonly owner: string, - protected readonly p2m: ProtocolToMonacoConverter - ) { - this.uri = monaco.Uri.parse(uri); - this.diagnostics = diagnostics; - monaco.editor.onDidCreateModel(model => this.doUpdateModelMarkers(model)); - } - - set diagnostics(diagnostics: Diagnostic[]) { - this._diagnostics = diagnostics; - this._markers = this.p2m.asDiagnostics(diagnostics); - this.updateModelMarkers(); - } - - get diagnostics(): Diagnostic[] { - return this._diagnostics; - } - - get markers(): ReadonlyArray { - return this._markers; - } - - dispose(): void { - this._markers = []; - this.updateModelMarkers(); - } - - updateModelMarkers(): void { - const model = monaco.editor.getModel(this.uri); - this.doUpdateModelMarkers(model ? model : undefined); - } - - protected doUpdateModelMarkers(model: monaco.editor.ITextModel | undefined): void { - if (model && this.uri.toString() === model.uri.toString()) { - monaco.editor.setModelMarkers(model, this.owner, this._markers); - } - } -} diff --git a/packages/monaco/src/browser/monaco-languages.ts b/packages/monaco/src/browser/monaco-languages.ts index 8240fc27c78df..46ec65dfe9049 100644 --- a/packages/monaco/src/browser/monaco-languages.ts +++ b/packages/monaco/src/browser/monaco-languages.ts @@ -14,7 +14,7 @@ // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 // ***************************************************************************** -import { Diagnostic, SymbolInformation, WorkspaceSymbolParams } from '@theia/core/shared/vscode-languageserver-protocol'; +import { SymbolInformation, WorkspaceSymbolParams } from '@theia/core/shared/vscode-languageserver-protocol'; import { injectable, inject, postConstruct } from '@theia/core/shared/inversify'; import { ProblemManager } from '@theia/markers/lib/browser/problem/problem-manager'; import URI from '@theia/core/lib/common/uri'; @@ -22,7 +22,7 @@ import { MaybePromise, Mutable } from '@theia/core/lib/common/types'; import { Disposable } from '@theia/core/lib/common/disposable'; import { CancellationToken } from '@theia/core/lib/common/cancellation'; import { Language, LanguageService } from '@theia/core/lib/browser/language-service'; -import { MonacoDiagnosticCollection } from './monaco-diagnostic-collection'; +import { MonacoMarkerCollection } from './monaco-marker-collection'; import { ProtocolToMonacoConverter } from './protocol-to-monaco-converter'; import * as monaco from '@theia/monaco-editor-core'; @@ -36,40 +36,31 @@ export class MonacoLanguages implements LanguageService { readonly workspaceSymbolProviders: WorkspaceSymbolProvider[] = []; - protected readonly makers = new Map(); + protected readonly markers = new Map(); @inject(ProblemManager) protected readonly problemManager: ProblemManager; @inject(ProtocolToMonacoConverter) protected readonly p2m: ProtocolToMonacoConverter; @postConstruct() protected init(): void { - for (const uri of this.problemManager.getUris()) { - this.updateMarkers(new URI(uri)); - } this.problemManager.onDidChangeMarkers(uri => this.updateMarkers(uri)); + monaco.editor.onDidCreateModel(model => this.updateModelMarkers(model)); } - protected updateMarkers(uri: URI): void { + updateMarkers(uri: URI): void { + const markers = this.problemManager.findMarkers({ uri }); const uriString = uri.toString(); - const owners = new Map(); - for (const marker of this.problemManager.findMarkers({ uri })) { - const diagnostics = owners.get(marker.owner) || []; - diagnostics.push(marker.data); - owners.set(marker.owner, diagnostics); - } - const toClean = new Set(this.makers.keys()); - for (const [owner, diagnostics] of owners) { - toClean.delete(owner); - const collection = this.makers.get(owner) || new MonacoDiagnosticCollection(owner, this.p2m); - collection.set(uriString, diagnostics); - this.makers.set(owner, collection); - } - for (const owner of toClean) { - const collection = this.makers.get(owner); - if (collection) { - collection.set(uriString, []); - } - } + const collection = this.markers.get(uriString) || new MonacoMarkerCollection(uri, this.p2m); + this.markers.set(uriString, collection); + collection.updateMarkers(markers); + } + + updateModelMarkers(model: monaco.editor.ITextModel): void { + const uriString = model.uri.toString(); + const uri = new URI(uriString); + const collection = this.markers.get(uriString) || new MonacoMarkerCollection(uri, this.p2m); + this.markers.set(uriString, collection); + collection.updateModelMarkers(model); } registerWorkspaceSymbolProvider(provider: WorkspaceSymbolProvider): Disposable { diff --git a/packages/monaco/src/browser/monaco-marker-collection.ts b/packages/monaco/src/browser/monaco-marker-collection.ts new file mode 100644 index 0000000000000..87a153a8c53de --- /dev/null +++ b/packages/monaco/src/browser/monaco-marker-collection.ts @@ -0,0 +1,83 @@ +// ***************************************************************************** +// Copyright (C) 2020 TypeFox and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0. +// +// This Source Code may also be made available under the following Secondary +// Licenses when the conditions for such availability set forth in the Eclipse +// Public License v. 2.0 are satisfied: GNU General Public License, version 2 +// with the GNU Classpath Exception which is available at +// https://www.gnu.org/software/classpath/license.html. +// +// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 +// ***************************************************************************** + +import { Diagnostic } from '@theia/core/shared/vscode-languageserver-protocol'; +import { ProtocolToMonacoConverter } from './protocol-to-monaco-converter'; +import * as monaco from '@theia/monaco-editor-core'; +import { Marker } from '@theia/markers/lib/common/marker'; +import URI from '@theia/core/lib/common/uri'; + +export class MonacoMarkerCollection { + protected readonly uri: monaco.Uri; + protected readonly p2m: ProtocolToMonacoConverter; + protected markers: Marker[] = []; + protected owners = new Map(); + protected didUpdate: boolean = false; + + constructor( + uri: URI, + p2m: ProtocolToMonacoConverter + ) { + this.uri = monaco.Uri.parse(uri.toString()); + this.p2m = p2m; + } + + updateMarkers(markers: Marker[]): void { + this.markers = markers; + const model = monaco.editor.getModel(this.uri); + this.doUpdateMarkers(model ? model : undefined); + } + + updateModelMarkers(model: monaco.editor.ITextModel): void { + if (!this.didUpdate) { + this.doUpdateMarkers(model); + return; + } + for (const [owner, diagnostics] of this.owners) { + this.setModelMarkers(model, owner, diagnostics); + } + } + + doUpdateMarkers(model: monaco.editor.ITextModel | undefined): void { + if (!model) { + this.didUpdate = false; + return; + } + this.didUpdate = true; + const toClean = new Set(this.owners.keys()); + this.owners.clear(); + for (const marker of this.markers) { + const diagnostics = this.owners.get(marker.owner) || []; + diagnostics.push(marker.data); + this.owners.set(marker.owner, diagnostics); + } + for (const [owner, diagnostics] of this.owners) { + toClean.delete(owner); + this.setModelMarkers(model, owner, diagnostics); + } + for (const owner of toClean) { + this.clearModelMarkers(model, owner); + } + } + + setModelMarkers(model: monaco.editor.ITextModel, owner: string, diagnostics: Diagnostic[]): void { + monaco.editor.setModelMarkers(model, owner, this.p2m.asDiagnostics(diagnostics)); + } + + clearModelMarkers(model: monaco.editor.ITextModel, owner: string): void { + monaco.editor.setModelMarkers(model, owner, []); + } +} From ce041722073bf486d1598f5154ab2779594ea457 Mon Sep 17 00:00:00 2001 From: huangyanyan Date: Thu, 13 Apr 2023 14:58:00 +0800 Subject: [PATCH 2/3] fix lint error Signed-off-by: huangyanyan --- .../markers/src/browser/problem/problem-decorations-provider.ts | 2 +- packages/markers/src/browser/problem/problem-tree-model.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/markers/src/browser/problem/problem-decorations-provider.ts b/packages/markers/src/browser/problem/problem-decorations-provider.ts index c3eeee175f704..452af1d2dd546 100644 --- a/packages/markers/src/browser/problem/problem-decorations-provider.ts +++ b/packages/markers/src/browser/problem/problem-decorations-provider.ts @@ -21,7 +21,7 @@ import { ProblemManager } from './problem-manager'; import { ProblemUtils } from './problem-utils'; import { FrontendApplicationContribution } from '@theia/core/lib/browser'; import { CancellationToken, Emitter, Event, nls } from '@theia/core'; -import debounce = require('lodash.debounce'); +import debounce = require('@theia/core/shared/lodash.debounce'); @injectable() export class ProblemDecorationsProvider implements DecorationsProvider { diff --git a/packages/markers/src/browser/problem/problem-tree-model.ts b/packages/markers/src/browser/problem/problem-tree-model.ts index 9f1ed0aa0a8ff..ca1102d5aebe8 100644 --- a/packages/markers/src/browser/problem/problem-tree-model.ts +++ b/packages/markers/src/browser/problem/problem-tree-model.ts @@ -24,7 +24,7 @@ import { OpenerOptions, TreeNode } from '@theia/core/lib/browser'; import { Marker } from '../../common/marker'; import { Diagnostic } from '@theia/core/shared/vscode-languageserver-protocol'; import { ProblemUtils } from './problem-utils'; -import debounce = require('lodash.debounce'); +import debounce = require('@theia/core/shared/lodash.debounce'); @injectable() export class ProblemTree extends MarkerTree { From f2b61487c730d727b2548fdbbfc64cc8ba58ae84 Mon Sep 17 00:00:00 2001 From: huangyanyan Date: Wed, 19 Apr 2023 15:11:25 +0800 Subject: [PATCH 3/3] receive review comments: 1. use protected instead of private 2. split the debounce part from the actual execution 3. increase delay to 50ms 4. use array to store markers Signed-off-by: huangyanyan --- .../problem-composite-tree-node.spec.ts | 72 +++++++++---------- .../problem/problem-composite-tree-node.ts | 12 ++-- .../problem/problem-decorations-provider.ts | 6 +- .../src/browser/problem/problem-tree-model.ts | 14 ++-- 4 files changed, 53 insertions(+), 51 deletions(-) diff --git a/packages/markers/src/browser/problem/problem-composite-tree-node.spec.ts b/packages/markers/src/browser/problem/problem-composite-tree-node.spec.ts index 5c3b9f5a4be47..cb821a8ea3c2c 100644 --- a/packages/markers/src/browser/problem/problem-composite-tree-node.spec.ts +++ b/packages/markers/src/browser/problem/problem-composite-tree-node.spec.ts @@ -75,16 +75,16 @@ describe('problem-composite-tree-node', () => { const lowMarkerNode = createMarkerInfo('2', new URI('b'), [lowNode]); const highFirstRoot = getRootNode('highFirstRoot'); - ProblemCompositeTreeNode.addChildren(highFirstRoot, new Map([ - [highMarkerNode.id, { node: highMarkerNode, markers: [highMarker] }], - [lowMarkerNode.id, { node: lowMarkerNode, markers: [lowMarker] }], - ])); + ProblemCompositeTreeNode.addChildren(highFirstRoot, [ + { node: highMarkerNode, markers: [highMarker] }, + { node: lowMarkerNode, markers: [lowMarker] }, + ]); expectCorrectOrdering(highFirstRoot); const lowFirstRoot = getRootNode('lowFirstRoot'); - ProblemCompositeTreeNode.addChildren(lowFirstRoot, new Map([ - [lowMarkerNode.id, { node: lowMarkerNode, markers: [lowMarker] }], - [highMarkerNode.id, { node: highMarkerNode, markers: [highMarker] }], - ])); + ProblemCompositeTreeNode.addChildren(lowFirstRoot, [ + { node: lowMarkerNode, markers: [lowMarker] }, + { node: highMarkerNode, markers: [highMarker] }, + ]); expectCorrectOrdering(lowFirstRoot); function expectCorrectOrdering(root: MarkerRootNode): void { @@ -128,10 +128,10 @@ describe('problem-composite-tree-node', () => { const nodeB = createMockMarkerNode(markerB); const markerInfoNodeA = createMarkerInfo('1', new URI('a'), [nodeA]); const markerInfoNodeB = createMarkerInfo('2', new URI('b'), [nodeB]); - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }], - [markerInfoNodeB.id, { node: markerInfoNodeB, markers: [markerB] }], - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeA, markers: [markerA] }, + { node: markerInfoNodeB, markers: [markerB] }, + ]); expect(rootNode.children.length).to.equal(2); expect(rootNode.children[0]).to.equal(markerInfoNodeA); @@ -144,14 +144,14 @@ describe('problem-composite-tree-node', () => { const markerA = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Error); const nodeA = createMockMarkerNode(markerA); const markerInfoNodeA = createMarkerInfo('1', new URI('a'), [nodeA]); - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeA, markers: [markerA] } + ]); markerA.data.severity = DiagnosticSeverity.Hint; - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeA, markers: [markerA] } + ]); expect(rootNode.children.length).to.equal(1); expect(rootNode.children[0]).to.equal(markerInfoNodeA); @@ -161,21 +161,21 @@ describe('problem-composite-tree-node', () => { const markerA = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Error); const nodeA = createMockMarkerNode(markerA); const markerInfoNodeA = createMarkerInfo('1', new URI('a'), [nodeA]); - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeA, markers: [markerA] } + ]); const markerB = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Error); const nodeB = createMockMarkerNode(markerB); const markerInfoNodeB = createMarkerInfo('2', new URI('b'), [nodeB]); - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeB.id, { node: markerInfoNodeB, markers: [markerB] }] - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeB, markers: [markerB] } + ]); markerA.data.severity = DiagnosticSeverity.Hint; - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeA, markers: [markerA] } + ]); expect(rootNode.children.length).to.equal(2); expect(rootNode.children[0]).to.equal(markerInfoNodeB); @@ -188,21 +188,21 @@ describe('problem-composite-tree-node', () => { const markerA = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Hint); const nodeA = createMockMarkerNode(markerA); const markerInfoNodeA = createMarkerInfo('1', new URI('a'), [nodeA]); - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeA, markers: [markerA] } + ]); const markerB = createMockMarker({ start: { line: 0, character: 10 }, end: { line: 0, character: 10 } }, DiagnosticSeverity.Error); const nodeB = createMockMarkerNode(markerB); const markerInfoNodeB = createMarkerInfo('2', new URI('b'), [nodeB]); - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeB.id, { node: markerInfoNodeB, markers: [markerB] }] - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeB, markers: [markerB] } + ]); markerA.data.severity = DiagnosticSeverity.Error; - ProblemCompositeTreeNode.addChildren(rootNode, new Map([ - [markerInfoNodeA.id, { node: markerInfoNodeA, markers: [markerA] }] - ])); + ProblemCompositeTreeNode.addChildren(rootNode, [ + { node: markerInfoNodeA, markers: [markerA] } + ]); expect(rootNode.children.length).to.equal(2); expect(rootNode.children[0]).to.equal(markerInfoNodeA); diff --git a/packages/markers/src/browser/problem/problem-composite-tree-node.ts b/packages/markers/src/browser/problem/problem-composite-tree-node.ts index 95bc08780904d..5e602c17a224a 100644 --- a/packages/markers/src/browser/problem/problem-composite-tree-node.ts +++ b/packages/markers/src/browser/problem/problem-composite-tree-node.ts @@ -33,18 +33,18 @@ export namespace ProblemCompositeTreeNode { parent.severity = maxSeverity; }; - export function addChildren(parent: CompositeTreeNode, insertChildren: Map[] }>): void { - for (const { node, markers } of insertChildren.values()) { + export function addChildren(parent: CompositeTreeNode, insertChildren: { node: MarkerInfoNode, markers: Marker[] }[]): void { + for (const { node, markers } of insertChildren) { ProblemCompositeTreeNode.setSeverity(node, markers); } - const sortedInsertChildren = new Map([...insertChildren.entries()].sort( - ([, a], [, b]) => (ProblemUtils.severityCompare(a.node.severity, b.node.severity) || compareURI(a.node.uri, b.node.uri)) - )); + const sortedInsertChildren = insertChildren.sort( + (a, b) => (ProblemUtils.severityCompare(a.node.severity, b.node.severity) || compareURI(a.node.uri, b.node.uri)) + ); let startIndex = 0; const children = parent.children as MarkerInfoNode[]; - for (const { node } of sortedInsertChildren.values()) { + for (const { node } of sortedInsertChildren) { const index = children.findIndex(value => value.id === node.id); if (index !== -1) { CompositeTreeNode.removeChild(parent, node); diff --git a/packages/markers/src/browser/problem/problem-decorations-provider.ts b/packages/markers/src/browser/problem/problem-decorations-provider.ts index 452af1d2dd546..059ca822e0e68 100644 --- a/packages/markers/src/browser/problem/problem-decorations-provider.ts +++ b/packages/markers/src/browser/problem/problem-decorations-provider.ts @@ -39,11 +39,13 @@ export class ProblemDecorationsProvider implements DecorationsProvider { this.problemManager.onDidChangeMarkers(() => this.fireDidDecorationsChanged()); } - private fireDidDecorationsChanged = debounce(() => { + protected fireDidDecorationsChanged = debounce(() => this.doFireDidDecorationsChanged(), 50); + + protected doFireDidDecorationsChanged(): void { const newUris = Array.from(this.problemManager.getUris(), stringified => new URI(stringified)); this.onDidChangeEmitter.fire(newUris.concat(this.currentUris)); this.currentUris = newUris; - }, 10); + } provideDecorations(uri: URI, token: CancellationToken): Decoration | Promise | undefined { const markers = this.problemManager.findMarkers({ uri }).filter(ProblemUtils.filterMarker).sort(ProblemUtils.severityCompareMarker); diff --git a/packages/markers/src/browser/problem/problem-tree-model.ts b/packages/markers/src/browser/problem/problem-tree-model.ts index ca1102d5aebe8..3267f0ec9ae64 100644 --- a/packages/markers/src/browser/problem/problem-tree-model.ts +++ b/packages/markers/src/browser/problem/problem-tree-model.ts @@ -28,7 +28,7 @@ import debounce = require('@theia/core/shared/lodash.debounce'); @injectable() export class ProblemTree extends MarkerTree { - private _markers = new Map[] }>(); + protected markers: { node: MarkerInfoNode, markers: Marker[] }[] = []; constructor( @inject(ProblemManager) markerManager: ProblemManager, @@ -79,21 +79,21 @@ export class ProblemTree extends MarkerTree { } protected override insertNodeWithMarkers(node: MarkerInfoNode, markers: Marker[]): void { - this._markers.set(node.id, { node, markers }); + this.markers.push({ node, markers }); this.doInsertNodesWithMarkers(); } - private doInsertNodesWithMarkers = debounce(() => { - ProblemCompositeTreeNode.addChildren(this.root as MarkerRootNode, this._markers); + protected doInsertNodesWithMarkers = debounce(() => { + ProblemCompositeTreeNode.addChildren(this.root as MarkerRootNode, this.markers); - for (const { node, markers } of this._markers.values()) { + for (const { node, markers } of this.markers) { const children = this.getMarkerNodes(node, markers); node.numberOfMarkers = markers.length; this.setChildren(node, children); } - this._markers.clear(); - }, 10); + this.markers.length = 0; + }, 50); } @injectable()