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

feat(perf): Add support for document viewer performance options #1249

Merged
merged 4 commits into from
Sep 8, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
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
35 changes: 27 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,18 @@ 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 pageRenderTimer = Timer.stop(pageRenderTag) || {};
jstoffan marked this conversation as resolved.
Show resolved Hide resolved

this.emitMetric({
name: RENDER_EVENT,
data: pageRenderTimer.elapsed,
});
this.startPageRendered = true;
jstoffan marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
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