Skip to content

Commit eb884ba

Browse files
jriekenjrieken
committed
use ResourceMap in markerService, microsoft#93368 
1 parent ba64335 commit eb884ba

2 files changed

Lines changed: 89 additions & 90 deletions

File tree

‎src/vs/base/common/map.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,12 @@ export class ResourceMap<T> {
536536
return keys(this.map).map(k => URI.parse(k));
537537
}
538538

539+
*[Symbol.iterator](): Iterator<[URI, T]> {
540+
for (let item of this.map) {
541+
yield [URI.parse(item[0]), item[1]];
542+
}
543+
}
544+
539545
clone(): ResourceMap<T> {
540546
const resourceMap = new ResourceMap<T>();
541547

‎src/vs/platform/markers/common/markerService.ts‎

Lines changed: 83 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -6,40 +6,64 @@
66
import { isFalsyOrEmpty, isNonEmptyArray } from 'vs/base/common/arrays';
77
import { Schemas } from 'vs/base/common/network';
88
import { IDisposable } from 'vs/base/common/lifecycle';
9-
import { isEmptyObject } from 'vs/base/common/types';
109
import { URI } from 'vs/base/common/uri';
1110
import { Event, Emitter } from 'vs/base/common/event';
1211
import { IMarkerService, IMarkerData, IResourceMarker, IMarker, MarkerStatistics, MarkerSeverity } from './markers';
12+
import { ResourceMap } from 'vs/base/common/map';
13+
import { Iterable } from 'vs/base/common/iterator';
1314

14-
interface MapMap<V> {
15-
[key: string]: { [key: string]: V };
16-
}
15+
class DoubleResourceMap<V>{
16+
17+
private _byResource = new ResourceMap<Map<string, V>>();
18+
private _byOwner = new Map<string, ResourceMap<V>>();
1719

18-
namespace MapMap {
20+
set(resource: URI, owner: string, value: V) {
21+
let ownerMap = this._byResource.get(resource);
22+
if (!ownerMap) {
23+
ownerMap = new Map();
24+
this._byResource.set(resource, ownerMap);
25+
}
26+
ownerMap.set(owner, value);
1927

20-
export function get<V>(map: MapMap<V>, key1: string, key2: string): V | undefined {
21-
if (map[key1]) {
22-
return map[key1][key2];
28+
let resourceMap = this._byOwner.get(owner);
29+
if (!resourceMap) {
30+
resourceMap = new ResourceMap();
31+
this._byOwner.set(owner, resourceMap);
2332
}
24-
return undefined;
33+
resourceMap.set(resource, value);
2534
}
2635

27-
export function set<V>(map: MapMap<V>, key1: string, key2: string, value: V): void {
28-
if (!map[key1]) {
29-
map[key1] = Object.create(null);
36+
get(resource: URI, owner: string): V | undefined {
37+
let ownerMap = this._byResource.get(resource);
38+
return ownerMap?.get(owner);
39+
}
40+
41+
delete(resource: URI, owner: string): boolean {
42+
let removedA = false;
43+
let removedB = false;
44+
let ownerMap = this._byResource.get(resource);
45+
if (ownerMap) {
46+
removedA = ownerMap.delete(owner);
47+
}
48+
let resourceMap = this._byOwner.get(owner);
49+
if (resourceMap) {
50+
removedB = resourceMap.delete(resource);
51+
}
52+
if (removedA !== removedB) {
53+
throw new Error('illegal state');
3054
}
31-
map[key1][key2] = value;
55+
return removedA && removedB;
3256
}
3357

34-
export function remove(map: MapMap<any>, key1: string, key2: string): boolean {
35-
if (map[key1] && map[key1][key2]) {
36-
delete map[key1][key2];
37-
if (isEmptyObject(map[key1])) {
38-
delete map[key1];
39-
}
40-
return true;
58+
values(key?: URI | string): Iterable<V> {
59+
if (typeof key === 'string') {
60+
return this._byOwner.get(key)?.values() ?? Iterable.empty();
61+
}
62+
if (URI.isUri(key)) {
63+
return this._byResource.get(key)?.values() ?? Iterable.empty();
4164
}
42-
return false;
65+
66+
return Iterable.map(Iterable.concat(...this._byOwner.values()), map => map[1]);
4367
}
4468
}
4569

@@ -50,9 +74,9 @@ class MarkerStats implements MarkerStatistics {
5074
warnings: number = 0;
5175
unknowns: number = 0;
5276

53-
private _data?: { [resource: string]: MarkerStatistics } = Object.create(null);
54-
private _service: IMarkerService;
55-
private _subscription: IDisposable;
77+
private readonly _data = new ResourceMap<MarkerStatistics>();
78+
private readonly _service: IMarkerService;
79+
private readonly _subscription: IDisposable;
5680

5781
constructor(service: IMarkerService) {
5882
this._service = service;
@@ -61,23 +85,17 @@ class MarkerStats implements MarkerStatistics {
6185

6286
dispose(): void {
6387
this._subscription.dispose();
64-
this._data = undefined;
6588
}
6689

6790
private _update(resources: readonly URI[]): void {
68-
if (!this._data) {
69-
return;
70-
}
71-
7291
for (const resource of resources) {
73-
const key = resource.toString();
74-
const oldStats = this._data[key];
92+
const oldStats = this._data.get(resource);
7593
if (oldStats) {
7694
this._substract(oldStats);
7795
}
7896
const newStats = this._resourceStats(resource);
7997
this._add(newStats);
80-
this._data[key] = newStats;
98+
this._data.set(resource, newStats);
8199
}
82100
}
83101

@@ -124,10 +142,10 @@ export class MarkerService implements IMarkerService {
124142
_serviceBrand: undefined;
125143

126144
private readonly _onMarkerChanged = new Emitter<readonly URI[]>();
127-
private _onMarkerChangedEvent: Event<readonly URI[]> = Event.debounce(this._onMarkerChanged.event, MarkerService._debouncer, 0);
128-
private _byResource: MapMap<IMarker[]> = Object.create(null);
129-
private _byOwner: MapMap<IMarker[]> = Object.create(null);
130-
private _stats: MarkerStats;
145+
readonly onMarkerChanged: Event<readonly URI[]> = Event.debounce(this._onMarkerChanged.event, MarkerService._debouncer, 0);
146+
147+
private readonly _data = new DoubleResourceMap<IMarker[]>();
148+
private readonly _stats: MarkerStats;
131149

132150
constructor() {
133151
this._stats = new MarkerStats(this);
@@ -137,10 +155,6 @@ export class MarkerService implements IMarkerService {
137155
this._stats.dispose();
138156
}
139157

140-
get onMarkerChanged(): Event<readonly URI[]> {
141-
return this._onMarkerChangedEvent;
142-
}
143-
144158
getStatistics(): MarkerStatistics {
145159
return this._stats;
146160
}
@@ -155,12 +169,8 @@ export class MarkerService implements IMarkerService {
155169

156170
if (isFalsyOrEmpty(markerData)) {
157171
// remove marker for this (owner,resource)-tuple
158-
const a = MapMap.remove(this._byResource, resource.toString(), owner);
159-
const b = MapMap.remove(this._byOwner, owner, resource.toString());
160-
if (a !== b) {
161-
throw new Error('invalid marker service state');
162-
}
163-
if (a && b) {
172+
const removed = this._data.delete(resource, owner);
173+
if (removed) {
164174
this._onMarkerChanged.fire([resource]);
165175
}
166176

@@ -173,8 +183,7 @@ export class MarkerService implements IMarkerService {
173183
markers.push(marker);
174184
}
175185
}
176-
MapMap.set(this._byResource, resource.toString(), owner, markers);
177-
MapMap.set(this._byOwner, owner, resource.toString(), markers);
186+
this._data.set(resource, owner, markers);
178187
this._onMarkerChanged.fire([resource]);
179188
}
180189
}
@@ -216,21 +225,15 @@ export class MarkerService implements IMarkerService {
216225

217226
changeAll(owner: string, data: IResourceMarker[]): void {
218227
const changes: URI[] = [];
219-
const map = this._byOwner[owner];
220228

221229
// remove old marker
222-
if (map) {
223-
delete this._byOwner[owner];
224-
for (const resource in map) {
225-
const entry = MapMap.get(this._byResource, resource, owner);
226-
if (entry) {
227-
// remeber what we remove
228-
const [first] = entry;
229-
if (first) {
230-
changes.push(first.resource);
231-
}
232-
// actual remove
233-
MapMap.remove(this._byResource, resource, owner);
230+
const existing = this._data.values(owner);
231+
if (existing) {
232+
for (let data of existing) {
233+
const first = Iterable.first(data);
234+
if (first) {
235+
changes.push(first.resource);
236+
this._data.delete(first.resource, owner);
234237
}
235238
}
236239
}
@@ -239,26 +242,25 @@ export class MarkerService implements IMarkerService {
239242
if (isNonEmptyArray(data)) {
240243

241244
// group by resource
242-
const groups: { [resource: string]: IMarker[] } = Object.create(null);
245+
const groups = new ResourceMap<IMarker[]>();
243246
for (const { resource, marker: markerData } of data) {
244247
const marker = MarkerService._toMarker(owner, resource, markerData);
245248
if (!marker) {
246249
// filter bad markers
247250
continue;
248251
}
249-
const array = groups[resource.toString()];
252+
const array = groups.get(resource);
250253
if (!array) {
251-
groups[resource.toString()] = [marker];
254+
groups.set(resource, [marker]);
252255
changes.push(resource);
253256
} else {
254257
array.push(marker);
255258
}
256259
}
257260

258261
// insert all
259-
for (const resource in groups) {
260-
MapMap.set(this._byResource, resource, owner, groups[resource]);
261-
MapMap.set(this._byOwner, owner, resource, groups[resource]);
262+
for (const [resource, value] of groups) {
263+
this._data.set(resource, owner, value);
262264
}
263265
}
264266

@@ -277,7 +279,7 @@ export class MarkerService implements IMarkerService {
277279

278280
if (owner && resource) {
279281
// exactly one owner AND resource
280-
const data = MapMap.get(this._byResource, resource.toString(), owner);
282+
const data = this._data.get(resource, owner);
281283
if (!data) {
282284
return [];
283285
} else {
@@ -296,14 +298,12 @@ export class MarkerService implements IMarkerService {
296298
} else if (!owner && !resource) {
297299
// all
298300
const result: IMarker[] = [];
299-
for (const key1 in this._byResource) {
300-
for (const key2 in this._byResource[key1]) {
301-
for (const data of this._byResource[key1][key2]) {
302-
if (MarkerService._accept(data, severities)) {
303-
const newLen = result.push(data);
304-
if (take > 0 && newLen === take) {
305-
return result;
306-
}
301+
for (let markers of this._data.values()) {
302+
for (let data of markers) {
303+
if (MarkerService._accept(data, severities)) {
304+
const newLen = result.push(data);
305+
if (take > 0 && newLen === take) {
306+
return result;
307307
}
308308
}
309309
}
@@ -312,17 +312,10 @@ export class MarkerService implements IMarkerService {
312312

313313
} else {
314314
// of one resource OR owner
315-
const map: { [key: string]: IMarker[] } | undefined = owner
316-
? this._byOwner[owner]
317-
: resource ? this._byResource[resource.toString()] : undefined;
318-
319-
if (!map) {
320-
return [];
321-
}
322-
315+
const iterable = this._data.values(resource ?? owner!);
323316
const result: IMarker[] = [];
324-
for (const key in map) {
325-
for (const data of map[key]) {
317+
for (const markers of iterable) {
318+
for (const data of markers) {
326319
if (MarkerService._accept(data, severities)) {
327320
const newLen = result.push(data);
328321
if (take > 0 && newLen === take) {
@@ -341,16 +334,16 @@ export class MarkerService implements IMarkerService {
341334

342335
// --- event debounce logic
343336

344-
private static _dedupeMap: { [uri: string]: boolean };
337+
private static _dedupeMap: ResourceMap<true>;
345338

346339
private static _debouncer(last: URI[] | undefined, event: readonly URI[]): URI[] {
347340
if (!last) {
348-
MarkerService._dedupeMap = Object.create(null);
341+
MarkerService._dedupeMap = new ResourceMap();
349342
last = [];
350343
}
351344
for (const uri of event) {
352-
if (MarkerService._dedupeMap[uri.toString()] === undefined) {
353-
MarkerService._dedupeMap[uri.toString()] = true;
345+
if (!MarkerService._dedupeMap.has(uri)) {
346+
MarkerService._dedupeMap.set(uri, true);
354347
last.push(uri);
355348
}
356349
}

0 commit comments

Comments
 (0)