Skip to content

Commit c4dbfb4

Browse files
crisbetoandrewseguin
authored andcommitted
perf(icon): avoid unnecessarily parsing icon sets (#18654)
Some internal refactoring to improve the performance of the icon registry by avoiding unnecessary work: * When an icon set is registered as a string literal, we parse it immediately into an SVG element. This is inefficient since the set may not be required immediately. These changes make it so that we store the SVG text and we only parse it when it's required. * When an icon is requested, we try to find it in all any of the icon sets which requires us to parse them since we don't know which one is supposed to have it. These changes add an inexpensive `indexOf` check before parsing so that we can quickly rule out the sets that absolutely can't have the icon. The check won't exclude 100% of the unnecessary parsing, but it should help with the majority. Once this is in we could make it slightly smarter by doing something like `svgText.indexOf(`id="${iconName}"`)`, but that's a bit more risky. Fixes #18644. (cherry picked from commit 5526ab9)
1 parent f4da874 commit c4dbfb4

File tree

1 file changed

+52
-62
lines changed

1 file changed

+52
-62
lines changed

src/material/icon/icon-registry.ts

Lines changed: 52 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -79,22 +79,17 @@ export interface IconOptions {
7979
* @docs-private
8080
*/
8181
class SvgIconConfig {
82-
url: SafeResourceUrl | null;
8382
svgElement: SVGElement | null;
8483

85-
constructor(url: SafeResourceUrl, options?: IconOptions);
86-
constructor(svgElement: SVGElement, options?: IconOptions);
87-
constructor(data: SafeResourceUrl | SVGElement, public options?: IconOptions) {
88-
// Note that we can't use `instanceof SVGElement` here,
89-
// because it'll break during server-side rendering.
90-
if (!!(data as any).nodeName) {
91-
this.svgElement = data as SVGElement;
92-
} else {
93-
this.url = data as SafeResourceUrl;
94-
}
95-
}
84+
constructor(
85+
public url: SafeResourceUrl,
86+
public svgText: string | null,
87+
public options?: IconOptions) {}
9688
}
9789

90+
/** Icon configuration whose content has already been loaded. */
91+
type LoadedSvgIconConfig = SvgIconConfig & {svgText: string};
92+
9893
/**
9994
* Service to register and display icons used by the `<mat-icon>` component.
10095
* - Registers icon URLs by namespace and name.
@@ -167,7 +162,7 @@ export class MatIconRegistry implements OnDestroy {
167162
*/
168163
addSvgIconInNamespace(namespace: string, iconName: string, url: SafeResourceUrl,
169164
options?: IconOptions): this {
170-
return this._addSvgIconConfig(namespace, iconName, new SvgIconConfig(url, options));
165+
return this._addSvgIconConfig(namespace, iconName, new SvgIconConfig(url, null, options));
171166
}
172167

173168
/**
@@ -178,14 +173,14 @@ export class MatIconRegistry implements OnDestroy {
178173
*/
179174
addSvgIconLiteralInNamespace(namespace: string, iconName: string, literal: SafeHtml,
180175
options?: IconOptions): this {
181-
const sanitizedLiteral = this._sanitizer.sanitize(SecurityContext.HTML, literal);
176+
const cleanLiteral = this._sanitizer.sanitize(SecurityContext.HTML, literal);
182177

183-
if (!sanitizedLiteral) {
178+
if (!cleanLiteral) {
184179
throw getMatIconFailedToSanitizeLiteralError(literal);
185180
}
186181

187-
const svgElement = this._createSvgElementForSingleIcon(sanitizedLiteral, options);
188-
return this._addSvgIconConfig(namespace, iconName, new SvgIconConfig(svgElement, options));
182+
return this._addSvgIconConfig(namespace, iconName,
183+
new SvgIconConfig('', cleanLiteral, options));
189184
}
190185

191186
/**
@@ -210,7 +205,7 @@ export class MatIconRegistry implements OnDestroy {
210205
* @param url
211206
*/
212207
addSvgIconSetInNamespace(namespace: string, url: SafeResourceUrl, options?: IconOptions): this {
213-
return this._addSvgIconSetConfig(namespace, new SvgIconConfig(url, options));
208+
return this._addSvgIconSetConfig(namespace, new SvgIconConfig(url, null, options));
214209
}
215210

216211
/**
@@ -220,14 +215,13 @@ export class MatIconRegistry implements OnDestroy {
220215
*/
221216
addSvgIconSetLiteralInNamespace(namespace: string, literal: SafeHtml,
222217
options?: IconOptions): this {
223-
const sanitizedLiteral = this._sanitizer.sanitize(SecurityContext.HTML, literal);
218+
const cleanLiteral = this._sanitizer.sanitize(SecurityContext.HTML, literal);
224219

225-
if (!sanitizedLiteral) {
220+
if (!cleanLiteral) {
226221
throw getMatIconFailedToSanitizeLiteralError(literal);
227222
}
228223

229-
const svgElement = this._svgElementFromString(sanitizedLiteral);
230-
return this._addSvgIconSetConfig(namespace, new SvgIconConfig(svgElement, options));
224+
return this._addSvgIconSetConfig(namespace, new SvgIconConfig('', cleanLiteral, options));
231225
}
232226

233227
/**
@@ -291,7 +285,7 @@ export class MatIconRegistry implements OnDestroy {
291285
return observableOf(cloneSvg(cachedIcon));
292286
}
293287

294-
return this._loadSvgIconFromConfig(new SvgIconConfig(safeUrl)).pipe(
288+
return this._loadSvgIconFromConfig(new SvgIconConfig(safeUrl, null)).pipe(
295289
tap(svg => this._cachedIconsByUrl.set(url!, svg)),
296290
map(svg => cloneSvg(svg)),
297291
);
@@ -334,15 +328,12 @@ export class MatIconRegistry implements OnDestroy {
334328
* Returns the cached icon for a SvgIconConfig if available, or fetches it from its URL if not.
335329
*/
336330
private _getSvgFromConfig(config: SvgIconConfig): Observable<SVGElement> {
337-
if (config.svgElement) {
331+
if (config.svgText) {
338332
// We already have the SVG element for this icon, return a copy.
339-
return observableOf(cloneSvg(config.svgElement));
333+
return observableOf(cloneSvg(this._svgElementFromConfig(config as LoadedSvgIconConfig)));
340334
} else {
341335
// Fetch the icon from the config's URL, cache it, and return a copy.
342-
return this._loadSvgIconFromConfig(config).pipe(
343-
tap(svg => config.svgElement = svg),
344-
map(svg => cloneSvg(svg)),
345-
);
336+
return this._loadSvgIconFromConfig(config).pipe(map(svg => cloneSvg(svg)));
346337
}
347338
}
348339

@@ -369,11 +360,11 @@ export class MatIconRegistry implements OnDestroy {
369360

370361
// Not found in any cached icon sets. If there are icon sets with URLs that we haven't
371362
// fetched, fetch them now and look for iconName in the results.
372-
const iconSetFetchRequests: Observable<SVGElement | null>[] = iconSetConfigs
373-
.filter(iconSetConfig => !iconSetConfig.svgElement)
363+
const iconSetFetchRequests: Observable<string | null>[] = iconSetConfigs
364+
.filter(iconSetConfig => !iconSetConfig.svgText)
374365
.map(iconSetConfig => {
375366
return this._loadSvgIconSetFromConfig(iconSetConfig).pipe(
376-
catchError((err: HttpErrorResponse): Observable<SVGElement | null> => {
367+
catchError((err: HttpErrorResponse) => {
377368
const url = this._sanitizer.sanitize(SecurityContext.RESOURCE_URL, iconSetConfig.url);
378369

379370
// Swallow errors fetching individual URLs so the
@@ -408,8 +399,14 @@ export class MatIconRegistry implements OnDestroy {
408399
// Iterate backwards, so icon sets added later have precedence.
409400
for (let i = iconSetConfigs.length - 1; i >= 0; i--) {
410401
const config = iconSetConfigs[i];
411-
if (config.svgElement) {
412-
const foundIcon = this._extractSvgIconFromSet(config.svgElement, iconName, config.options);
402+
403+
// Parsing the icon set's text into an SVG element can be expensive. We can avoid some of
404+
// the parsing by doing a quick check using `indexOf` to see if there's any chance for the
405+
// icon to be in the set. This won't be 100% accurate, but it should help us avoid at least
406+
// some of the parsing.
407+
if (config.svgText && config.svgText.indexOf(iconName) > -1) {
408+
const svg = this._svgElementFromConfig(config as LoadedSvgIconConfig);
409+
const foundIcon = this._extractSvgIconFromSet(svg, iconName, config.options);
413410
if (foundIcon) {
414411
return foundIcon;
415412
}
@@ -423,38 +420,22 @@ export class MatIconRegistry implements OnDestroy {
423420
* from it.
424421
*/
425422
private _loadSvgIconFromConfig(config: SvgIconConfig): Observable<SVGElement> {
426-
return this._fetchIcon(config)
427-
.pipe(map(svgText => this._createSvgElementForSingleIcon(svgText, config.options)));
423+
return this._fetchIcon(config).pipe(
424+
tap(svgText => config.svgText = svgText),
425+
map(() => this._svgElementFromConfig(config as LoadedSvgIconConfig))
426+
);
428427
}
429428

430429
/**
431-
* Loads the content of the icon set URL specified in the SvgIconConfig and creates an SVG element
432-
* from it.
430+
* Loads the content of the icon set URL specified in the
431+
* SvgIconConfig and attaches it to the config.
433432
*/
434-
private _loadSvgIconSetFromConfig(config: SvgIconConfig): Observable<SVGElement> {
435-
// If the SVG for this icon set has already been parsed, do nothing.
436-
if (config.svgElement) {
437-
return observableOf(config.svgElement);
433+
private _loadSvgIconSetFromConfig(config: SvgIconConfig): Observable<string | null> {
434+
if (config.svgText) {
435+
return observableOf(null);
438436
}
439437

440-
return this._fetchIcon(config).pipe(map(svgText => {
441-
// It is possible that the icon set was parsed and cached by an earlier request, so parsing
442-
// only needs to occur if the cache is yet unset.
443-
if (!config.svgElement) {
444-
config.svgElement = this._svgElementFromString(svgText);
445-
}
446-
447-
return config.svgElement;
448-
}));
449-
}
450-
451-
/**
452-
* Creates a DOM element from the given SVG string, and adds default attributes.
453-
*/
454-
private _createSvgElementForSingleIcon(responseText: string, options?: IconOptions): SVGElement {
455-
const svg = this._svgElementFromString(responseText);
456-
this._setSvgAttributes(svg, options);
457-
return svg;
438+
return this._fetchIcon(config).pipe(tap(svgText => config.svgText = svgText));
458439
}
459440

460441
/**
@@ -590,8 +571,6 @@ export class MatIconRegistry implements OnDestroy {
590571
return inProgressFetch;
591572
}
592573

593-
// TODO(jelbourn): for some reason, the `finalize` operator "loses" the generic type on the
594-
// Observable. Figure out why and fix it.
595574
const req = this._httpClient.get(url, {responseType: 'text', withCredentials}).pipe(
596575
finalize(() => this._inProgressUrlFetches.delete(url)),
597576
share(),
@@ -628,6 +607,17 @@ export class MatIconRegistry implements OnDestroy {
628607

629608
return this;
630609
}
610+
611+
/** Parses a config's text into an SVG element. */
612+
private _svgElementFromConfig(config: LoadedSvgIconConfig): SVGElement {
613+
if (!config.svgElement) {
614+
const svg = this._svgElementFromString(config.svgText);
615+
this._setSvgAttributes(svg, config.options);
616+
config.svgElement = svg;
617+
}
618+
619+
return config.svgElement;
620+
}
631621
}
632622

633623
/** @docs-private */

0 commit comments

Comments
 (0)