Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Re-factor resetting of StatTimer instances to fix completely broken benchmarking (PR 9245 follow-up)

Browse Source 
It turns out that PR 9245 unfortunately broke benchmarking completely, sorry about that!

The bug is that we were attempting to reset the current instance of `StatTimer`, instead of creating a new one as was previously done. By resetting the current instance, the `StatTimer` data fetched in `test/driver.js` is now wiped out since it points to the *same* underlying object.
This re-use of a `StatTimer` instance was asked for during review, and unfortunately I didn't test this thoroughly enough before submitting the final version of the PR.[1]

---

[1] Note that while I did test the benchmarking scripts with that PR *before* initially submitting it, I did however forget to do that after addressing the review comments which might explain why this problem went unnoticed.
This commit is contained in:
Jonas Jenwald 2018-01-25 19:27:34 +01:00
parent b6c57d9088
commit d33c763dd5
2 changed files with 2 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/display/api.js
View File

@ -1056,8 +1056,8 @@ var PDFPageProxy = (function PDFPageProxyClosure() {
}, this); }, this);
this.objs.clear(); this.objs.clear();
this.annotationsPromise = null; this.annotationsPromise = null;
if (resetStats) { if (resetStats && this._stats instanceof StatTimer) {
this._stats.reset(); this._stats = new StatTimer();
} }
this.pendingCleanup = false; this.pendingCleanup = false;
}, },

6
src/display/dom_utils.js
View File

@ -411,10 +411,6 @@ function isExternalLinkTargetSet() {
class StatTimer { class StatTimer {
constructor(enable = true) { constructor(enable = true) {
this.enabled = !!enable; this.enabled = !!enable;
this.reset();
}
reset() {
this.started = Object.create(null); this.started = Object.create(null);
this.times = []; this.times = [];
} }
@ -477,8 +473,6 @@ class DummyStatTimer {
unreachable('Cannot initialize DummyStatTimer.'); unreachable('Cannot initialize DummyStatTimer.');
} }
static reset() {}
static time(name) {} static time(name) {}
static timeEnd(name) {} static timeEnd(name) {}