Commit 9c96e1fe authored by Robert Knight's avatar Robert Knight

Optimize perceived loading time by varying /api/search page size

When fetching annotations from the backend there is a trade-off between
the time taken to generate a page (less if the page size is smaller) and
the time taken to fetch all pages (less if the page size is larger).

To optimize the perceived loading time we can pick a smaller page size
for the first page, enabling the first screenful of annotations/threads
to appear quickly, and then use a larger page size for the remaining
pages.

 - Change the `pageSize` option for `SearchClient` from a fixed number
   to a callback that returns the page size for a given page index

 - Set the default value for the `pageSize` option to use a small page
   size (50) for the first page and a larger size (200) for remaining
   pages
parent 0727d13b
...@@ -11,6 +11,19 @@ import { TinyEmitter } from 'tiny-emitter'; ...@@ -11,6 +11,19 @@ import { TinyEmitter } from 'tiny-emitter';
* @typedef {'asc'|'desc'} SortBy * @typedef {'asc'|'desc'} SortBy
*/ */
/**
* Default callback used to get the page size for iterating through annotations.
*
* This uses a small number for the first page to reduce the time until some
* results are displayed and a larger number for remaining pages to lower the
* total fetch time.
*
* @param {number} index
*/
function defaultPageSize(index) {
return index === 0 ? 50 : 200;
}
/** /**
* Client for the Hypothesis search API [1] * Client for the Hypothesis search API [1]
* *
...@@ -22,7 +35,13 @@ export default class SearchClient extends TinyEmitter { ...@@ -22,7 +35,13 @@ export default class SearchClient extends TinyEmitter {
/** /**
* @param {(query: SearchQuery) => Promise<SearchResult>} searchFn - Function for querying the search API * @param {(query: SearchQuery) => Promise<SearchResult>} searchFn - Function for querying the search API
* @param {Object} options * @param {Object} options
* @param {number} [options.pageSize] - page size to use when fetching results * @param {(index: number) => number} [options.pageSize] - Callback that returns
* the page size to use when fetching the index'th page of results.
* Callers can vary this to balance the latency of getting some results
* against the time taken to fetch all results.
*
* The returned page size must be at least 1 and no more than the maximum
* value of the `limit` query param for the search API.
* @param {boolean} [options.separateReplies] - When `true`, request that * @param {boolean} [options.separateReplies] - When `true`, request that
* top-level annotations and replies be returned separately. * top-level annotations and replies be returned separately.
* NOTE: This has issues with annotations that have large numbers of * NOTE: This has issues with annotations that have large numbers of
...@@ -42,7 +61,7 @@ export default class SearchClient extends TinyEmitter { ...@@ -42,7 +61,7 @@ export default class SearchClient extends TinyEmitter {
constructor( constructor(
searchFn, searchFn,
{ {
pageSize = 200, pageSize = defaultPageSize,
separateReplies = true, separateReplies = true,
incremental = true, incremental = true,
maxResults = null, maxResults = null,
...@@ -72,11 +91,14 @@ export default class SearchClient extends TinyEmitter { ...@@ -72,11 +91,14 @@ export default class SearchClient extends TinyEmitter {
* @param {string} [searchAfter] - Cursor value to use when paginating * @param {string} [searchAfter] - Cursor value to use when paginating
* through results. Omitted for the first page. See docs for `search_after` * through results. Omitted for the first page. See docs for `search_after`
* query param for /api/search API. * query param for /api/search API.
* @param {number} [pageIndex]
*/ */
async _getPage(query, searchAfter) { async _getPage(query, searchAfter, pageIndex = 0) {
const pageSize = this._pageSize(pageIndex);
/** @type {SearchQuery} */ /** @type {SearchQuery} */
const searchQuery = { const searchQuery = {
limit: this._pageSize, limit: pageSize,
sort: this._sortBy, sort: this._sortBy,
order: this._sortOrder, order: this._sortOrder,
_separate_replies: this._separateReplies, _separate_replies: this._separateReplies,
...@@ -128,7 +150,7 @@ export default class SearchClient extends TinyEmitter { ...@@ -128,7 +150,7 @@ export default class SearchClient extends TinyEmitter {
} }
// If the current page was full, there might be additional pages available. // If the current page was full, there might be additional pages available.
const nextPageAvailable = page.length === this._pageSize; const nextPageAvailable = page.length === pageSize;
// Get the cursor for the start of the next page. This is the last // Get the cursor for the start of the next page. This is the last
// value for whatever field results are sorted by from the current page. // value for whatever field results are sorted by from the current page.
...@@ -136,7 +158,7 @@ export default class SearchClient extends TinyEmitter { ...@@ -136,7 +158,7 @@ export default class SearchClient extends TinyEmitter {
page.length > 0 ? page[page.length - 1][this._sortBy] : null; page.length > 0 ? page[page.length - 1][this._sortBy] : null;
if (nextPageAvailable && nextSearchAfter) { if (nextPageAvailable && nextSearchAfter) {
this._getPage(query, nextSearchAfter); this._getPage(query, nextSearchAfter, pageIndex + 1);
} else { } else {
if (!this._incremental) { if (!this._incremental) {
this.emit('results', this._results); this.emit('results', this._results);
......
...@@ -68,7 +68,7 @@ describe('SearchClient', () => { ...@@ -68,7 +68,7 @@ describe('SearchClient', () => {
}); });
it('fetches pages of results for a single URI', async () => { it('fetches pages of results for a single URI', async () => {
const client = new SearchClient(fakeSearchFn, { pageSize: 3 }); const client = new SearchClient(fakeSearchFn, { pageSize: () => 3 });
client.get({ uri: 'http://example.com' }); client.get({ uri: 'http://example.com' });
await awaitEvent(client, 'end'); await awaitEvent(client, 'end');
...@@ -115,7 +115,7 @@ describe('SearchClient', () => { ...@@ -115,7 +115,7 @@ describe('SearchClient', () => {
}); });
it('emits "end" only once', done => { it('emits "end" only once', done => {
const client = new SearchClient(fakeSearchFn, { pageSize: 2 }); const client = new SearchClient(fakeSearchFn, { pageSize: () => 2 });
client.on('results', sinon.stub()); client.on('results', sinon.stub());
let emitEndCounter = 0; let emitEndCounter = 0;
client.on('end', () => { client.on('end', () => {
...@@ -127,7 +127,7 @@ describe('SearchClient', () => { ...@@ -127,7 +127,7 @@ describe('SearchClient', () => {
}); });
it('emits "results" with pages in incremental mode', async () => { it('emits "results" with pages in incremental mode', async () => {
const client = new SearchClient(fakeSearchFn, { pageSize: 2 }); const client = new SearchClient(fakeSearchFn, { pageSize: () => 2 });
const onResults = sinon.stub(); const onResults = sinon.stub();
client.on('results', onResults); client.on('results', onResults);
...@@ -139,7 +139,7 @@ describe('SearchClient', () => { ...@@ -139,7 +139,7 @@ describe('SearchClient', () => {
}); });
it('emits "resultCount" only once in incremental mode', async () => { it('emits "resultCount" only once in incremental mode', async () => {
const client = new SearchClient(fakeSearchFn, { pageSize: 2 }); const client = new SearchClient(fakeSearchFn, { pageSize: () => 2 });
const onResultCount = sinon.stub(); const onResultCount = sinon.stub();
client.on('resultCount', onResultCount); client.on('resultCount', onResultCount);
...@@ -152,7 +152,7 @@ describe('SearchClient', () => { ...@@ -152,7 +152,7 @@ describe('SearchClient', () => {
it('emits "results" once in non-incremental mode', async () => { it('emits "results" once in non-incremental mode', async () => {
const client = new SearchClient(fakeSearchFn, { const client = new SearchClient(fakeSearchFn, {
pageSize: 2, pageSize: () => 2,
incremental: false, incremental: false,
}); });
const onResults = sinon.stub(); const onResults = sinon.stub();
...@@ -280,7 +280,7 @@ describe('SearchClient', () => { ...@@ -280,7 +280,7 @@ describe('SearchClient', () => {
].forEach(({ sortBy, sortOrder, expectedSearchAfter }) => { ].forEach(({ sortBy, sortOrder, expectedSearchAfter }) => {
it('sets correct "search_after" query parameter depending on `sortBy` and `sortOrder`', async () => { it('sets correct "search_after" query parameter depending on `sortBy` and `sortOrder`', async () => {
const client = new SearchClient(fakeSearchFn, { const client = new SearchClient(fakeSearchFn, {
pageSize: 2, pageSize: () => 2,
sortBy, sortBy,
sortOrder, sortOrder,
}); });
...@@ -294,4 +294,21 @@ describe('SearchClient', () => { ...@@ -294,4 +294,21 @@ describe('SearchClient', () => {
assert.deepEqual(searchAfterParams, expectedSearchAfter); assert.deepEqual(searchAfterParams, expectedSearchAfter);
}); });
}); });
it('fetches pages in sizes specified by `pageSize` callback', async () => {
const pageSizes = [1, 2, 10];
const pageSizeCallback = sinon.spy(index => pageSizes[index]);
const client = new SearchClient(fakeSearchFn, {
pageSize: pageSizeCallback,
});
client.get({ uri: 'http://example.com' });
await awaitEvent(client, 'end');
const limitParams = fakeSearchFn.getCalls().map(call => call.args[0].limit);
assert.deepEqual(limitParams, pageSizes);
const pageIndexes = pageSizeCallback.getCalls().map(call => call.args[0]);
assert.deepEqual(pageIndexes, [0, 1, 2]);
});
}); });
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment