Skip to content

Commit e557210

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Add component for experimental DuplicatedJavaScript insight
The Table component can now render hierarchical data. Rows that have subRows will render as indented rows. https://i.imgur.com/3sdWIQn.png Bug: 394373632 Change-Id: Ie727a11bf83c32e63d1bda8c448c640a69702643 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6319816 Auto-Submit: Connor Clark <[email protected]> Commit-Queue: Connor Clark <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent 8a3c413 commit e557210

File tree

16 files changed

+276
-96
lines changed

16 files changed

+276
-96
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1147,7 +1147,7 @@ grd_files_debug_sources = [
11471147
"front_end/models/trace/insights/Common.js",
11481148
"front_end/models/trace/insights/DOMSize.js",
11491149
"front_end/models/trace/insights/DocumentLatency.js",
1150-
"front_end/models/trace/insights/DuplicateJavaScript.js",
1150+
"front_end/models/trace/insights/DuplicatedJavaScript.js",
11511151
"front_end/models/trace/insights/FontDisplay.js",
11521152
"front_end/models/trace/insights/ForcedReflow.js",
11531153
"front_end/models/trace/insights/ImageDelivery.js",
@@ -1920,6 +1920,7 @@ grd_files_debug_sources = [
19201920
"front_end/panels/timeline/components/insights/Checklist.js",
19211921
"front_end/panels/timeline/components/insights/DOMSize.js",
19221922
"front_end/panels/timeline/components/insights/DocumentLatency.js",
1923+
"front_end/panels/timeline/components/insights/DuplicatedJavaScript.js",
19231924
"front_end/panels/timeline/components/insights/EventRef.js",
19241925
"front_end/panels/timeline/components/insights/FontDisplay.js",
19251926
"front_end/panels/timeline/components/insights/ForcedReflow.js",

front_end/models/trace/Processor.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ describeWithEnvironment('TraceProcessor', function() {
393393
'FontDisplay',
394394
'DOMSize',
395395
'ThirdParties',
396-
'DuplicateJavaScript',
396+
'DuplicatedJavaScript',
397397
'SlowCSSSelector',
398398
'ForcedReflow',
399399
'UseCache',
@@ -414,7 +414,7 @@ describeWithEnvironment('TraceProcessor', function() {
414414
'FontDisplay',
415415
'DOMSize',
416416
'ThirdParties',
417-
'DuplicateJavaScript',
417+
'DuplicatedJavaScript',
418418
'SlowCSSSelector',
419419
'ForcedReflow',
420420
'UseCache',

front_end/models/trace/Processor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ export class TraceProcessor extends EventTarget {
368368
Viewport: null,
369369
DOMSize: null,
370370
ThirdParties: null,
371-
DuplicateJavaScript: null,
371+
DuplicatedJavaScript: null,
372372
SlowCSSSelector: null,
373373
ForcedReflow: null,
374374
UseCache: null,

front_end/models/trace/extras/ScriptDuplication.test.ts

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -247,64 +247,66 @@ describeWithEnvironment('ScriptDuplication', function() {
247247
};
248248

249249
const results = Object.fromEntries(
250-
Trace.Extras.ScriptDuplication.computeScriptDuplication(scriptsData).entries().map(([key, value]) => {
251-
return [key, value.map(v => ({scriptId: v.script.scriptId as string, resourceSize: v.resourceSize}))];
250+
[...Trace.Extras.ScriptDuplication.computeScriptDuplication(scriptsData).entries()].map(([key, data]) => {
251+
return [
252+
key, data.duplicates.map(v => ({scriptId: v.script.scriptId as string, resourceSize: v.attributedSize}))
253+
];
252254
}));
253255
assert.deepEqual(results, {
254256
'coursehero:///Control/assets/js/vendor/ng/select/select.js': [
255257
{scriptId: '1.coursehero-bundle-1', resourceSize: 48513},
256258
{scriptId: '1.coursehero-bundle-2', resourceSize: 48513}
257259
],
260+
'coursehero:///js/src/search/results/store/filter-store.ts': [
261+
{scriptId: '1.coursehero-bundle-1', resourceSize: 12717},
262+
{scriptId: '1.coursehero-bundle-2', resourceSize: 12650}
263+
],
258264
'coursehero:///Control/assets/js/vendor/ng/select/angular-sanitize.js': [
259265
{scriptId: '1.coursehero-bundle-1', resourceSize: 9135},
260266
{scriptId: '1.coursehero-bundle-2', resourceSize: 9135}
261267
],
262-
'node_modules/@babel/runtime/helpers/inherits.js': [
263-
{scriptId: '1.coursehero-bundle-1', resourceSize: 528}, {scriptId: '1.coursehero-bundle-2', resourceSize: 528}
268+
'coursehero:///js/src/common/component/school-search.tsx': [
269+
{scriptId: '1.coursehero-bundle-2', resourceSize: 5840},
270+
{scriptId: '1.coursehero-bundle-1', resourceSize: 5316}
264271
],
265-
'node_modules/@babel/runtime/helpers/typeof.js': [
266-
{scriptId: '1.coursehero-bundle-1', resourceSize: 992}, {scriptId: '1.coursehero-bundle-2', resourceSize: 992}
272+
'coursehero:///js/src/search/results/view/filter/autocomplete-filter.tsx': [
273+
{scriptId: '1.coursehero-bundle-1', resourceSize: 3823},
274+
{scriptId: '1.coursehero-bundle-2', resourceSize: 3812}
275+
],
276+
'coursehero:///js/src/common/component/search/abstract-taxonomy-search.tsx': [
277+
{scriptId: '1.coursehero-bundle-1', resourceSize: 3103},
278+
{scriptId: '1.coursehero-bundle-2', resourceSize: 3098}
279+
],
280+
'coursehero:///js/src/search/results/view/filter/autocomplete-filter-with-icon.tsx': [
281+
{scriptId: '1.coursehero-bundle-1', resourceSize: 2696},
282+
{scriptId: '1.coursehero-bundle-2', resourceSize: 2693}
267283
],
268284
'coursehero:///js/src/utils/service/amplitude-service.ts': [
269285
{scriptId: '1.coursehero-bundle-1', resourceSize: 1348},
270286
{scriptId: '1.coursehero-bundle-2', resourceSize: 1325}
271287
],
272-
'coursehero:///js/src/utils/service/gsa-inmeta-tags.ts': [
273-
{scriptId: '1.coursehero-bundle-1', resourceSize: 591}, {scriptId: '1.coursehero-bundle-2', resourceSize: 563}
288+
'coursehero:///js/src/search/results/view/filter/autocomplete-list.tsx': [
289+
{scriptId: '1.coursehero-bundle-2', resourceSize: 1143},
290+
{scriptId: '1.coursehero-bundle-1', resourceSize: 1134}
291+
],
292+
'node_modules/@babel/runtime/helpers/typeof.js': [
293+
{scriptId: '1.coursehero-bundle-1', resourceSize: 992}, {scriptId: '1.coursehero-bundle-2', resourceSize: 992}
274294
],
275295
'coursehero:///js/src/search/results/store/filter-actions.ts': [
276296
{scriptId: '1.coursehero-bundle-2', resourceSize: 956}, {scriptId: '1.coursehero-bundle-1', resourceSize: 946}
277297
],
278298
'coursehero:///js/src/search/results/store/item/resource-types.ts': [
279299
{scriptId: '1.coursehero-bundle-1', resourceSize: 783}, {scriptId: '1.coursehero-bundle-2', resourceSize: 775}
280300
],
281-
'coursehero:///js/src/search/results/store/filter-store.ts': [
282-
{scriptId: '1.coursehero-bundle-1', resourceSize: 12717},
283-
{scriptId: '1.coursehero-bundle-2', resourceSize: 12650}
284-
],
285-
'coursehero:///js/src/search/results/view/filter/autocomplete-list.tsx': [
286-
{scriptId: '1.coursehero-bundle-2', resourceSize: 1143},
287-
{scriptId: '1.coursehero-bundle-1', resourceSize: 1134}
288-
],
289-
'coursehero:///js/src/search/results/view/filter/autocomplete-filter.tsx': [
290-
{scriptId: '1.coursehero-bundle-1', resourceSize: 3823},
291-
{scriptId: '1.coursehero-bundle-2', resourceSize: 3812}
301+
'coursehero:///js/src/utils/service/gsa-inmeta-tags.ts': [
302+
{scriptId: '1.coursehero-bundle-1', resourceSize: 591}, {scriptId: '1.coursehero-bundle-2', resourceSize: 563}
292303
],
293-
'coursehero:///js/src/search/results/view/filter/autocomplete-filter-with-icon.tsx': [
294-
{scriptId: '1.coursehero-bundle-1', resourceSize: 2696},
295-
{scriptId: '1.coursehero-bundle-2', resourceSize: 2693}
304+
'node_modules/@babel/runtime/helpers/inherits.js': [
305+
{scriptId: '1.coursehero-bundle-1', resourceSize: 528}, {scriptId: '1.coursehero-bundle-2', resourceSize: 528}
296306
],
297307
'coursehero:///js/src/search/results/service/api/filter-api-service.ts': [
298308
{scriptId: '1.coursehero-bundle-1', resourceSize: 554}, {scriptId: '1.coursehero-bundle-2', resourceSize: 534}
299309
],
300-
'coursehero:///js/src/common/component/school-search.tsx': [
301-
{scriptId: '1.coursehero-bundle-2', resourceSize: 5840},
302-
{scriptId: '1.coursehero-bundle-1', resourceSize: 5316}
303-
],
304-
'coursehero:///js/src/common/component/search/abstract-taxonomy-search.tsx': [
305-
{scriptId: '1.coursehero-bundle-1', resourceSize: 3103},
306-
{scriptId: '1.coursehero-bundle-2', resourceSize: 3098}
307-
],
308310
'coursehero:///js/src/common/component/search/course-search.tsx': [
309311
{scriptId: '1.coursehero-bundle-2', resourceSize: 545}, {scriptId: '1.coursehero-bundle-1', resourceSize: 544}
310312
]
@@ -313,28 +315,46 @@ describeWithEnvironment('ScriptDuplication', function() {
313315
});
314316

315317
describe('normalizeDuplication', () => {
318+
function makeDuplication(entries: Array<{source: string, resourceSize: number}>):
319+
Trace.Extras.ScriptDuplication.ScriptDuplication {
320+
const duplication = new Map();
321+
322+
for (const {source, resourceSize} of entries) {
323+
const data = duplication.get(source) ?? {estimatedWastedBytes: 0, duplicates: []};
324+
duplication.set(source, data);
325+
data.duplicates.push({resourceSize});
326+
}
327+
328+
return duplication;
329+
}
330+
316331
it('removes entries with just one value', () => {
317-
const duplication = new Map([['1', [{resourceSize: 100}]]]) as Trace.Extras.ScriptDuplication.ScriptDuplication;
332+
const duplication = makeDuplication([{source: '1', resourceSize: 100}]);
318333
Trace.Extras.ScriptDuplication.normalizeDuplication(duplication);
319334
const results = Object.fromEntries(duplication);
320335
assert.deepEqual(results, {});
321336
});
322337

323338
it('sorts entries based on resource size', () => {
324-
const duplication = new Map([
325-
['1', [{resourceSize: 250}, {resourceSize: 200}]],
326-
['2', [{resourceSize: 200}, {resourceSize: 250}]],
327-
]) as Trace.Extras.ScriptDuplication.ScriptDuplication;
339+
const duplication = makeDuplication([
340+
{source: '1', resourceSize: 250},
341+
{source: '1', resourceSize: 200},
342+
{source: '2', resourceSize: 200},
343+
{source: '2', resourceSize: 250},
344+
]);
328345
Trace.Extras.ScriptDuplication.normalizeDuplication(duplication);
329346
const results = Object.fromEntries(duplication);
330347
assert.deepEqual(results, {});
331348
});
332349

333350
it('removes duplication if size is much smaller than the largest', () => {
334-
const duplication = new Map([
335-
['1', [{resourceSize: 200}, {resourceSize: 1}, {resourceSize: 250}]],
336-
['2', [{resourceSize: 250}, {resourceSize: 1}]],
337-
]) as Trace.Extras.ScriptDuplication.ScriptDuplication;
351+
const duplication = makeDuplication([
352+
{source: '1', resourceSize: 200},
353+
{source: '1', resourceSize: 1},
354+
{source: '1', resourceSize: 250},
355+
{source: '2', resourceSize: 250},
356+
{source: '2', resourceSize: 1},
357+
]);
338358
Trace.Extras.ScriptDuplication.normalizeDuplication(duplication);
339359
const results = Object.fromEntries(duplication);
340360
assert.deepEqual(results, {});

front_end/models/trace/extras/ScriptDuplication.ts

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
import type * as SDK from '../../../core/sdk/sdk.js';
66
import type * as Handlers from '../handlers/handlers.js';
77

8-
const RELATIVE_SIZE_THRESHOLD = 0.1;
8+
// Ignore modules smaller than an absolute threshold.
99
const ABSOLUTE_SIZE_THRESHOLD_BYTES = 1024 * 0.5;
10+
// Ignore modules smaller than a % size of largest copy of the module.
11+
const RELATIVE_SIZE_THRESHOLD = 0.1;
1012

1113
type GeneratedFileSizes = {
1214
errorMessage: string,
@@ -113,44 +115,64 @@ function shouldIgnoreSource(source: string): boolean {
113115
}
114116

115117
/**
116-
* The key is a source map `sources` entry, but normalized via `normalizeSource`.
118+
* The key is a source map `sources` entry (these are URLs/file paths), but normalized
119+
* via `normalizeSource`.
117120
*
118-
* The value is an array with an entry for every script that has a source map which
121+
* The value is an object with an entry for every script that has a source map which
119122
* denotes that this source was used, along with the estimated resource size it takes
120123
* up in the script.
121124
*/
122-
export type ScriptDuplication =
123-
Map<string, Array<{script: Handlers.ModelHandlers.Scripts.Script, resourceSize: number}>>;
125+
export type ScriptDuplication = Map<string, {
126+
/**
127+
* This is the sum of all (but one) `attributedSize` in `scripts`.
128+
*
129+
* One copy of this module is treated as the canonical version - the rest will
130+
* have non-zero `wastedBytes`. The canonical copy is the first entry of
131+
* `scripts`.
132+
*
133+
* In the case of all copies being the same version, all sizes are
134+
* equal and the selection doesn't matter (ignoring compression ratios). When
135+
* the copies are different versions, it does matter. Ideally the newest
136+
* version would be the canonical copy, but version information is not present.
137+
* Instead, size is used as a heuristic for latest version. This makes the
138+
* value here conserative in its estimation.
139+
*/
140+
estimatedDuplicateBytes: number,
141+
duplicates: Array<{
142+
script: Handlers.ModelHandlers.Scripts.Script,
143+
/** The number of bytes in the script bundle that map back to this module. */
144+
attributedSize: number,
145+
}>,
146+
}>;
124147

125148
/**
126-
* Sorts each array within @see ScriptDuplication by resource size, and drops information
127-
* on sources that are too small.
149+
* Sorts each array within @see ScriptDuplication by attributedSize, drops information
150+
* on sources that are too small, and calculates esimatedDuplicateBytes.
128151
*/
129152
export function normalizeDuplication(duplication: ScriptDuplication): void {
130-
for (const [key, originalSourceData] of duplication.entries()) {
131-
let sourceData = originalSourceData;
132-
153+
for (const [key, data] of duplication) {
133154
// Sort by resource size.
134-
sourceData.sort((a, b) => b.resourceSize - a.resourceSize);
155+
data.duplicates.sort((a, b) => b.attributedSize - a.attributedSize);
135156

136-
// Remove modules smaller than a % size of largest.
137-
if (sourceData.length > 1) {
138-
const largestResourceSize = sourceData[0].resourceSize;
139-
sourceData = sourceData.filter(data => {
140-
const percentSize = data.resourceSize / largestResourceSize;
157+
// Ignore modules smaller than a % size of largest.
158+
if (data.duplicates.length > 1) {
159+
const largestResourceSize = data.duplicates[0].attributedSize;
160+
data.duplicates = data.duplicates.filter(duplicate => {
161+
const percentSize = duplicate.attributedSize / largestResourceSize;
141162
return percentSize >= RELATIVE_SIZE_THRESHOLD;
142163
});
143164
}
144165

145-
// Remove modules smaller than an absolute threshold.
146-
sourceData = sourceData.filter(data => data.resourceSize >= ABSOLUTE_SIZE_THRESHOLD_BYTES);
166+
// Ignore modules smaller than an absolute threshold.
167+
data.duplicates = data.duplicates.filter(duplicate => duplicate.attributedSize >= ABSOLUTE_SIZE_THRESHOLD_BYTES);
147168

148-
// Delete any that now don't have multiple source data entries.
149-
if (sourceData.length > 1) {
150-
duplication.set(key, sourceData);
151-
} else {
169+
// Delete any that now don't have multiple entries.
170+
if (data.duplicates.length <= 1) {
152171
duplication.delete(key);
172+
continue;
153173
}
174+
175+
data.estimatedDuplicateBytes = data.duplicates.slice(1).reduce((acc, cur) => acc + cur.attributedSize, 0);
154176
}
155177
}
156178

@@ -211,21 +233,23 @@ export function computeScriptDuplication(scriptsData: Handlers.ModelHandlers.Scr
211233
}
212234
}
213235

214-
const moduleNameToSourceData: ScriptDuplication = new Map();
236+
const duplication: ScriptDuplication = new Map();
215237
for (const [script, sourceDataArray] of sourceDatasMap) {
216238
for (const sourceData of sourceDataArray) {
217-
let data = moduleNameToSourceData.get(sourceData.source);
239+
let data = duplication.get(sourceData.source);
218240
if (!data) {
219-
data = [];
220-
moduleNameToSourceData.set(sourceData.source, data);
241+
data = {estimatedDuplicateBytes: 0, duplicates: []};
242+
duplication.set(sourceData.source, data);
221243
}
222-
data.push({
244+
data.duplicates.push({
223245
script,
224-
resourceSize: sourceData.resourceSize,
246+
attributedSize: sourceData.resourceSize,
225247
});
226248
}
227249
}
228250

229-
normalizeDuplication(moduleNameToSourceData);
230-
return moduleNameToSourceData;
251+
normalizeDuplication(duplication);
252+
253+
// Sort by estimated savings.
254+
return new Map([...duplication].sort((a, b) => b[1].estimatedDuplicateBytes - a[1].estimatedDuplicateBytes));
231255
}

front_end/models/trace/insights/BUILD.gn

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ devtools_module("insights") {
1212
"Common.ts",
1313
"DOMSize.ts",
1414
"DocumentLatency.ts",
15-
"DuplicateJavaScript.ts",
15+
"DuplicatedJavaScript.ts",
1616
"FontDisplay.ts",
1717
"ForcedReflow.ts",
1818
"ImageDelivery.ts",

front_end/models/trace/insights/DuplicateJavaScript.ts renamed to front_end/models/trace/insights/DuplicatedJavaScript.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,29 @@ export const UIStrings = {
1919
/**
2020
* @description Title of an insight that identifies multiple copies of the same JavaScript sources, and recommends removing the duplication.
2121
*/
22-
title: 'Duplicate JavaScript',
22+
title: 'Duplicated JavaScript',
2323
/**
2424
* @description Description of an insight that identifies multiple copies of the same JavaScript sources, and recommends removing the duplication.
2525
*/
2626
description:
2727
'Remove large, duplicate JavaScript modules from bundles to reduce unnecessary bytes consumed by network activity.',
28+
/** Label for a column in a data table; entries will be the locations of JavaScript or CSS code, e.g. the name of a Javascript package or module. */
29+
columnSource: 'Source',
30+
/** Label for a column in a data table; entries will be the file size of a web resource in kilobytes. */
31+
columnResourceSize: 'Resource size',
2832
} as const;
2933

30-
const str_ = i18n.i18n.registerUIStrings('models/trace/insights/DuplicateJavaScript.ts', UIStrings);
34+
const str_ = i18n.i18n.registerUIStrings('models/trace/insights/DuplicatedJavaScript.ts', UIStrings);
3135
export const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
3236

3337
export type DuplicateJavaScriptInsightModel = InsightModel<typeof UIStrings, {
3438
duplication: Extras.ScriptDuplication.ScriptDuplication,
39+
scriptsWithDuplication: Handlers.ModelHandlers.Scripts.Script[],
3540
}>;
3641

3742
function finalize(partialModel: PartialInsightModel<DuplicateJavaScriptInsightModel>): DuplicateJavaScriptInsightModel {
38-
const requests = [...partialModel.duplication.values().flatMap(array => array.map(v => v.script.request))].filter(
39-
e => !!e); // eslint-disable-line no-implicit-coercion
43+
const requests = partialModel.scriptsWithDuplication.map(script => script.request)
44+
.filter(e => !!e); // eslint-disable-line no-implicit-coercion
4045

4146
return {
4247
insightKey: InsightKeys.DUPLICATE_JAVASCRIPT,
@@ -65,5 +70,9 @@ export function generateInsight(
6570
});
6671

6772
const duplication = Extras.ScriptDuplication.computeScriptDuplication({scripts});
68-
return finalize({duplication});
73+
const scriptsWithDuplication = [...duplication.values().flatMap(data => data.duplicates.map(d => d.script))];
74+
return finalize({
75+
duplication,
76+
scriptsWithDuplication: [...new Set(scriptsWithDuplication)],
77+
});
6978
}

front_end/models/trace/insights/Models.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
export * as CLSCulprits from './CLSCulprits.js';
66
export * as DocumentLatency from './DocumentLatency.js';
77
export * as DOMSize from './DOMSize.js';
8-
export * as DuplicateJavaScript from './DuplicateJavaScript.js';
8+
export * as DuplicatedJavaScript from './DuplicatedJavaScript.js';
99
export * as FontDisplay from './FontDisplay.js';
1010
export * as ForcedReflow from './ForcedReflow.js';
1111
export * as ImageDelivery from './ImageDelivery.js';

front_end/models/trace/insights/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export const enum InsightKeys {
126126
THIRD_PARTIES = 'ThirdParties',
127127
DOCUMENT_LATENCY = 'DocumentLatency',
128128
DOM_SIZE = 'DOMSize',
129-
DUPLICATE_JAVASCRIPT = 'DuplicateJavaScript',
129+
DUPLICATE_JAVASCRIPT = 'DuplicatedJavaScript',
130130
FONT_DISPLAY = 'FontDisplay',
131131
FORCED_REFLOW = 'ForcedReflow',
132132
IMAGE_DELIVERY = 'ImageDelivery',

0 commit comments

Comments
 (0)