[api-major] Only create a StatTimer for pages when enableStats == true (issue 5215)

Unless the debugging tools (i.e. `PDFBug`) are enabled, or the `browsertest` is running, the `PDFPageProxy.stats` aren't actually used for anything.
Rather than initializing unnecessary `StatTimer` instances, we can simply re-use *one* dummy class (with static methods) for every page. Note that by using a dummy `StatTimer` in this way, rather than letting `PDFPageProxy.stats` be undefined, we don't need to guard *every* single stats collection callsite.

Since it wouldn't make much sense to attempt to use `PDFPageProxy.stats` when stat collection is disabled, it was instead changed to a "private" property (i.e. `PDFPageProxy._stats`) and a getter was added for accessing `PDFPageProxy.stats`. This getter will now return `null` when stat collection is disabled, making that case easy to handle.

For benchmarking purposes, the test-suite used to re-create the `StatTimer` after loading/rendering each page. However, modifying properties on various API code from the outside in this way seems very error-prone, and is an anti-pattern that we really should avoid at all cost. Hence the `PDFPageProxy.cleanup` method was modified to accept an optional parameter, which will take care of resetting `this.stats` when necessary, and `test/driver.js` was updated accordingly.

Finally, a tiny bit more validation was added on the viewer side, to ensure that all the code we're attempting to access is defined when handling `PDFPageProxy` stats.
This commit is contained in:
Jonas Jenwald 2017-12-06 16:30:04 +01:00
parent 50b72dec6e
commit 7c5ba9aad5
5 changed files with 59 additions and 19 deletions

View file

@ -463,9 +463,13 @@ function isExternalLinkTargetSet() {
class StatTimer {
constructor(enable = true) {
this.enabled = !!enable;
this.reset();
}
reset() {
this.started = Object.create(null);
this.times = [];
this.enabled = !!enable;
}
time(name) {
@ -513,6 +517,30 @@ class StatTimer {
}
}
/**
* 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() {
throw new Error('Cannot initialize DummyStatTimer.');
}
static reset() {}
static time(name) {}
static timeEnd(name) {}
static toString() {
return '';
}
}
export {
CustomStyle,
RenderingCancelledException,
@ -527,4 +555,5 @@ export {
DOMSVGFactory,
SimpleXMLParser,
StatTimer,
DummyStatTimer,
};