Skip to content

Commit

Permalink
feat(perf): Add support for document viewer performance options (#1249)
Browse files Browse the repository at this point in the history
* feat(perf): Add support for document viewer performance options

* feat(perf): Add unit tests

* feat(perf): Address review feedback

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
jstoffan and mergify[bot] authored Sep 8, 2020
1 parent aa7f6f6 commit 4047402
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 21 deletions.
17 changes: 9 additions & 8 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,15 @@ import {
FILE_OPTION_FILE_VERSION_ID,
} from './constants';
import {
VIEWER_EVENT,
DURATION_METRIC,
ERROR_CODE,
PREVIEW_ERROR,
PREVIEW_METRIC,
LOAD_METRIC,
DURATION_METRIC,
PREVIEW_END_EVENT,
PREVIEW_DOWNLOAD_ATTEMPT_EVENT,
PREVIEW_END_EVENT,
PREVIEW_ERROR,
PREVIEW_METRIC,
RENDER_METRIC,
VIEWER_EVENT,
} from './events';
import { getClientLogDetails, getISOTime } from './logUtils';
import './Preview.scss';
Expand Down Expand Up @@ -774,9 +775,9 @@ class Preview extends EventEmitter {
);
}

// Start the preview duration timer when the user starts to perceive preview's load
const tag = Timer.createTag(this.file.id, LOAD_METRIC.previewLoadTime);
Timer.start(tag);
// Start the preview load and render timers when the user starts to perceive preview's load
Timer.start(Timer.createTag(this.file.id, LOAD_METRIC.previewLoadTime));
Timer.start(Timer.createTag(this.file.id, RENDER_METRIC));

// If file version ID is specified, increment retry count if it matches current file version ID
if (fileVersionId) {
Expand Down
6 changes: 4 additions & 2 deletions src/lib/Timer.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,20 @@ class Timer {
*
* @public
* @param {string} tag - Time structure to look up, at this string.
* @return {void}
* @return {Object} The time structure, or undefined if none with that tag
*/
stop(tag) {
const time = this.get(tag);

// The timer has already been stopped, or hasn't started. Don't stop it again.
if (!time || time.start === undefined || time.end !== undefined) {
return;
return undefined;
}

time.end = global.performance.now();
time.elapsed = Math.round(time.end - time.start);

return time;
}

/**
Expand Down
7 changes: 7 additions & 0 deletions src/lib/__tests__/Timer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ describe('lib/Timer', () => {
Timer.stop(tag);
expect(Timer.get(tag).elapsed).to.equal(2); // 5 - 3.5 = 1.5, rounded = 2
});

it('should stop and return the value at the given tag', () => {
sandbox.stub(global.performance, 'now').returns(5);
Timer.times[tag] = { start: 3.5 };

expect(Timer.stop(tag).elapsed).to.equal(2);
});
});

describe('get()', () => {
Expand Down
6 changes: 5 additions & 1 deletion src/lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,14 @@ export const LOAD_METRIC = {
previewLoadTime: 'preview_loading', // Total preview load time. Maps to "value" of load event
previewPreloadEvent: 'preload', // Event name for preview_metrics based on preload times.
};
// Event fired from preview based on when the content was actually shown
export const RENDER_EVENT = 'preview_render';
export const RENDER_METRIC = 'preview_render_metric';

export const DURATION_METRIC = 'preview_duration_metric';
// Event fired from preview with preview duration metrics
export const DURATION_METRIC = 'preview_duration_metric';
export const PREVIEW_END_EVENT = 'preview_end';

// Event fired when the user attempts to download the file
export const PREVIEW_DOWNLOAD_ATTEMPT_EVENT = 'preview_download_attempt';
// Events around download reachability
Expand Down
37 changes: 29 additions & 8 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,14 @@ import {
ICON_THUMBNAILS_TOGGLE,
} from '../../icons/icons';
import { JS, PRELOAD_JS, CSS } from './docAssets';
import { ERROR_CODE, VIEWER_EVENT, LOAD_METRIC, USER_DOCUMENT_THUMBNAIL_EVENTS } from '../../events';
import {
ERROR_CODE,
LOAD_METRIC,
RENDER_EVENT,
RENDER_METRIC,
USER_DOCUMENT_THUMBNAIL_EVENTS,
VIEWER_EVENT,
} from '../../events';
import Timer from '../../Timer';

const CURRENT_PAGE_MAP_KEY = 'doc-current-page-map';
Expand Down Expand Up @@ -657,20 +664,20 @@ class DocBaseViewer extends BaseViewer {
// Disable font faces on IOS 10.3.X
const disableFontFace = Browser.hasFontIssue() || this.getViewerOption('disableFontFace');

// Disable streaming via fetch until performance is improved
const disableStream = true;

// Disable range requests for files smaller than MINIMUM_RANGE_REQUEST_FILE_SIZE (25MB)
const isRangeSupported = size >= RANGE_REQUEST_MINIMUM_SIZE;
// Disable range requests for files smaller than minimum range request size
const isRangeSupported = size >= (this.getViewerOption('rangeMinSize') || RANGE_REQUEST_MINIMUM_SIZE);
const isWatermarked = watermarkInfo && watermarkInfo.is_watermarked;
const disableRange = isWatermarked || !isRangeSupported;

// Use larger chunk sizes because we assume that en-US users have better connections to Box's servers
const rangeChunkSizeDefault = location.locale === 'en-US' ? RANGE_CHUNK_SIZE_US : RANGE_CHUNK_SIZE_NON_US;
const rangeChunkSize = this.getViewerOption('rangeChunkSize') || rangeChunkSizeDefault;

// If range requests are disabled, request the gzip compressed version of the representation
this.encoding = disableRange ? ENCODING_TYPES.GZIP : undefined;
// Disable streaming by default unless it is explicitly enabled via options
const disableStream = this.getViewerOption('disableStream') !== false;

// If range requests and streaming are disabled, request the gzip compressed version of the representation
this.encoding = disableRange && disableStream ? ENCODING_TYPES.GZIP : undefined;

if (this.encoding) {
queryParams[QUERY_PARAM_ENCODING] = this.encoding;
Expand Down Expand Up @@ -1202,6 +1209,20 @@ class DocBaseViewer extends BaseViewer {
this.resize();
}
}

// Fire rendered metric to indicate that the specific page of content the user requested has been shown
if (!this.startPageRendered && (this.startPageNum === pageNumber || this.getCachedPage() === pageNumber)) {
const pageRenderTag = Timer.createTag(this.options.file.id, RENDER_METRIC);
const pageRenderTime = Timer.stop(pageRenderTag);

if (pageRenderTime) {
this.emitMetric({
name: RENDER_EVENT,
data: pageRenderTime.elapsed,
});
this.startPageRendered = true;
}
}
}

/**
Expand Down
44 changes: 42 additions & 2 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
ICON_SEARCH,
ICON_THUMBNAILS_TOGGLE,
} from '../../../icons/icons';
import { VIEWER_EVENT, LOAD_METRIC, USER_DOCUMENT_THUMBNAIL_EVENTS } from '../../../events';
import { LOAD_METRIC, RENDER_EVENT, USER_DOCUMENT_THUMBNAIL_EVENTS, VIEWER_EVENT } from '../../../events';
import Timer from '../../../Timer';

const LOAD_TIMEOUT_MS = 180000; // 3 min timeout
Expand Down Expand Up @@ -1145,12 +1145,20 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
});
});

it('should not disable streaming', () => {
it('should disable streaming by default', () => {
return docBase.initViewer('').then(() => {
expect(stubs.getDocument).to.be.calledWith(sinon.match({ disableStream: true }));
});
});

it('should enable streaming if the proper option is provided', () => {
stubs.getViewerOption.returns(false);

return docBase.initViewer('').then(() => {
expect(stubs.getDocument).to.be.calledWith(sinon.match({ disableStream: false }));
});
});

it('should enable range requests if the file is greater than 25MB', () => {
docBase.options.file.size = 26500000;

Expand All @@ -1177,6 +1185,16 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
});
});

it('should enable range requests if the file is smaller than the provided minimum size', () => {
stubs.getViewerOption.returns(2097152); // 2 MB minimum

docBase.options.file.size = 5242880; // 5 MB file size

return docBase.initViewer('').then(() => {
expect(stubs.getDocument).to.be.calledWith(sinon.match({ disableRange: false }));
});
});

it('should set disableCreateObjectURL to false', () => {
return docBase.initViewer('').then(() => {
expect(stubs.getDocument).to.be.calledWith(sinon.match({ disableCreateObjectURL: false }));
Expand Down Expand Up @@ -1798,10 +1816,13 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
};

docBase.somePageRendered = false;
docBase.startPageRendered = false;
stubs.emit = sandbox.stub(docBase, 'emit');
stubs.emitMetric = sandbox.stub(docBase, 'emitMetric');
stubs.initThumbnails = sandbox.stub(docBase, 'initThumbnails');
stubs.hidePreload = sandbox.stub(docBase, 'hidePreload');
stubs.resize = sandbox.stub(docBase, 'resize');
stubs.stop = sandbox.stub(Timer, 'stop').returns({ elapsed: 1000 });
});

it('should emit the pagerender event', () => {
Expand All @@ -1817,6 +1838,25 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
expect(docBase.zoomControls.setCurrentScale).to.be.calledWith(docBase.pdfViewer.currentScale);
});

it('should emit render metric event for start page if not already emitted', () => {
docBase.pagerenderedHandler(docBase.event);
expect(stubs.emitMetric).to.be.calledWith({
name: RENDER_EVENT,
data: 1000,
});
});

it('should not emit render metric event if it was already emitted', () => {
docBase.startPageRendered = true;
docBase.pagerenderedHandler(docBase.event);
expect(stubs.emitMetric).not.to.be.called;
});

it('should not emit render metric event if rendered page is not start page', () => {
docBase.pagerenderedHandler({ pageNumber: 5 });
expect(stubs.emitMetric).not.to.be.called;
});

it('should hide the preload and init thumbnails if no pages were previously rendered', () => {
docBase.options.enableThumbnailsSidebar = true;
docBase.pagerenderedHandler(docBase.event);
Expand Down

0 comments on commit 4047402

Please sign in to comment.