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

Plots: make measure script more flexible (CLI args) #2183

Merged
merged 30 commits into from
May 31, 2017
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@ lighthouse-cli/types/*.map
lighthouse-core/report/partials/templates/
lighthouse-core/report/templates/*.js

plots/out/
plots/out**
67 changes: 45 additions & 22 deletions plots/ab-screenshot/analyze.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const path = require('path');
const opn = require('opn');
const args = require('yargs').argv;

const runs = args['runs'] || 1;

const Metrics = require('../../lighthouse-core/lib/traces/pwmetrics-events');

const constants = require('../constants');
Expand Down Expand Up @@ -84,25 +86,58 @@ function aggregate(outPathA, outPathB) {
if (!utils.isDir(sitePathB)) {
return;
}
const siteScreenshotsComparison = {
siteName: siteDir,
runA: analyzeSingleRunScreenshots(sitePathA),
runB: analyzeSingleRunScreenshots(sitePathB)
};
results.push(siteScreenshotsComparison);

for (let i = 0; i < runs; i++) {
const runDirA = getRunDir(sitePathA, i);
const runDirB = getRunDir(sitePathB, i);

const runPathA = path.resolve(sitePathA, runDirA);
const runPathB = path.resolve(sitePathB, runDirB);

const lighthouseFileA = path.resolve(runPathA, constants.LIGHTHOUSE_RESULTS_FILENAME);
const lighthouseFileB = path.resolve(runPathB, constants.LIGHTHOUSE_RESULTS_FILENAME);

if (!utils.isFile(lighthouseFileA) || !utils.isFile(lighthouseFileB)) {
continue;
}

const siteScreenshotsComparison = {
siteName: `${siteDir} runA: ${runDirA} runB: ${runDirB}`,
runA: analyzeSingleRunScreenshots(runPathA),
runB: analyzeSingleRunScreenshots(runPathB),
};
results.push(siteScreenshotsComparison);
}
});

return results;
}

/**
* Analyzes the screenshots for the first run of a particular site.
* @param {string} sitePath
* @param {number} runIndex
* @return {string}
*/
function getRunDir(sitePath, runIndex) {
return sortAndFilterRunFolders(fs.readdirSync(sitePath))[runIndex];
}

/**
* @param {!Array<string>} folders
* @return {!Array<string>}
*/
function sortAndFilterRunFolders(folders) {
return folders
.filter(folder => folder !== '.DS_Store')
.sort((a, b) => Number(a) - Number(b));
}

/**
* Analyzes the screenshots for the first run of a particular site.
* @param {string} runPath
* @return {!SingleRunScreenshots}
*/
function analyzeSingleRunScreenshots(sitePath) {
const runDir = sortAndFilterRunFolders(fs.readdirSync(sitePath))[0];
const runPath = path.resolve(sitePath, runDir);
function analyzeSingleRunScreenshots(runPath) {
const lighthouseResultsPath = path.resolve(runPath, constants.LIGHTHOUSE_RESULTS_FILENAME);
const lighthouseResults = JSON.parse(fs.readFileSync(lighthouseResultsPath));

Expand Down Expand Up @@ -152,18 +187,6 @@ function analyzeSingleRunScreenshots(sitePath) {
}
}

/**
* @param {!Array<string>} folders
* @return {!Array<string>}
*/
function sortAndFilterRunFolders(folders) {
return folders
.filter(folder => folder !== '.DS_Store')
.map(folder => Number(folder))
.sort((a, b) => a - b)
.map(folder => folder.toString());
}

/**
* Marks the first screenshot that happens after a particular perf timing.
* @param {SingleRunScreenshots} results
Expand Down
222 changes: 115 additions & 107 deletions plots/measure.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,39 @@
'use strict';

/* eslint-disable no-console */

const fs = require('fs');
Copy link
Member

Choose a reason for hiding this comment

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

no longer neccessary

const path = require('path');
const parseURL = require('url').parse;

const mkdirp = require('mkdirp');
const args = require('yargs')
Copy link
Member

Choose a reason for hiding this comment

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

with the new style of invoking, can you update the readme?

to me, it makes more sense to use the node script.js invocation rather than the npm run one now that we have a CLI. but no big deal.

.wrap(Math.min(process.stdout.columns, 120))
.help('help')
.usage('node $0 [options]')
.example('node $0 -n 3 --sites-path ./sample-sites.json')
.example('node $0 --site https://google.com/')
.example('node $0 --subset')
.describe({
'n': 'Number of runs per site',
'reuse-chrome': 'Reuse the same Chrome instance across all site runs',
'keep-first-run': 'If you use --reuse-chrome, by default the first run results are discarded',
})
.group(
['disable-device-emulation', 'disable-cpu-throttling', 'disable-network-throttling'],
'Chrome DevTools settings:')
Copy link
Member

Choose a reason for hiding this comment

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

let's call this Lighthouse settings:

.boolean(['disable-device-emulation', 'disable-cpu-throttling', 'disable-network-throttling'])
.describe({
'disable-device-emulation': 'Disable Nexus 5X emulation',
'disable-cpu-throttling': 'Disable CPU throttling',
'disable-network-throttling': 'Disable network throttling',
})
.group(['sites-path', 'subset', 'site'], 'Options to specify sites:')
.describe({
'sites-path': 'Include relative path of a json file with urls to run',
'subset': 'Measure a subset of popular sites',
'site': 'Include a specific site url to run',
})
.argv;

const constants = require('./constants.js');
const utils = require('./utils.js');
Expand All @@ -31,135 +59,107 @@ const ChromeLauncher = require('../chrome-launcher/chrome-launcher.js');
const Printer = require('../lighthouse-cli/printer');
const assetSaver = require('../lighthouse-core/lib/asset-saver.js');

const NUMBER_OF_RUNS = 20;

const URLS = [
// Flagship sites
'https://nytimes.com',
'https://flipkart.com',
'http://www.espn.com/',
'https://www.washingtonpost.com/pwa/',

// TTI Tester sites
'https://housing.com/in/buy/real-estate-hyderabad',
'http://www.npr.org/',
'http://www.vevo.com/',
'https://weather.com/',
'https://www.nasa.gov/',
'https://vine.co/',
'http://www.booking.com/',
'http://www.thestar.com.my',
'http://www.58pic.com',
'http://www.dawn.com/',
'https://www.ebs.in/IPS/',

// Sourced from: https://en.wikipedia.org/wiki/List_of_most_popular_websites
// (http://www.alexa.com/topsites)
// Removed adult websites and duplicates (e.g. google int'l websites)
// Also removed sites that don't have significant index pages:
// "t.co", "popads.net", "onclickads.net", "microsoftonline.com", "onclckds.com", "cnzz.com",
// "live.com", "adf.ly", "googleusercontent.com",

'https://google.com',
'https://youtube.com',
'https://facebook.com',
'https://baidu.com',
'https://wikipedia.org',
'https://yahoo.com',
'https://amazon.com',
'http://www.qq.com/',
'https://taobao.com',
'https://vk.com',
'https://twitter.com',
'https://instagram.com',
'http://www.hao123.cn/',
'http://www.sohu.com/',
'https://sina.com.cn',
'https://reddit.com',
'https://linkedin.com',
'https://tmall.com',
'https://weibo.com',
'https://360.cn',
'https://yandex.ru',
'https://ebay.com',
'https://bing.com',
'https://msn.com',
'https://www.sogou.com/',
'https://wordpress.com',
'https://microsoft.com',
'https://tumblr.com',
'https://aliexpress.com',
'https://blogspot.com',
'https://netflix.com',
'https://ok.ru',
'https://stackoverflow.com',
'https://imgur.com',
'https://apple.com',
'http://www.naver.com/',
'https://mail.ru',
'http://www.imdb.com/',
'https://office.com',
'https://github.com',
'https://pinterest.com',
'https://paypal.com',
'http://www.tianya.cn/',
'https://diply.com',
'https://twitch.tv',
'https://adobe.com',
'https://wikia.com',
'https://coccoc.com',
'https://so.com',
'https://fc2.com',
'https://www.pixnet.net/',
'https://dropbox.com',
'https://zhihu.com',
'https://whatsapp.com',
'https://alibaba.com',
'https://ask.com',
'https://bbc.com'
];
const keepFirstRun = args['keep-first-run'] || !args['reuse-chrome'];
Copy link
Member

Choose a reason for hiding this comment

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

you can use camelcase instead of this square bracket dance.

args.keepFirstRun etc. yargs does it for you.

const numberOfRuns = args['n'] || 3;

function getUrls() {
if (args['site']) {
return [args['site']];
}

if (args['sites-path']) {
const sitesPath = path.resolve(__dirname, args['sites-path']);
Copy link
Member

Choose a reason for hiding this comment

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

if (args.sitesPath) {
  return require(path.resolve(__dirname, args.sitesPath));
}

Copy link
Contributor Author

@wwwillchen wwwillchen May 30, 2017

Choose a reason for hiding this comment

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

Yeah - this is nice. I did it the more verbose way because the downside is require() will hold onto these files indefinitely in node's module cache. It's definitely OK now since the sites list are needed for the entire lifetime of the script but if we start loading a lot of different sites files, we'll need to watch out for heap usage.

if (path.extname(sitesPath) === '.json') {
return JSON.parse(fs.readFileSync(sitesPath, 'utf-8'));
} else if (path.extname(sitesPath) === '.js') {
return eval(fs.readFileSync(sitesPath, 'utf-8'));
} else {
throw new Error('Must pass a js or json file to --sites-path');
}
}

if (args['subset']) {
return eval(fs.readFileSync(path.resolve(__dirname, 'sites_subset.js'), 'utf-8'));
}

return eval(fs.readFileSync(path.resolve(__dirname, 'sites.js'), 'utf-8'));
Copy link
Member

Choose a reason for hiding this comment

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

instead. you can set the default on yargs:

.default('sites-path', 'sites.js')

}

const URLS = getUrls();

/**
* Launches Chrome once at the beginning, runs all the analysis,
* and then kills Chrome.
* TODO(chenwilliam): measure the overhead of starting chrome, if it's minimal
* then open a fresh Chrome instance for each run.
*/
function main() {
if (numberOfRuns === 1 && !keepFirstRun) {
console.log('ERROR: You are only doing one run and re-using chrome');
console.log('but did not specify --keep-first-run');
return;
}

if (utils.isDir(constants.OUT_PATH)) {
console.log('ERROR: Found output from previous run at: ', constants.OUT_PATH);
console.log('Please run: npm run clean');
return;
}

return ChromeLauncher.launch({port: 9222})
.then(launcher => {
return runAnalysis()
if (args['reuse-chrome']) {
ChromeLauncher.launch().then(launcher => {
return runAnalysisWithExistingChromeInstances(launcher)
.catch(err => console.error(err))
.then(() => launcher.kill());
});
return;
}
runAnalysisWithNewChromeInstances();
}

main();

/**
* Launches a new Chrome instance for each site run.
* Returns a promise chain that analyzes all the sites n times.
* @return {!Promise}
*/
function runAnalysisWithNewChromeInstances() {
Copy link
Member

Choose a reason for hiding this comment

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

👍

let promise = Promise.resolve();

for (let i = 0; i < numberOfRuns; i++) {
// Averages out any order-dependent effects such as memory pressure
utils.shuffle(URLS);

const id = i.toString();
const isFirstRun = i === 0;
const ignoreRun = keepFirstRun ? false : isFirstRun;
for (const url of URLS) {
promise = promise.then(() => {
return ChromeLauncher.launch().then(launcher => {
return singleRunAnalysis(url, id, launcher, {ignoreRun})
.catch(err => console.error(err))
.then(() => launcher.kill());
})
.catch(err => console.error(err));
});
}
}
return promise;
}

/**
* Reuses existing Chrome instance for all site runs.
* Returns a promise chain that analyzes all the sites n times.
* @param {!Launcher} launcher
* @return {!Promise}
*/
function runAnalysis() {
function runAnalysisWithExistingChromeInstances(launcher) {
let promise = Promise.resolve();

// Running it n + 1 times because the first run is deliberately ignored
// because it has different perf characteristics from subsequent runs
// (e.g. DNS cache which can't be easily reset between runs)
for (let i = 0; i <= NUMBER_OF_RUNS; i++) {
for (let i = 0; i < numberOfRuns; i++) {
// Averages out any order-dependent effects such as memory pressure
utils.shuffle(URLS);

const id = i.toString();
const isFirstRun = i === 0;
const ignoreRun = keepFirstRun ? false : isFirstRun;
for (const url of URLS) {
promise = promise.then(() => singleRunAnalysis(url, id, {ignoreRun: isFirstRun}));
promise = promise.then(() => singleRunAnalysis(url, id, launcher, {ignoreRun}));
}
}
return promise;
Expand All @@ -169,10 +169,11 @@ function runAnalysis() {
* Analyzes a site a single time using lighthouse.
* @param {string} url
* @param {string} id
* @param {!Launcher} launcher
* @param {{ignoreRun: boolean}} options
* @return {!Promise}
*/
function singleRunAnalysis(url, id, {ignoreRun}) {
function singleRunAnalysis(url, id, launcher, {ignoreRun}) {
console.log('Measuring site:', url, 'run:', id);
const parsedURL = parseURL(url);
const urlBasedFilename = sanitizeURL(`${parsedURL.host}-${parsedURL.pathname}`);
Expand All @@ -182,20 +183,27 @@ function singleRunAnalysis(url, id, {ignoreRun}) {
}
const outputPath = path.resolve(runPath, constants.LIGHTHOUSE_RESULTS_FILENAME);
const assetsPath = path.resolve(runPath, 'assets');
return analyzeWithLighthouse(url, outputPath, assetsPath, {ignoreRun});
return analyzeWithLighthouse(launcher, url, outputPath, assetsPath, {ignoreRun});
}

/**
* Runs lighthouse and save the artifacts (not used directly by plots,
* but may be helpful for debugging outlier runs).
* @param {!Launcher} launcher
* @param {string} url
* @param {string} outputPath
* @param {string} assetsPath
* @param {{ignoreRun: boolean}} options
* @return {!Promise}
*/
function analyzeWithLighthouse(url, outputPath, assetsPath, {ignoreRun}) {
const flags = {output: 'json'};
function analyzeWithLighthouse(launcher, url, outputPath, assetsPath, {ignoreRun}) {
const flags = {
Copy link
Member

Choose a reason for hiding this comment

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

if ignoreRun is true then there's no reason to have throttling on. I'd force disable cpu & network throttling in these cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

output: 'json',
disableCpuThrottling: ignoreRun ? true : args['disable-cpu-throttling'],
disableNetworkThrottling: ignoreRun ? true : args['disable-network-throttling'],
disableDeviceEmulation: args['disable-device-emulation'],
port: launcher.port,
};
return lighthouse(url, flags, config)
.then(lighthouseResults => {
if (ignoreRun) {
Expand Down
Loading