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.

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

new_audit: add script-treemap-data to experimental #11271

Merged
merged 30 commits into from
Oct 6, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
118e461
new_audit: add treemap-data to experimental
connorjclark Aug 15, 2020
c6bed57
remove execution time
connorjclark Aug 15, 2020
bc6c57c
comments
connorjclark Aug 15, 2020
32a5c37
rm script id
connorjclark Aug 15, 2020
4b60ca3
tweaks
connorjclark Aug 15, 2020
a2cfff5
Merge remote-tracking branch 'origin/master' into treemap-data
connorjclark Sep 25, 2020
e6b3eb6
first pass
connorjclark Sep 25, 2020
ac4e8a6
no scriptData
connorjclark Sep 25, 2020
b12967e
reverse ifs
connorjclark Sep 25, 2020
f811334
comments
connorjclark Sep 25, 2020
a39a444
fix case with no map or usage
connorjclark Sep 25, 2020
f28ef76
pr
connorjclark Sep 25, 2020
7008e55
load
connorjclark Sep 26, 2020
1eec2e5
minor test
connorjclark Sep 26, 2020
9c7e3a5
tests
connorjclark Sep 28, 2020
dca7c44
Merge remote-tracking branch 'origin/master' into treemap-data
connorjclark Oct 1, 2020
5d72ee9
pr
connorjclark Oct 1, 2020
8c05fc0
pr
connorjclark Oct 1, 2020
5976c3b
match object
connorjclark Oct 1, 2020
27137af
root node container
connorjclark Oct 1, 2020
1446910
rm resource summary
connorjclark Oct 1, 2020
97e548f
rename
connorjclark Oct 2, 2020
556aa87
update
connorjclark Oct 5, 2020
754ce53
update tests
connorjclark Oct 5, 2020
5e97161
deps: add @types/yargs-parser
connorjclark Oct 5, 2020
d878992
Merge branch 'deps-yargs-parser-types' into treemap-data
connorjclark Oct 5, 2020
bb028dd
Merge remote-tracking branch 'origin/master' into treemap-data
connorjclark Oct 5, 2020
c2d78f1
pr
connorjclark Oct 6, 2020
106e608
Merge remote-tracking branch 'origin/master' into treemap-data
connorjclark Oct 6, 2020
0eac4f6
fixtest
connorjclark Oct 6, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
309 changes: 309 additions & 0 deletions lighthouse-core/audits/treemap-data.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,309 @@
/**
* @license Copyright 2020 Google Inc. All Rights Reserved.
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/**
* @fileoverview
* Creates treemap data for webtreemap.
*/

const Audit = require('./audit.js');
const JsBundles = require('../computed/js-bundles.js');
const UnusedJavaScriptSummary = require('../computed/unused-javascript-summary.js');
const ModuleDuplication = require('../computed/module-duplication.js');
const NetworkRecords = require('../computed/network-records.js');
const ResourceSummary = require('../computed/resource-summary.js');

/**
* @typedef {Record<string, RootNode[]>} TreemapData
*/

/**
* @typedef RootNode
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
* @property {string} name
* @property {Node} node
*/

/**
* @typedef Node
* @property {string} name
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
* @property {number} resourceBytes
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be convinced to use transferBytes instead ..

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resource seems fine for the treemap visualization (though eventually we might want both?), we don't exactly want to minimize the size of these over there :)

* @property {number=} unusedBytes
* @property {number=} executionTime
* @property {string=} duplicate
* @property {Node[]=} children
*/

/**
* @typedef {Omit<Node, 'name'|'children'>} SourceData
*/

class TreemapDataAudit extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'treemap-data',
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
title: 'Treemap Data',
description: 'Used for treemap app.',
requiredArtifacts:
['traces', 'devtoolsLogs', 'SourceMaps', 'ScriptElements', 'JsUsage', 'URL'],
};
}

/**
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
* @param {string} sourceRoot
* @param {Record<string, SourceData>} sourcesData
* @return {Node}
*/
static prepareTreemapNodes(sourceRoot, sourcesData) {
/**
* @param {string} name
* @return {Node}
*/
function newNode(name) {
return {
name,
resourceBytes: 0,
};
}

/**
* Given a slash-delimited path, traverse the Node structure and increment
* the data provided for each node in the chain. Creates nodes as needed.
* Ex: path/to/file.js will find or create "path" on `node`, increment the data fields,
* and continue with "to", and so on.
* @param {string} source
* @param {SourceData} data
* @param {Node} node
*/
function addNode(source, data, node) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
// Strip off the shared root.
const sourcePathSegments = source.replace(sourceRoot, '').split(/\/+/);
sourcePathSegments.forEach((sourcePathSegment, i) => {
const isLastSegment = i === sourcePathSegments.length - 1;

let child = node.children && node.children.find(child => child.name === sourcePathSegment);
if (!child) {
child = newNode(sourcePathSegment);
node.children = node.children || [];
node.children.push(child);
}
node = child;
connorjclark marked this conversation as resolved.
Show resolved Hide resolved

// Now that we've found or created the next node in the path, apply the data.
node.resourceBytes += data.resourceBytes;
if (data.unusedBytes) node.unusedBytes = (node.unusedBytes || 0) + data.unusedBytes;
if (data.duplicate !== undefined && isLastSegment) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
node.duplicate = data.duplicate;
}
});
}

const rootNode = newNode(sourceRoot);

// For every source file, apply the data to all components
// of the source path, creating nodes as necessary.
for (const [source, data] of Object.entries(sourcesData)) {
addNode(source || `<unmapped>`, data, rootNode);

// Apply the data to the rootNode.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've expected the addNode function to handle this case. any reason it's separate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a simple way to reduce the repetition here.

I at least moved it to inside the function.

rootNode.resourceBytes += data.resourceBytes;
if (data.unusedBytes) rootNode.unusedBytes = (rootNode.unusedBytes || 0) + data.unusedBytes;
}

/**
* Collapse nodes that have only one child.
* @param {Node} node
*/
function collapse(node) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
while (node.children && node.children.length === 1) {
node.name += '/' + node.children[0].name;
node.children = node.children[0].children;
}

if (node.children) {
for (const child of node.children) {
collapse(child);
}
}
}
collapse(rootNode);

// TODO(cjamcl): Should this structure be flattened for space savings?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this to be resolved/decided in this PR? how much space savings are we talking here? I'd vote to leave it as is without TODOs unless we run into a problem with the canonical structure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd vote to leave it as is without TODOs unless we run into a problem with the canonical structure

yeah, I'd definitely say do a size check for typical and maybe some pathological pages before landing in default-config, but better to default to the straightforward encoding if it's reasonable in size.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is after removing fullpage screenshots from experimental; and removed resource summary.

coursehero.com, 105 KB aka 22% of .audits
cnn.com, 48 KB aka 6% of .audits

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100K doesn't seem like a showstopper, even if it is suprisingly large just for some file names.

Maybe leave an issue rather than TODO to explore flattening save difference if we ever need some LHR trimming?

// Like DOM Snapshot.
// Less JSON (no super nested children, and no repeated property names).

return rootNode;
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<RootNode[]>}
*/
static async makeJavaScriptRootNodes(artifacts, context) {
/** @type {RootNode[]} */
const rootNodes = [];

const bundles = await JsBundles.request(artifacts, context);
const duplication = await ModuleDuplication.request(artifacts, context);
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
connorjclark marked this conversation as resolved.
Show resolved Hide resolved

/** @type {Array<{src: string, length: number, unusedJavascriptSummary?: import('../computed/unused-javascript-summary.js').Summary}>} */
const scriptData = [];
const inlineScriptData = {
src: artifacts.URL.finalUrl,
length: 0,
};
for (const scriptElement of artifacts.ScriptElements) {
// Normalize ScriptElements so that inline scripts show up as a single entity.
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
if (!scriptElement.src) {
inlineScriptData.length += (scriptElement.content || '').length;
continue;
}

const url = scriptElement.src;
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const bundle = bundles.find(bundle => url === bundle.script.src);
const scriptCoverages = artifacts.JsUsage[url];
if (!bundle || !scriptCoverages) continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when does this happen? If it happens for decent-sized scripts, should they be included in some way still? (even if just an "other" category or whatever). If it's just for type checking or shouldn't really occur, add a comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was accidentally skipping all scripts without a source map. Messed that up during some refactoring.


scriptData.push({
src: scriptElement.src,
length: (scriptElement.content || '').length,
unusedJavascriptSummary:
await UnusedJavaScriptSummary.request({url, scriptCoverages, bundle}, context),
});
}
if (inlineScriptData.length) scriptData.unshift(inlineScriptData);
connorjclark marked this conversation as resolved.
Show resolved Hide resolved

for (const {src, length, unusedJavascriptSummary} of scriptData) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const bundle = bundles.find(bundle => bundle.script.src === src);
const name = src;

let node;
if (bundle && unusedJavascriptSummary && unusedJavascriptSummary.sourcesWastedBytes) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
/** @type {Record<string, SourceData>} */
const sourcesData = {};
for (const source of Object.keys(bundle.sizes.files)) {
/** @type {SourceData} */
const sourceData = {
resourceBytes: bundle.sizes.files[source],
};

if (unusedJavascriptSummary && unusedJavascriptSummary.sourcesWastedBytes) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
sourceData.unusedBytes = unusedJavascriptSummary.sourcesWastedBytes[source];
}

if (duplication) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const key = ModuleDuplication._normalizeSource(source);
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
if (duplication.has(key)) sourceData.duplicate = key;
}

sourcesData[source] = sourceData;
}

node = this.prepareTreemapNodes(bundle.rawMap.sourceRoot || '', sourcesData);
} else if (unusedJavascriptSummary) {
node = {
name,
resourceBytes: unusedJavascriptSummary.totalBytes,
unusedBytes: unusedJavascriptSummary.wastedBytes,
executionTime: 0,
};
} else {
node = {
name,
resourceBytes: length,
unusedBytes: 0,
executionTime: 0,
};
}

rootNodes.push({
name,
node,
});
}

return rootNodes;
}

/**
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<RootNode>}
*/
static async makeResourceSummaryRootNode(artifacts, context) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const networkRecords = await NetworkRecords.request(devtoolsLog, context);

const totalCount = networkRecords.length;
let totalSize = 0;

/** @type {Node[]} */
const children = [];
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
for (const networkRecord of networkRecords) {
const resourceType = ResourceSummary.determineResourceType(networkRecord);

let child = children.find(child => child.name === resourceType);
if (!child) {
child = {
name: resourceType,
resourceBytes: 0,
children: [],
};
children.push(child);
}

totalSize += networkRecord.resourceSize;
child.resourceBytes += networkRecord.resourceSize;

let name = networkRecord.url;
child.children = child.children || [];
child.children.push({
name,
resourceBytes: networkRecord.resourceSize,
});
}

return {
name: 'Resource Summary',
node: {
name: `${totalCount} requests`,
resourceBytes: totalSize,
children,
},
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
/** @type {TreemapData} */
const treemapData = {
scripts: await TreemapDataAudit.makeJavaScriptRootNodes(artifacts, context),
resources: [await TreemapDataAudit.makeResourceSummaryRootNode(artifacts, context)],
};

/** @type {LH.Audit.Details.DebugData} */
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const details = {
type: 'debugdata',
treemapData,
};

return {
score: 1,
details,
};
}
}

module.exports = TreemapDataAudit;
1 change: 1 addition & 0 deletions lighthouse-core/computed/resource-summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class ResourceSummary {
* @return {Record<LH.Budget.ResourceType, ResourceEntry>}
*/
static summarize(networkRecords, mainResourceURL, context) {
/** @type {Record<LH.Budget.ResourceType, ResourceEntry>} */
const resourceSummary = {
'stylesheet': {count: 0, resourceSize: 0, transferSize: 0},
'image': {count: 0, resourceSize: 0, transferSize: 0},
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/config/experimental-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const config = {
extends: 'lighthouse:default',
audits: [
'full-page-screenshot',
'treemap-data',
],
passes: [{
passName: 'defaultPass',
Expand Down
Loading