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.

Sign up for GitHub

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

Jump to bottom

add limits for how many files can have diagnostics #188161

Merged
merged 1 commit into from
Jul 18, 2023
Merged

add limits for how many files can have diagnostics #188161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 19 additions & 2 deletions src/vs/workbench/api/common/extHostDiagnostics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ export class DiagnosticCollection implements vscode.DiagnosticCollection {
constructor(
private readonly _name: string,
private readonly _owner: string,
private readonly _maxDiagnosticsTotal: number,
private readonly _maxDiagnosticsPerFile: number,
private readonly _modelVersionIdProvider: (uri: URI) => number | undefined,
extUri: IExtUri,
proxy: MainThreadDiagnosticsShape | undefined,
onDidChangeDiagnostics: Emitter<readonly vscode.Uri[]>
) {
this._maxDiagnosticsTotal = Math.max(_maxDiagnosticsPerFile, _maxDiagnosticsTotal);
this.#data = new ResourceMap(uri => extUri.getComparisonKey(uri));
this.#proxy = proxy;
this.#onDidChangeDiagnostics = onDidChangeDiagnostics;
Expand Down Expand Up @@ -123,6 +125,7 @@ export class DiagnosticCollection implements vscode.DiagnosticCollection {
return;
}
const entries: [URI, IMarkerData[]][] = [];
let totalMarkerCount = 0;
for (const uri of toSync) {
let marker: IMarkerData[] = [];
const diagnostics = this.#data.get(uri);
Expand Down Expand Up @@ -158,6 +161,12 @@ export class DiagnosticCollection implements vscode.DiagnosticCollection {
}

entries.push([uri, marker]);

totalMarkerCount += marker.length;
if (totalMarkerCount > this._maxDiagnosticsTotal) {
// ignore markers that are above the limit
break;
}
}
this.#proxy.$changeMany(this._owner, entries);
}
Expand Down Expand Up @@ -225,6 +234,7 @@ export class ExtHostDiagnostics implements ExtHostDiagnosticsShape {

private static _idPool: number = 0;
private static readonly _maxDiagnosticsPerFile: number = 1000;
private static readonly _maxDiagnosticsTotal: number = 1.1 * ExtHostDiagnostics._maxDiagnosticsPerFile;

private readonly _proxy: MainThreadDiagnosticsShape;
private readonly _collections = new Map<string, DiagnosticCollection>();
Expand Down Expand Up @@ -284,7 +294,9 @@ export class ExtHostDiagnostics implements ExtHostDiagnosticsShape {
const result = new class extends DiagnosticCollection {
constructor() {
super(
name!, owner, ExtHostDiagnostics._maxDiagnosticsPerFile,
name!, owner,
ExtHostDiagnostics._maxDiagnosticsTotal,
ExtHostDiagnostics._maxDiagnosticsPerFile,
uri => _extHostDocumentsAndEditors.getDocument(uri)?.version,
_fileSystemInfoService.extUri, loggingProxy, _onDidChangeDiagnostics
);
Expand Down Expand Up @@ -339,7 +351,12 @@ export class ExtHostDiagnostics implements ExtHostDiagnosticsShape {

if (!this._mirrorCollection) {
const name = '_generated_mirror';
const collection = new DiagnosticCollection(name, name, ExtHostDiagnostics._maxDiagnosticsPerFile, _uri => undefined, this._fileSystemInfoService.extUri, undefined, this._onDidChangeDiagnostics);
const collection = new DiagnosticCollection(
name, name,
Number.MAX_SAFE_INTEGER, Number.MAX_SAFE_INTEGER, // no limits because this collection is just a mirror of "sanitized" data
_uri => undefined,
this._fileSystemInfoService.extUri, undefined, this._onDidChangeDiagnostics
);
this._collections.set(name, collection);
this._mirrorCollection = collection;
}
Expand Down
54 changes: 38 additions & 16 deletions src/vs/workbench/api/test/browser/extHostDiagnostics.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ suite('ExtHostDiagnostics', () => {

test('disposeCheck', () => {

const collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
const collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());

collection.dispose();
collection.dispose(); // that's OK
Expand All @@ -56,13 +56,13 @@ suite('ExtHostDiagnostics', () => {


test('diagnostic collection, forEach, clear, has', function () {
let collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
let collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
assert.strictEqual(collection.name, 'test');
collection.dispose();
assert.throws(() => collection.name);

let c = 0;
collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
collection.forEach(() => c++);
assert.strictEqual(c, 0);

Expand Down Expand Up @@ -99,7 +99,7 @@ suite('ExtHostDiagnostics', () => {
});

test('diagnostic collection, immutable read', function () {
const collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
const collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
collection.set(URI.parse('foo:bar'), [
new Diagnostic(new Range(0, 0, 1, 1), 'message-1'),
new Diagnostic(new Range(0, 0, 1, 1), 'message-2')
Expand All @@ -124,7 +124,7 @@ suite('ExtHostDiagnostics', () => {


test('diagnostics collection, set with dupliclated tuples', function () {
const collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
const collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
const uri = URI.parse('sc:hightower');
collection.set([
[uri, [new Diagnostic(new Range(0, 0, 0, 1), 'message-1')]],
Expand Down Expand Up @@ -175,7 +175,7 @@ suite('ExtHostDiagnostics', () => {
test('diagnostics collection, set tuple overrides, #11547', function () {

let lastEntries!: [UriComponents, IMarkerData[]][];
const collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new class extends DiagnosticsShape {
const collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new class extends DiagnosticsShape {
override $changeMany(owner: string, entries: [UriComponents, IMarkerData[]][]): void {
lastEntries = entries;
return super.$changeMany(owner, entries);
Expand Down Expand Up @@ -209,7 +209,7 @@ suite('ExtHostDiagnostics', () => {

const emitter = new Emitter<any>();
emitter.event(_ => eventCount += 1);
const collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new class extends DiagnosticsShape {
const collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new class extends DiagnosticsShape {
override $changeMany() {
changeCount += 1;
}
Expand All @@ -229,7 +229,7 @@ suite('ExtHostDiagnostics', () => {

test('diagnostics collection, tuples and undefined (small array), #15585', function () {

const collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
const collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
const uri = URI.parse('sc:hightower');
const uri2 = URI.parse('sc:nomad');
const diag = new Diagnostic(new Range(0, 0, 0, 1), 'ffff');
Expand All @@ -250,7 +250,7 @@ suite('ExtHostDiagnostics', () => {

test('diagnostics collection, tuples and undefined (large array), #15585', function () {

const collection = new DiagnosticCollection('test', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
const collection = new DiagnosticCollection('test', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), new Emitter());
const tuples: [URI, Diagnostic[]][] = [];

for (let i = 0; i < 500; i++) {
Expand All @@ -271,10 +271,10 @@ suite('ExtHostDiagnostics', () => {
}
});

test('diagnostic capping', function () {
test('diagnostic capping (max per file)', function () {

let lastEntries!: [UriComponents, IMarkerData[]][];
const collection = new DiagnosticCollection('test', 'test', 250, versionProvider, extUri, new class extends DiagnosticsShape {
const collection = new DiagnosticCollection('test', 'test', 100, 250, versionProvider, extUri, new class extends DiagnosticsShape {
override $changeMany(owner: string, entries: [UriComponents, IMarkerData[]][]): void {
lastEntries = entries;
return super.$changeMany(owner, entries);
Expand All @@ -298,9 +298,31 @@ suite('ExtHostDiagnostics', () => {
assert.strictEqual(lastEntries[0][1][250].severity, MarkerSeverity.Info);
});

test('diagnostic capping (max files)', function () {

let lastEntries!: [UriComponents, IMarkerData[]][];
const collection = new DiagnosticCollection('test', 'test', 2, 1, versionProvider, extUri, new class extends DiagnosticsShape {
override $changeMany(owner: string, entries: [UriComponents, IMarkerData[]][]): void {
lastEntries = entries;
return super.$changeMany(owner, entries);
}
}, new Emitter());

const diag = new Diagnostic(new Range(0, 0, 1, 1), 'Hello');


collection.set([
[URI.parse('aa:bb1'), [diag]],
[URI.parse('aa:bb2'), [diag]],
[URI.parse('aa:bb3'), [diag]],
[URI.parse('aa:bb4'), [diag]],
]);
assert.strictEqual(lastEntries.length, 3); // goes above the limit and then stops
});

test('diagnostic eventing', async function () {
const emitter = new Emitter<readonly URI[]>();
const collection = new DiagnosticCollection('ddd', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), emitter);
const collection = new DiagnosticCollection('ddd', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), emitter);

const diag1 = new Diagnostic(new Range(1, 1, 2, 3), 'diag1');
const diag2 = new Diagnostic(new Range(1, 1, 2, 3), 'diag2');
Expand Down Expand Up @@ -338,7 +360,7 @@ suite('ExtHostDiagnostics', () => {

test('vscode.languages.onDidChangeDiagnostics Does Not Provide Document URI #49582', async function () {
const emitter = new Emitter<readonly URI[]>();
const collection = new DiagnosticCollection('ddd', 'test', 100, versionProvider, extUri, new DiagnosticsShape(), emitter);
const collection = new DiagnosticCollection('ddd', 'test', 100, 100, versionProvider, extUri, new DiagnosticsShape(), emitter);

const diag1 = new Diagnostic(new Range(1, 1, 2, 3), 'diag1');

Expand All @@ -361,7 +383,7 @@ suite('ExtHostDiagnostics', () => {

test('diagnostics with related information', function (done) {

const collection = new DiagnosticCollection('ddd', 'test', 100, versionProvider, extUri, new class extends DiagnosticsShape {
const collection = new DiagnosticCollection('ddd', 'test', 100, 100, versionProvider, extUri, new class extends DiagnosticsShape {
override $changeMany(owner: string, entries: [UriComponents, IMarkerData[]][]) {

const [[, data]] = entries;
Expand Down Expand Up @@ -424,7 +446,7 @@ suite('ExtHostDiagnostics', () => {

test('Error updating diagnostics from extension #60394', function () {
let callCount = 0;
const collection = new DiagnosticCollection('ddd', 'test', 100, versionProvider, extUri, new class extends DiagnosticsShape {
const collection = new DiagnosticCollection('ddd', 'test', 100, 100, versionProvider, extUri, new class extends DiagnosticsShape {
override $changeMany(owner: string, entries: [UriComponents, IMarkerData[]][]) {
callCount += 1;
}
Expand All @@ -451,7 +473,7 @@ suite('ExtHostDiagnostics', () => {

const all: [UriComponents, IMarkerData[]][] = [];

const collection = new DiagnosticCollection('ddd', 'test', 100, uri => {
const collection = new DiagnosticCollection('ddd', 'test', 100, 100, uri => {
return 7;
}, extUri, new class extends DiagnosticsShape {
override $changeMany(_owner: string, entries: [UriComponents, IMarkerData[]][]) {
Expand Down