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

Re-factor StatTimer usage, in src/display/api.js, and remove DummyStatTimer #11271

Merged
merged 4 commits into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 25 additions & 14 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import {
unreachable, warn
} from '../shared/util';
import {
DOMCanvasFactory, DOMCMapReaderFactory, DummyStatTimer, loadScript,
PageViewport, releaseImageResources, RenderingCancelledException, StatTimer
DOMCanvasFactory, DOMCMapReaderFactory, loadScript, PageViewport,
releaseImageResources, RenderingCancelledException, StatTimer
} from './display_utils';
import { FontFaceObject, FontLoader } from './font_loader';
import { apiCompatibilityParams } from './api_compatibility';
Expand Down Expand Up @@ -905,7 +905,7 @@ class PDFPageProxy {
this.pageIndex = pageIndex;
this._pageInfo = pageInfo;
this._transport = transport;
this._stats = (pdfBug ? new StatTimer() : DummyStatTimer);
this._stats = (pdfBug ? new StatTimer() : null);
this._pdfBug = pdfBug;
this.commonObjs = transport.commonObjs;
this.objs = new PDFObjects();
Expand Down Expand Up @@ -995,8 +995,9 @@ class PDFPageProxy {
render({ canvasContext, viewport, intent = 'display', enableWebGL = false,
renderInteractiveForms = false, transform = null, imageLayer = null,
canvasFactory = null, background = null, }) {
const stats = this._stats;
stats.time('Overall');
if (this._stats) {
this._stats.time('Overall');
}

const renderingIntent = (intent === 'print' ? 'print' : 'display');
// If there was a pending destroy, cancel it so no cleanup happens during
Expand Down Expand Up @@ -1029,7 +1030,9 @@ class PDFPageProxy {
lastChunk: false,
};

stats.time('Page Request');
if (this._stats) {
this._stats.time('Page Request');
}
this._pumpOperatorList({
pageIndex: this.pageNumber - 1,
intent: renderingIntent,
Expand Down Expand Up @@ -1060,8 +1063,10 @@ class PDFPageProxy {
} else {
internalRenderTask.capability.resolve();
}
stats.timeEnd('Rendering');
stats.timeEnd('Overall');
if (this._stats) {
this._stats.timeEnd('Rendering');
this._stats.timeEnd('Overall');
}
};

const internalRenderTask = new InternalRenderTask({
Expand Down Expand Up @@ -1094,7 +1099,9 @@ class PDFPageProxy {
complete();
return;
}
stats.time('Rendering');
if (this._stats) {
this._stats.time('Rendering');
}
internalRenderTask.initializeGraphics(transparency);
internalRenderTask.operatorListChanged();
}).catch(complete);
Expand Down Expand Up @@ -1137,7 +1144,9 @@ class PDFPageProxy {
lastChunk: false,
};

this._stats.time('Page Request');
if (this._stats) {
this._stats.time('Page Request');
}
this._pumpOperatorList({
pageIndex: this.pageIndex,
intent: renderingIntent,
Expand Down Expand Up @@ -1259,7 +1268,7 @@ class PDFPageProxy {
});
this.objs.clear();
this.annotationsPromise = null;
if (resetStats && this._stats instanceof StatTimer) {
if (resetStats && this._stats) {
this._stats = new StatTimer();
}
this.pendingCleanup = false;
Expand All @@ -1273,7 +1282,9 @@ class PDFPageProxy {
if (!intentState) {
return; // Rendering was cancelled.
}
this._stats.timeEnd('Page Request');
if (this._stats) {
this._stats.timeEnd('Page Request');
}
// TODO Refactor RenderPageRequest to separate rendering
// and operator list logic
if (intentState.displayReadyCapability) {
Expand Down Expand Up @@ -1404,10 +1415,10 @@ class PDFPageProxy {
}

/**
* @type {Object} Returns page stats, if enabled.
* @type {Object} Returns page stats, if enabled; returns `null` otherwise.
*/
get stats() {
return (this._stats instanceof StatTimer ? this._stats : null);
return this._stats;
}
}

Expand Down
44 changes: 7 additions & 37 deletions src/display/display_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import {
assert, BaseException, CMapCompressionType, isString, removeNullCharacters,
stringToBytes, unreachable, Util, warn
stringToBytes, Util, warn
} from '../shared/util';

const DEFAULT_LINK_REL = 'noopener noreferrer nofollow';
Expand Down Expand Up @@ -380,28 +380,21 @@ function getFilenameFromUrl(url) {
}

class StatTimer {
constructor(enable = true) {
this.enabled = !!enable;
constructor() {
this.started = Object.create(null);
this.times = [];
}

time(name) {
if (!this.enabled) {
return;
}
if (name in this.started) {
warn('Timer is already running for ' + name);
warn(`Timer is already running for ${name}`);
}
this.started[name] = Date.now();
}

timeEnd(name) {
if (!this.enabled) {
return;
}
if (!(name in this.started)) {
warn('Timer has not been started for ' + name);
warn(`Timer has not been started for ${name}`);
}
this.times.push({
'name': name,
Expand All @@ -414,7 +407,7 @@ class StatTimer {

toString() {
// Find the longest name for padding purposes.
let out = '', longest = 0;
let outBuf = [], longest = 0;
for (const time of this.times) {
const name = time.name;
if (name.length > longest) {
Expand All @@ -423,31 +416,9 @@ class StatTimer {
}
for (const time of this.times) {
const duration = time.end - time.start;
out += `${time.name.padEnd(longest)} ${duration}ms\n`;
outBuf.push(`${time.name.padEnd(longest)} ${duration}ms\n`);
}
return out;
}
}

/**
* Helps avoid having to initialize {StatTimer} instances, e.g. one for every
* page, in cases where the collected stats are not actually being used.
* This (dummy) class can thus, since all its methods are `static`, be directly
* shared between multiple call-sites without the need to be initialized first.
*
* NOTE: This must implement the same interface as {StatTimer}.
*/
class DummyStatTimer {
constructor() {
unreachable('Cannot initialize DummyStatTimer.');
}

static time(name) {}

static timeEnd(name) {}

static toString() {
return '';
return outBuf.join('');
}
}

Expand Down Expand Up @@ -593,7 +564,6 @@ export {
DOMCMapReaderFactory,
DOMSVGFactory,
StatTimer,
DummyStatTimer,
isFetchSupported,
isValidFetchUrl,
loadScript,
Expand Down