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.

Sign up for GitHub

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

Jump to bottom

core(tti): update ignorable network requests and start point #5021

Merged
merged 3 commits into from
Apr 25, 2018
Merged

core(tti): update ignorable network requests and start point #5021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02e0e05
core(tti): update ignorable network requests and start point
patrickhulce Apr 23, 2018
d8596ae
feedback
patrickhulce Apr 25, 2018
302a1cc
simpler
patrickhulce Apr 25, 2018
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
23 changes: 17 additions & 6 deletions lighthouse-core/gather/computed/metrics/consistently-interactive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ const LHError = require('../../../lib/errors');
const REQUIRED_QUIET_WINDOW = 5000;
const ALLOWED_CONCURRENT_REQUESTS = 2;

/**
* @fileoverview Computes "Time To Interactive", the time at which the page has loaded critical
* resources and is mostly idle.
* @see https://docs.google.com/document/d/1yE4YWsusi5wVXrnwhR61j-QyjK9tzENIzfxrCjA1NAk/edit#heading=h.yozfsuqcgpc4
*/
class ConsistentlyInteractive extends MetricArtifact {
get name() {
return 'ConsistentlyInteractive';
Expand All @@ -28,7 +33,13 @@ class ConsistentlyInteractive extends MetricArtifact {
*/
static _findNetworkQuietPeriods(networkRecords, traceOfTab) {
const traceEndTsInMs = traceOfTab.timestamps.traceEnd / 1000;
return NetworkRecorder.findNetworkQuietPeriods(networkRecords,
// Ignore records that failed, never finished, or were POST/PUT/etc.
const filteredNetworkRecords = networkRecords.filter(record => {
return record.finished && record.requestMethod === 'GET' && !record.failed &&
// Consider network records that had 4xx/5xx status code as "failed"
record.statusCode < 400;
});
return NetworkRecorder.findNetworkQuietPeriods(filteredNetworkRecords,
ALLOWED_CONCURRENT_REQUESTS, traceEndTsInMs);
}

Expand Down Expand Up @@ -79,11 +90,11 @@ class ConsistentlyInteractive extends MetricArtifact {
* @return {{cpuQuietPeriod: TimePeriod, networkQuietPeriod: TimePeriod, cpuQuietPeriods: Array<TimePeriod>, networkQuietPeriods: Array<TimePeriod>}}
*/
static findOverlappingQuietPeriods(longTasks, networkRecords, traceOfTab) {
const FMPTsInMs = traceOfTab.timestamps.firstMeaningfulPaint / 1000;
const FcpTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000;

/** @type {function(TimePeriod):boolean} */
const isLongEnoughQuietPeriod = period =>
period.end > FMPTsInMs + REQUIRED_QUIET_WINDOW &&
period.end > FcpTsInMs + REQUIRED_QUIET_WINDOW &&
period.end - period.start >= REQUIRED_QUIET_WINDOW;
const networkQuietPeriods = this._findNetworkQuietPeriods(networkRecords, traceOfTab)
.filter(isLongEnoughQuietPeriod);
Expand Down Expand Up @@ -137,8 +148,8 @@ class ConsistentlyInteractive extends MetricArtifact {
*/
computeObservedMetric(data) {
const {traceOfTab, networkRecords} = data;
if (!traceOfTab.timestamps.firstMeaningfulPaint) {
throw new LHError(LHError.errors.NO_FMP);
if (!traceOfTab.timestamps.firstContentfulPaint) {
throw new LHError(LHError.errors.NO_FCP);
}

if (!traceOfTab.timestamps.domContentLoaded) {
Expand All @@ -157,7 +168,7 @@ class ConsistentlyInteractive extends MetricArtifact {

const timestamp = Math.max(
cpuQuietPeriod.start,
traceOfTab.timestamps.firstMeaningfulPaint / 1000,
traceOfTab.timestamps.firstContentfulPaint / 1000,
traceOfTab.timestamps.domContentLoaded / 1000
) * 1000;
const timing = (timestamp - traceOfTab.timestamps.navigationStart) / 1000;
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/lib/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const ERRORS = {

Object.keys(ERRORS).forEach(code => ERRORS[code].code = code);

/** @type {Object<string, LighthouseErrorDefinition>} */
/** @type {Object<keyof ERRORS, LighthouseErrorDefinition>} */
Copy link
Member

Choose a reason for hiding this comment

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

👍

LighthouseError.errors = ERRORS;
module.exports = LighthouseError;

44 changes: 25 additions & 19 deletions lighthouse-core/test/gather/computed/metrics/consistently-interactive-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ function generateNetworkRecords(records, navStart) {
const navStartInMs = navStart / 1000;
return records.map(item => {
return {
failed: item.failed || false,
statusCode: item.statusCode || 200,
requestMethod: item.requestMethod || 'GET',
finished: typeof item.finished === 'undefined' ? true : item.finished,
startTime: (item.start + navStartInMs) / 1000,
endTime: item.end === -1 ? -1 : (item.end + navStartInMs) / 1000,
Expand Down Expand Up @@ -52,9 +55,9 @@ describe('Metrics: TTCI', () => {
describe('#findOverlappingQuietPeriods', () => {
it('should return entire range when no activity is present', () => {
const navigationStart = 220023532;
const firstMeaningfulPaint = 2500 * 1000 + navigationStart;
const firstContentfulPaint = 2500 * 1000 + navigationStart;
const traceEnd = 10000 * 1000 + navigationStart;
const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}};
const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}};

const cpu = [];
const network = generateNetworkRecords([], navigationStart);
Expand All @@ -66,9 +69,9 @@ describe('Metrics: TTCI', () => {

it('should throw when trace ended too soon after FMP', () => {
const navigationStart = 220023532;
const firstMeaningfulPaint = 2500 * 1000 + navigationStart;
const firstContentfulPaint = 2500 * 1000 + navigationStart;
const traceEnd = 5000 * 1000 + navigationStart;
const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}};
const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}};

const cpu = [];
const network = generateNetworkRecords([], navigationStart);
Expand All @@ -80,9 +83,9 @@ describe('Metrics: TTCI', () => {

it('should throw when CPU is quiet but network is not', () => {
const navigationStart = 220023532;
const firstMeaningfulPaint = 2500 * 1000 + navigationStart;
const firstContentfulPaint = 2500 * 1000 + navigationStart;
const traceEnd = 10000 * 1000 + navigationStart;
const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}};
const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}};

const cpu = [];
const network = generateNetworkRecords([
Expand All @@ -99,9 +102,9 @@ describe('Metrics: TTCI', () => {

it('should throw when network is quiet but CPU is not', () => {
const navigationStart = 220023532;
const firstMeaningfulPaint = 2500 * 1000 + navigationStart;
const firstContentfulPaint = 2500 * 1000 + navigationStart;
const traceEnd = 10000 * 1000 + navigationStart;
const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}};
const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}};

const cpu = [
{start: 3000, end: 8000},
Expand All @@ -115,29 +118,32 @@ describe('Metrics: TTCI', () => {
}, /NO.*CPU_IDLE_PERIOD/);
});

it('should handle unfinished network requests', () => {
it('should ignore unnecessary network requests', () => {
const navigationStart = 220023532;
const firstMeaningfulPaint = 2500 * 1000 + navigationStart;
const firstContentfulPaint = 2500 * 1000 + navigationStart;
const traceEnd = 10000 * 1000 + navigationStart;
const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}};
const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}};

const cpu = [];
const network = generateNetworkRecords([
{start: 0, end: -1, finished: false},
{start: 0, end: -1, finished: false},
let network = generateNetworkRecords([
{start: 0, end: -1, finished: false},
{start: 0, end: 11000, failed: true},
{start: 0, end: 11000, requestMethod: 'POST'},
{start: 0, end: 11000, statusCode: 500},
], navigationStart);
// Triple the requests to ensure it's not just the 2-quiet kicking in
network = network.concat(network).concat(network);

assert.throws(() => {
ConsistentlyInteractive.findOverlappingQuietPeriods(cpu, network, traceOfTab);
}, /NO.*NETWORK_IDLE_PERIOD/);
const result = ConsistentlyInteractive.findOverlappingQuietPeriods(cpu, network, traceOfTab);
assert.deepEqual(result.cpuQuietPeriod, {start: 0, end: traceEnd / 1000});
assert.deepEqual(result.networkQuietPeriod, {start: 0, end: traceEnd / 1000});
});

it('should find first overlapping quiet period', () => {
const navigationStart = 220023532;
const firstMeaningfulPaint = 10000 * 1000 + navigationStart;
const firstContentfulPaint = 10000 * 1000 + navigationStart;
const traceEnd = 45000 * 1000 + navigationStart;
const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}};
const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}};

const cpu = [
// quiet period before FMP
Expand Down
1 change: 1 addition & 0 deletions typings/web-inspector.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ declare global {
_resourceSize?: number;

finished: boolean;
requestMethod: string;
statusCode: number;
redirectSource?: {
url: string;
Expand Down