Skip to content

Commit

Permalink
PERF-813 Capture diagnostic details (#63)
Browse files Browse the repository at this point in the history
* Assert expected diagnostic detail

* Capture and report diagnostic details

* fix: ?? operator is lower precedence than >= !

* Consolidate diagnostic detail in `lastVisualUpdate` key

* lastVisualUpdate -> lastVisibleChange

* Document new diagnostic detail in README

Co-authored-by: Andrew Hyndman <ahyndman@dropbox.com>
  • Loading branch information
ajhyndman and Andrew Hyndman authored Aug 16, 2022
1 parent cb8fd99 commit 52f33e4
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 17 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ export type Metric = {
detail: {
// if ttvc ignored a stalled network request, this value will be true
didNetworkTimeOut: boolean;

// the most recent visible update
// (this can be either a mutation or a load event target, whichever
// occurred last)
lastVisibleChange?: HTMLElement | TimestampedMutationRecord;
};
};
```
Expand Down
6 changes: 3 additions & 3 deletions src/inViewportImageObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import {Logger} from './util/logger';
export class InViewportImageObserver {
private intersectionObserver: IntersectionObserver;
private imageLoadTimes = new Map<HTMLImageElement | HTMLIFrameElement, number>();
private callback: (timestamp: number) => void;
private callback: (timestamp: number, img: HTMLImageElement | HTMLIFrameElement) => void;

public lastImageLoadTimestamp = 0;

constructor(callback: (timestamp: number) => void) {
constructor(callback: (timestamp: number, img: HTMLElement) => void) {
this.callback = callback;
this.intersectionObserver = new IntersectionObserver(this.intersectionObserverCallback);
}
Expand All @@ -36,7 +36,7 @@ export class InViewportImageObserver {
const timestamp = this.imageLoadTimes.get(img);
if (entry.isIntersecting && timestamp != null) {
Logger.info('InViewportImageObserver.callback()', '::', 'timestamp =', timestamp);
this.callback(timestamp);
this.callback(timestamp, img);
}
this.intersectionObserver.unobserve(img);
this.imageLoadTimes.delete(img);
Expand Down
41 changes: 27 additions & 14 deletions src/visuallyCompleteCalculator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {InViewportMutationObserver} from './inViewportMutationObserver';
import {InViewportMutationObserver, TimestampedMutationRecord} from './inViewportMutationObserver';
import {waitForPageLoad} from './util';
import {requestAllIdleCallback} from './requestAllIdleCallback';
import {InViewportImageObserver} from './inViewportImageObserver';
Expand All @@ -19,6 +19,9 @@ export type Metric = {
detail: {
// if ttvc ignored a stalled network request, this value will be true
didNetworkTimeOut: boolean;

// the most recent visual update; this can be either a mutation or a load event target
lastVisibleChange?: HTMLElement | TimestampedMutationRecord;
};
};

Expand All @@ -35,8 +38,9 @@ class VisuallyCompleteCalculator {
private inViewportImageObserver: InViewportImageObserver;

// measurement state
private lastMutationTimestamp = 0;
private lastImageLoadTimestamp = 0;
private lastMutation?: TimestampedMutationRecord;
private lastImageLoadTimestamp = -1;
private lastImageLoadTarget?: HTMLElement;
private subscribers = new Set<MetricSubscriber>();
private navigationCount = 0;

Expand All @@ -58,14 +62,17 @@ class VisuallyCompleteCalculator {
throw new Error('VisuallyCompleteCalculator: This browser/runtime is not supported.');
}

this.inViewportMutationObserver = new InViewportMutationObserver(
(mutation) =>
(this.lastMutationTimestamp = Math.max(this.lastMutationTimestamp, mutation.timestamp ?? 0))
);
this.inViewportImageObserver = new InViewportImageObserver(
(timestamp) =>
(this.lastImageLoadTimestamp = Math.max(this.lastImageLoadTimestamp, timestamp))
);
this.inViewportMutationObserver = new InViewportMutationObserver((mutation) => {
if ((mutation.timestamp ?? 0) >= (this.lastMutation?.timestamp ?? 0)) {
this.lastMutation = mutation;
}
});
this.inViewportImageObserver = new InViewportImageObserver((timestamp, img) => {
if (timestamp >= this.lastImageLoadTimestamp) {
this.lastImageLoadTimestamp = timestamp;
this.lastImageLoadTarget = img;
}
});
}

/** begin measuring a new navigation */
Expand Down Expand Up @@ -100,14 +107,20 @@ class VisuallyCompleteCalculator {

if (!shouldCancel) {
// identify timestamp of last visible change
const end = Math.max(start, this.lastImageLoadTimestamp, this.lastMutationTimestamp);
const end = Math.max(start, this.lastImageLoadTimestamp, this.lastMutation?.timestamp ?? 0);

// report result to subscribers
this.next({
start,
end,
duration: end - start,
detail: {didNetworkTimeOut},
detail: {
didNetworkTimeOut,
lastVisibleChange:
this.lastImageLoadTimestamp > (this.lastMutation?.timestamp ?? 0)
? this.lastImageLoadTarget
: this.lastMutation,
},
});
}

Expand Down Expand Up @@ -141,7 +154,7 @@ class VisuallyCompleteCalculator {
'lastImageLoadTimestamp =',
this.lastImageLoadTimestamp,
'lastMutationTimestamp =',
this.lastMutationTimestamp
this.lastMutation?.timestamp ?? 0
);
Logger.info('TTVC:', measurement);
this.subscribers.forEach((subscriber) => subscriber(measurement));
Expand Down
9 changes: 9 additions & 0 deletions test/e2e/output1/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<head>
<script src="/dist/index.min.js"></script>
<script src="/analytics.js"></script>
</head>

<body>
<h1 id="h1">Hello world!</h1>
<img src="/150.png?delay=500" />
</body>
39 changes: 39 additions & 0 deletions test/e2e/output1/index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import {test, expect} from '@playwright/test';

import {FUDGE} from '../../util/constants';
import {getEntries} from '../../util/entries';

const PAGELOAD_DELAY = 200;
const IMAGE_DELAY = 500;

test.describe('TTVC', () => {
test('output reports timing data', async ({page}) => {
await page.goto(`/test/output1?delay=${PAGELOAD_DELAY}`, {
waitUntil: 'networkidle',
});

const entries = await getEntries(page);

expect(entries.length).toBe(1);
expect(entries[0].duration).toBeGreaterThanOrEqual(PAGELOAD_DELAY + IMAGE_DELAY);
expect(entries[0].duration).toBeLessThanOrEqual(PAGELOAD_DELAY + IMAGE_DELAY + FUDGE);
expect(entries[0].start).toBeDefined();
expect(entries[0].end).toBeDefined();
});

test('output includes detail', async ({page}) => {
await page.goto(`/test/output1?delay=${PAGELOAD_DELAY}`, {
waitUntil: 'networkidle',
});

const entries = await getEntries(page);

expect(entries.length).toBe(1);
expect(entries[0].detail).toBeDefined();
expect(entries[0].detail.didNetworkTimeOut).toBe(false);
const isImageElement = await page.evaluate(() => {
return window.entries[0].detail.lastVisibleChange instanceof HTMLImageElement;
});
expect(isImageElement).toBe(true);
});
});
8 changes: 8 additions & 0 deletions test/e2e/output2/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<head>
<script src="/dist/index.min.js"></script>
<script src="/analytics.js"></script>
</head>

<body>
<h1 id="h1">Hello world!</h1>
</body>
39 changes: 39 additions & 0 deletions test/e2e/output2/index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import {test, expect} from '@playwright/test';

import {FUDGE} from '../../util/constants';
import {getEntries} from '../../util/entries';

const PAGELOAD_DELAY = 200;

test.describe('TTVC', () => {
test('output reports timing data', async ({page}) => {
await page.goto(`/test/output2?delay=${PAGELOAD_DELAY}`, {
waitUntil: 'networkidle',
});

const entries = await getEntries(page);

expect(entries.length).toBe(1);
expect(entries[0].duration).toBeGreaterThanOrEqual(PAGELOAD_DELAY);
expect(entries[0].duration).toBeLessThanOrEqual(PAGELOAD_DELAY + FUDGE);
expect(entries[0].start).toBeDefined();
expect(entries[0].end).toBeDefined();
});

test('output includes detail', async ({page}) => {
await page.goto(`/test/output2?delay=${PAGELOAD_DELAY}`, {
waitUntil: 'networkidle',
});

const entries = await getEntries(page);

expect(entries.length).toBe(1);
expect(entries[0].detail).toBeDefined();
expect(entries[0].detail.didNetworkTimeOut).toBe(false);

const isMutationRecord = await page.evaluate(() => {
return window.entries[0].detail.lastVisibleChange instanceof MutationRecord;
});
expect(isMutationRecord).toBe(true);
});
});

0 comments on commit 52f33e4

Please sign in to comment.