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

Add instrumentation for performance metrics tracking #9035

Merged
merged 14 commits into from
Dec 19, 2019
Merged
5 changes: 3 additions & 2 deletions build/rollup_plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import unassert from 'rollup-plugin-unassert';
import json from 'rollup-plugin-json';
import {terser} from 'rollup-plugin-terser';
import minifyStyleSpec from './rollup_plugin_minify_style_spec';
import strip from '@rollup/plugin-strip';
import {createFilter} from 'rollup-pluginutils';
import strip from '@rollup/plugin-strip';

// Common set of plugins/transformations shared across different rollup
// builds (main mapboxgl bundle, style-spec package, benchmarks bundle)
Expand All @@ -18,7 +18,8 @@ export const plugins = (minified, production) => [
minifyStyleSpec(),
json(),
production ? strip({
functions: ['Debug.*']
sourceMap: true,
functions: ['PerformanceUtils.*', 'Debug.*']
}) : false,
glsl('./src/shaders/*.glsl', production),
buble({transforms: {dangerousForOf: true}, objectAssign: "Object.assign"}),
Expand Down
3 changes: 2 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {isSafari} from './util/util';
import {setRTLTextPlugin, getRTLTextPluginStatus} from './source/rtl_text_plugin';
import WorkerPool from './util/worker_pool';
import {clearTileCache} from './util/tile_request_cache';
import {PerformanceUtils} from './util/performance';

const exported = {
version,
Expand Down Expand Up @@ -133,7 +134,7 @@ const exported = {
};

//This gets automatically stripped out in production builds.
Debug.extend(exported, {isSafari});
Debug.extend(exported, {isSafari, getPerformanceMetrics: PerformanceUtils.getPerformanceMetrics});

/**
* The version of Mapbox GL JS in use as specified in `package.json`,
Expand Down
4 changes: 2 additions & 2 deletions src/source/geojson_worker_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import {getJSON} from '../util/ajax';

import performance from '../util/performance';
import {RequestPerformance} from '../util/performance';
import rewind from '@mapbox/geojson-rewind';
import GeoJSONWrapper from './geojson_wrapper';
import vtpbf from 'vt-pbf';
Expand Down Expand Up @@ -161,7 +161,7 @@ class GeoJSONWorkerSource extends VectorTileWorkerSource {
delete this._pendingLoadDataParams;

const perf = (params && params.request && params.request.collectResourceTiming) ?
new performance.Performance(params.request) : false;
new RequestPerformance(params.request) : false;

this.loadGeoJSON(params, (err: ?Error, data: ?Object) => {
if (err || !data) {
Expand Down
4 changes: 2 additions & 2 deletions src/source/vector_tile_worker_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vt from '@mapbox/vector-tile';
import Protobuf from 'pbf';
import WorkerTile from './worker_tile';
import {extend} from '../util/util';
import performance from '../util/performance';
import {RequestPerformance} from '../util/performance';

import type {
WorkerSource,
Expand Down Expand Up @@ -104,7 +104,7 @@ class VectorTileWorkerSource implements WorkerSource {
this.loading = {};

const perf = (params && params.request && params.request.collectResourceTiming) ?
new performance.Performance(params.request) : false;
new RequestPerformance(params.request) : false;

const workerTile = this.loading[uid] = new WorkerTile(params);
workerTile.abort = this.loadVectorData(params, (err, response) => {
Expand Down
19 changes: 17 additions & 2 deletions src/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import {Event, ErrorEvent} from '../util/evented';
import {MapMouseEvent} from './events';
import TaskQueue from '../util/task_queue';
import webpSupported from '../util/webp_supported';
import {PerformanceMarkers, PerformanceUtils} from '../util/performance';

import {setCacheLimits} from '../util/tile_request_cache';

import type {PointLike} from '@mapbox/point-geometry';
Expand Down Expand Up @@ -279,6 +281,8 @@ class Map extends Camera {
_sourcesDirty: ?boolean;
_placementDirty: ?boolean;
_loaded: boolean;
// accounts for placement finishing as well
_fullyLoaded: boolean;
_trackResize: boolean;
_preserveDrawingBuffer: boolean;
_failIfMajorPerformanceCaveat: boolean;
Expand Down Expand Up @@ -341,6 +345,8 @@ class Map extends Camera {
touchZoomRotate: TouchZoomRotateHandler;

constructor(options: MapOptions) {
PerformanceUtils.mark(PerformanceMarkers.create);

options = extend({}, defaultOptions, options);

if (options.minZoom != null && options.maxZoom != null && options.minZoom > options.maxZoom) {
Expand Down Expand Up @@ -2138,6 +2144,7 @@ class Map extends Camera {

if (this.loaded() && !this._loaded) {
this._loaded = true;
PerformanceUtils.mark(PerformanceMarkers.load);
this.fire(new Event('load'));
}

Expand Down Expand Up @@ -2184,9 +2191,14 @@ class Map extends Camera {
// Even though `_styleDirty` and `_sourcesDirty` are reset in this
// method, synchronous events fired during Style#update or
// Style#_updateSources could have caused them to be set again.
if (this._sourcesDirty || this._repaint || this._styleDirty || this._placementDirty) {
const somethingDirty = this._sourcesDirty || this._styleDirty || this._placementDirty;
if (somethingDirty || this._repaint) {
this.triggerRepaint();
} else if (!this.isMoving() && this.loaded()) {
if (!this._fullyLoaded) {
this._fullyLoaded = true;
PerformanceUtils.mark(PerformanceMarkers.fullLoad);
}
this.fire(new Event('idle'));
}

Expand Down Expand Up @@ -2225,6 +2237,8 @@ class Map extends Camera {
removeNode(this._controlContainer);
removeNode(this._missingCSSCanary);
this._container.classList.remove('mapboxgl-map');

PerformanceUtils.clearMetrics();
this.fire(new Event('remove'));
}

Expand All @@ -2235,7 +2249,8 @@ class Map extends Camera {
*/
triggerRepaint() {
if (this.style && !this._frame) {
this._frame = browser.frame(() => {
this._frame = browser.frame((paintStartTimestamp: number) => {
PerformanceUtils.frame(paintStartTimestamp);
this._frame = null;
this._render();
});
Expand Down
2 changes: 1 addition & 1 deletion src/util/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const exported = {
*/
now,

frame(fn: () => void): Cancelable {
frame(fn: (paintStartTimestamp: number) => void): Cancelable {
const frame = raf(fn);
return {cancel: () => cancel(frame)};
},
Expand Down
1 change: 0 additions & 1 deletion src/util/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ function copyImage(srcImg: *, dstImg: *, srcPt: Point, dstPt: Point, size: Size,
dstData[dstOffset + i] = srcData[srcOffset + i];
}
}

return dstImg;
}

Expand Down
119 changes: 73 additions & 46 deletions src/util/performance.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,74 @@
// @flow

import window from '../util/window';
import type {RequestParameters} from '../util/ajax';

// Wraps performance to facilitate testing
// Not incorporated into browser.js because the latter is poisonous when used outside the main thread
const performanceExists = typeof performance !== 'undefined';
const wrapper = {};
const performance = window.performance;

wrapper.getEntriesByName = (url: string) => {
if (performanceExists && performance && performance.getEntriesByName)
return performance.getEntriesByName(url);
else
return false;
};

wrapper.mark = (name: string) => {
if (performanceExists && performance && performance.mark)
return performance.mark(name);
else
return false;
};

wrapper.measure = (name: string, startMark: string, endMark: string) => {
if (performanceExists && performance && performance.measure)
return performance.measure(name, startMark, endMark);
else
return false;
};
export type PerformanceMetrics = {
loadTime: number,
fullLoadTime: number,
fps: number,
percentDroppedFrames: number
}

wrapper.clearMarks = (name: string) => {
if (performanceExists && performance && performance.clearMarks)
return performance.clearMarks(name);
else
return false;
export const PerformanceMarkers = {
create: 'create',
load: 'load',
fullLoad: 'fullLoad'
};

wrapper.clearMeasures = (name: string) => {
if (performanceExists && performance && performance.clearMeasures)
return performance.clearMeasures(name);
else
return false;
let lastFrameTime = null;
let frameTimes = [];

const minFramerateTarget = 30;
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the tradeoffs between using 30 and 60 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Setting it at seemed to introduce a lot of noise due to a combination of measurement error and being very rarely always locked at 60, so almost every frame ends up missing the target.
The idea is to capture really bad frames in a metric, and half of our ideal seemed like a good start.

const frameTimeTarget = 1000 / minFramerateTarget;

export const PerformanceUtils = {
mark(marker: $Keys<typeof PerformanceMarkers>) {
performance.mark(marker);
},
frame(timestamp: number) {
const currTimestamp = timestamp;
if (lastFrameTime != null) {
const frameTime = currTimestamp - lastFrameTime;
frameTimes.push(frameTime);
}
lastFrameTime = currTimestamp;
},
clearMetrics() {
lastFrameTime = null;
frameTimes = [];
performance.clearMeasures('loadTime');
performance.clearMeasures('fullLoadTime');

for (const marker in PerformanceMarkers) {
performance.clearMarks(PerformanceMarkers[marker]);
}
},
getPerformanceMetrics(): PerformanceMetrics {
const loadTime = performance.measure('loadTime', PerformanceMarkers.create, PerformanceMarkers.load).duration;
const fullLoadTime = performance.measure('fullLoadTime', PerformanceMarkers.create, PerformanceMarkers.fullLoad).duration;
const totalFrames = frameTimes.length;

const avgFrameTime = frameTimes.reduce((prev, curr) => prev + curr, 0) / totalFrames / 1000;
const fps = 1 / avgFrameTime;

// count frames that missed our framerate target
const droppedFrames = frameTimes
.filter((frameTime) => frameTime > frameTimeTarget)
.reduce((acc, curr) => {
return acc + (curr - frameTimeTarget) / frameTimeTarget;
}, 0);
const percentDroppedFrames = (droppedFrames / (totalFrames + droppedFrames)) * 100;

return {
loadTime,
fullLoadTime,
fps,
percentDroppedFrames
};
}
};

/**
Expand All @@ -48,7 +77,7 @@ wrapper.clearMeasures = (name: string) => {
* @param {RequestParameters} request
* @private
*/
class Performance {
export class RequestPerformance {
_marks: {start: string, end: string, measure: string};

constructor (request: RequestParameters) {
Expand All @@ -58,28 +87,26 @@ class Performance {
measure: request.url.toString()
};

wrapper.mark(this._marks.start);
performance.mark(this._marks.start);
}

finish() {
wrapper.mark(this._marks.end);
let resourceTimingData = wrapper.getEntriesByName(this._marks.measure);
performance.mark(this._marks.end);
let resourceTimingData = performance.getEntriesByName(this._marks.measure);

// fallback if web worker implementation of perf.getEntriesByName returns empty
if (resourceTimingData.length === 0) {
wrapper.measure(this._marks.measure, this._marks.start, this._marks.end);
resourceTimingData = wrapper.getEntriesByName(this._marks.measure);
performance.measure(this._marks.measure, this._marks.start, this._marks.end);
resourceTimingData = performance.getEntriesByName(this._marks.measure);

// cleanup
wrapper.clearMarks(this._marks.start);
wrapper.clearMarks(this._marks.end);
wrapper.clearMeasures(this._marks.measure);
performance.clearMarks(this._marks.start);
performance.clearMarks(this._marks.end);
performance.clearMeasures(this._marks.measure);
}

return resourceTimingData;
}
}

wrapper.Performance = Performance;

export default wrapper;
export default performance;
6 changes: 6 additions & 0 deletions src/util/window.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ function restore(): Window {

window.restore = restore;

window.performance.getEntriesByName = function() {};
window.performance.mark = function() {};
window.performance.measure = function() {};
window.performance.clearMarks = function() {};
window.performance.clearMeasures = function() {};

window.ImageData = window.ImageData || function() { return false; };
window.ImageBitmap = window.ImageBitmap || function() { return false; };
window.WebGLFramebuffer = window.WebGLFramebuffer || Object;
Expand Down