From a4eed2f8eff2e96a7c8a0eb09dda7c023f2c726a Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Fri, 13 Apr 2018 15:45:36 -0400 Subject: [PATCH] turn LRUCache into tile-specific TileCache LRUCache was only being used for tiles and recently we started adding some non-standard behaviour including storing multiple values for a single key. This pr: - replaces `number` key with `OverscaledTileID` so that we can enforce the type and enforce tile id wrapping - moves expiry logic into cache and gets rid of `cacheTimers` in `SourceCache` --- src/source/source_cache.js | 48 ++----- src/source/tile_cache.js | 184 ++++++++++++++++++++++++++ src/util/lru_cache.js | 165 ----------------------- test/unit/source/source_cache.test.js | 15 +-- test/unit/source/tile_cache.test.js | 139 +++++++++++++++++++ test/unit/util/lru_cache.test.js | 107 --------------- 6 files changed, 338 insertions(+), 320 deletions(-) create mode 100644 src/source/tile_cache.js delete mode 100644 src/util/lru_cache.js create mode 100644 test/unit/source/tile_cache.test.js delete mode 100644 test/unit/util/lru_cache.test.js diff --git a/src/source/source_cache.js b/src/source/source_cache.js index 36a69738c75..fbd49c80088 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -4,7 +4,7 @@ import { create as createSource } from './source'; import Tile from './tile'; import { Event, ErrorEvent, Evented } from '../util/evented'; -import Cache from '../util/lru_cache'; +import TileCache from './tile_cache'; import Coordinate from '../geo/coordinate'; import { keysDifference } from '../util/util'; import EXTENT from '../data/extent'; @@ -44,7 +44,7 @@ class SourceCache extends Evented { _sourceErrored: boolean; _tiles: {[any]: Tile}; _prevLng: number | void; - _cache: Cache; + _cache: TileCache; _timers: {[any]: TimeoutID}; _cacheTimers: {[any]: TimeoutID}; _maxTileCacheSize: ?number; @@ -86,7 +86,7 @@ class SourceCache extends Evented { this._source = createSource(id, options, dispatcher, this); this._tiles = {}; - this._cache = new Cache(0, this._unloadTile.bind(this)); + this._cache = new TileCache(0, this._unloadTile.bind(this)); this._timers = {}; this._cacheTimers = {}; this._maxTileCacheSize = null; @@ -208,7 +208,7 @@ class SourceCache extends Evented { return; } - this._resetCache(); + this._cache.reset(); for (const i in this._tiles) { this._reloadTile(i, 'reloading'); @@ -366,9 +366,9 @@ class SourceCache extends Evented { retain[id] = parent; return tile; } - if (this._cache.has(id)) { + if (this._cache.has(parent)) { retain[id] = parent; - return this._cache.get(id); + return this._cache.get(parent); } } } @@ -616,13 +616,9 @@ class SourceCache extends Evented { return tile; - tile = this._cache.getAndRemove((tileID.wrapped().key: any)); + tile = this._cache.getAndRemove(tileID); if (tile) { - if (this._cacheTimers[tileID.key]) { - clearTimeout(this._cacheTimers[tileID.key]); - delete this._cacheTimers[tileID.key]; - this._setTileReloadTimer(tileID.key, tile); - } + this._setTileReloadTimer(tileID.key, tile); // set the tileID because the cached tile could have had a different wrap value tile.tileID = tileID; } @@ -658,21 +654,6 @@ class SourceCache extends Evented { } } - _setCacheInvalidationTimer(id: string | number, tile: Tile) { - if (id in this._cacheTimers) { - clearTimeout(this._cacheTimers[id]); - delete this._cacheTimers[id]; - } - - const expiryTimeout = tile.getExpiryTimeout(); - if (expiryTimeout) { - this._cacheTimers[id] = setTimeout(() => { - this._cache.remove((id: any)); - delete this._cacheTimers[id]; - }, expiryTimeout); - } - } - /** * Remove a tile, given its id, from the pyramid * @private @@ -693,10 +674,7 @@ class SourceCache extends Evented { return; if (tile.hasData()) { - tile.tileID = tile.tileID.wrapped(); - const wrappedId = tile.tileID.key; - this._cache.add((wrappedId: any), tile); - this._setCacheInvalidationTimer(wrappedId, tile); + this._cache.add(tile.tileID, tile, tile.getExpiryTimeout()); } else { tile.aborted = true; this._abortTile(tile); @@ -714,14 +692,6 @@ class SourceCache extends Evented { for (const id in this._tiles) this._removeTile(id); - this._resetCache(); - } - - _resetCache() { - for (const id in this._cacheTimers) - clearTimeout(this._cacheTimers[id]); - - this._cacheTimers = {}; this._cache.reset(); } diff --git a/src/source/tile_cache.js b/src/source/tile_cache.js new file mode 100644 index 00000000000..97d3b2de4c8 --- /dev/null +++ b/src/source/tile_cache.js @@ -0,0 +1,184 @@ +// @flow + +import { OverscaledTileID } from './tile_id'; +import type Tile from './tile'; + +/** + * A [least-recently-used cache](http://en.wikipedia.org/wiki/Cache_algorithms) + * with hash lookup made possible by keeping a list of keys in parallel to + * an array of dictionary of values + * + * @private + */ +class TileCache { + max: number; + data: {[key: number | string]: Array<{ value: Tile, timeout: ?TimeoutID}>}; + order: Array; + onRemove: (element: Tile) => void; + /** + * @param {number} max number of permitted values + * @param {Function} onRemove callback called with items when they expire + */ + constructor(max: number, onRemove: (element: Tile) => void) { + this.max = max; + this.onRemove = onRemove; + this.reset(); + } + + /** + * Clear the cache + * + * @returns {TileCache} this cache + * @private + */ + reset() { + for (const key in this.data) { + for (const removedData of this.data[key]) { + if (removedData.timeout) clearTimeout(removedData.timeout); + this.onRemove(removedData.value); + } + } + + this.data = {}; + this.order = []; + + return this; + } + + /** + * Add a key, value combination to the cache, trimming its size if this pushes + * it over max length. + * + * @param {OverscaledTileID} tileID lookup key for the item + * @param {*} data any value + * + * @returns {TileCache} this cache + * @private + */ + add(tileID: OverscaledTileID, data: Tile, expiryTimeout: number | void) { + const key = tileID.wrapped().key; + if (this.data[key] === undefined) { + this.data[key] = []; + } + + const dataWrapper = { + value: data, + timeout: undefined + }; + + if (expiryTimeout !== undefined) { + dataWrapper.timeout = setTimeout(() => { + this.remove(tileID, dataWrapper); + }, expiryTimeout); + } + + this.data[key].push(dataWrapper); + this.order.push(key); + + if (this.order.length > this.max) { + const removedData = this._getAndRemoveByKey(this.order[0]); + if (removedData) this.onRemove(removedData); + } + + return this; + } + + /** + * Determine whether the value attached to `key` is present + * + * @param {OverscaledTileID} tileID the key to be looked-up + * @returns {boolean} whether the cache has this value + * @private + */ + has(tileID: OverscaledTileID): boolean { + return tileID.wrapped().key in this.data; + } + + /** + * Get the value attached to a specific key and remove data from cache. + * If the key is not found, returns `null` + * + * @param {OverscaledTileID} tileID the key to look up + * @returns {*} the data, or null if it isn't found + * @private + */ + getAndRemove(tileID: OverscaledTileID): ?Tile { + if (!this.has(tileID)) { return null; } + return this._getAndRemoveByKey(tileID.wrapped().key); + } + + /* + * Get and remove the value with the specified key. + */ + _getAndRemoveByKey(key: number): ?Tile { + const data = this.data[key].shift(); + if (data.timeout) clearTimeout(data.timeout); + + if (this.data[key].length === 0) { + delete this.data[key]; + } + this.order.splice(this.order.indexOf(key), 1); + + return data.value; + } + + /** + * Get the value attached to a specific key without removing data + * from the cache. If the key is not found, returns `null` + * + * @param {OverscaledTileID} tileID the key to look up + * @returns {*} the data, or null if it isn't found + * @private + */ + get(tileID: OverscaledTileID): ?Tile { + if (!this.has(tileID)) { return null; } + + const data = this.data[tileID.wrapped().key][0]; + return data.value; + } + + /** + * Remove a key/value combination from the cache. + * + * @param {OverscaledTileID} tileID the key for the pair to delete + * @param {Tile} value If a value is provided, remove that exact version of the value. + * @returns {TileCache} this cache + * @private + */ + remove(tileID: OverscaledTileID, value: ?{ value: Tile, timeout: ?TimeoutID}) { + if (!this.has(tileID)) { return this; } + const key = tileID.wrapped().key; + + const dataIndex = value === undefined ? 0 : this.data[key].indexOf(value); + const data = this.data[key][dataIndex]; + this.data[key].splice(dataIndex, 1); + if (data.timeout) clearTimeout(data.timeout); + if (this.data[key].length === 0) { + delete this.data[key]; + } + this.onRemove(data.value); + this.order.splice(this.order.indexOf(key), 1); + + return this; + } + + /** + * Change the max size of the cache. + * + * @param {number} max the max size of the cache + * @returns {TileCache} this cache + * @private + */ + setMaxSize(max: number): TileCache { + this.max = max; + + while (this.order.length > this.max) { + const removedData = this._getAndRemoveByKey(this.order[0]); + if (removedData) this.onRemove(removedData); + } + + return this; + } +} + +export default TileCache; diff --git a/src/util/lru_cache.js b/src/util/lru_cache.js deleted file mode 100644 index 6d05725eaad..00000000000 --- a/src/util/lru_cache.js +++ /dev/null @@ -1,165 +0,0 @@ -// @flow - -/** - * A [least-recently-used cache](http://en.wikipedia.org/wiki/Cache_algorithms) - * with hash lookup made possible by keeping a list of keys in parallel to - * an array of dictionary of values - * - * @private - */ -class LRUCache { - max: number; - data: {[key: string]: Array}; - order: Array; - onRemove: (element: T) => void; - /** - * @param {number} max number of permitted values - * @param {Function} onRemove callback called with items when they expire - */ - constructor(max: number, onRemove: (element: T) => void) { - this.max = max; - this.onRemove = onRemove; - this.reset(); - } - - /** - * Clear the cache - * - * @returns {LRUCache} this cache - * @private - */ - reset() { - for (const key in this.data) { - for (const removedData of this.data[key]) { - this.onRemove(removedData); - } - } - - this.data = {}; - this.order = []; - - return this; - } - - /** - * Add a key, value combination to the cache, trimming its size if this pushes - * it over max length. - * - * @param {string} key lookup key for the item - * @param {*} data any value - * - * @returns {LRUCache} this cache - * @private - */ - add(key: string, data: T) { - if (this.data[key] === undefined) { - this.data[key] = []; - } - this.data[key].push(data); - this.order.push(key); - - if (this.order.length > this.max) { - const removedData = this.getAndRemove(this.order[0]); - if (removedData) this.onRemove(removedData); - } - - return this; - } - - /** - * Determine whether the value attached to `key` is present - * - * @param {string} key the key to be looked-up - * @returns {boolean} whether the cache has this value - * @private - */ - has(key: string): boolean { - return key in this.data; - } - - /** - * List all keys in the cache - * - * @returns {Array} an array of keys in this cache. - * @private - */ - keys(): Array { - return this.order; - } - - /** - * Get the value attached to a specific key and remove data from cache. - * If the key is not found, returns `null` - * - * @param {string} key the key to look up - * @returns {*} the data, or null if it isn't found - * @private - */ - getAndRemove(key: string): ?T { - if (!this.has(key)) { return null; } - - const data = this.data[key].shift(); - - if (this.data[key].length === 0) { - delete this.data[key]; - } - this.order.splice(this.order.indexOf(key), 1); - - return data; - } - - /** - * Get the value attached to a specific key without removing data - * from the cache. If the key is not found, returns `null` - * - * @param {string} key the key to look up - * @returns {*} the data, or null if it isn't found - * @private - */ - get(key: string): ?T { - if (!this.has(key)) { return null; } - - const data = this.data[key][0]; - return data; - } - - /** - * Remove a key/value combination from the cache. - * - * @param {string} key the key for the pair to delete - * @returns {LRUCache} this cache - * @private - */ - remove(key: string) { - if (!this.has(key)) { return this; } - - const data = this.data[key].pop(); - if (this.data[key].length === 0) { - delete this.data[key]; - } - this.onRemove(data); - this.order.splice(this.order.indexOf(key), 1); - - return this; - } - - /** - * Change the max size of the cache. - * - * @param {number} max the max size of the cache - * @returns {LRUCache} this cache - * @private - */ - setMaxSize(max: number): LRUCache { - this.max = max; - - while (this.order.length > this.max) { - const removedData = this.getAndRemove(this.order[0]); - if (removedData) this.onRemove(removedData); - } - - return this; - } -} - -export default LRUCache; diff --git a/test/unit/source/source_cache.test.js b/test/unit/source/source_cache.test.js index 3a8ff81a7d2..02c297dbcca 100644 --- a/test/unit/source/source_cache.test.js +++ b/test/unit/source/source_cache.test.js @@ -125,12 +125,9 @@ test('SourceCache#addTile', (t) => { sourceCache._setTileReloadTimer = (id) => { sourceCache._timers[id] = setTimeout(() => {}, 0); }; - sourceCache._setCacheInvalidationTimer = (id) => { - sourceCache._cacheTimers[id] = setTimeout(() => {}, 0); - }; sourceCache._loadTile = (tile, callback) => { tile.state = 'loaded'; - tile.getExpiryTimeout = () => time; + tile.getExpiryTimeout = () => 1000 * 60; sourceCache._setTileReloadTimer(tileID.key, tile); callback(); }; @@ -142,22 +139,22 @@ test('SourceCache#addTile', (t) => { const id = tileID.key; t.notOk(sourceCache._timers[id]); - t.notOk(sourceCache._cacheTimers[id]); + t.notOk(sourceCache._cache.has(tileID)); sourceCache._addTile(tileID); t.ok(sourceCache._timers[id]); - t.notOk(sourceCache._cacheTimers[id]); + t.notOk(sourceCache._cache.has(tileID)); sourceCache._removeTile(tileID.key); t.notOk(sourceCache._timers[id]); - t.ok(sourceCache._cacheTimers[id]); + t.ok(sourceCache._cache.has(tileID)); sourceCache._addTile(tileID); t.ok(sourceCache._timers[id]); - t.notOk(sourceCache._cacheTimers[id]); + t.notOk(sourceCache._cache.has(tileID)); t.end(); }); @@ -1392,7 +1389,7 @@ test('SourceCache#findLoadedParent', (t) => { sourceCache.updateCacheSize(tr); const tile = new Tile(new OverscaledTileID(1, 0, 1, 0, 0), 512, 22); - sourceCache._cache.add(tile.tileID.key, tile); + sourceCache._cache.add(tile.tileID, tile); const retain = {}; const expectedRetain = {}; diff --git a/test/unit/source/tile_cache.test.js b/test/unit/source/tile_cache.test.js new file mode 100644 index 00000000000..557f875b8cc --- /dev/null +++ b/test/unit/source/tile_cache.test.js @@ -0,0 +1,139 @@ +import { test } from 'mapbox-gl-js-test'; +import TileCache from '../../../src/source/tile_cache'; +import { OverscaledTileID } from '../../../src/source/tile_id'; + +const idA = new OverscaledTileID(10, 0, 10, 0, 1); +const idB = new OverscaledTileID(10, 0, 10, 0, 2); +const idC = new OverscaledTileID(10, 0, 10, 0, 3); +const idD = new OverscaledTileID(10, 0, 10, 0, 4); +const tileA = { tileID: idA }; +const tileA2 = { tileID: idA }; +const tileB = { tileID: idB }; +const tileC = { tileID: idC }; +const tileD = { tileID: idD }; + +function keysExpected(t, cache, ids) { + t.deepEqual(cache.order, ids.map((id) => id.key), 'keys'); +} + +test('TileCache', (t) => { + const cache = new TileCache(10, (removed) => { + t.equal(removed, 'dc'); + }); + t.equal(cache.getAndRemove(idC), null, '.getAndRemove() to null'); + t.equal(cache.add(idA, tileA), cache, '.add()'); + keysExpected(t, cache, [idA]); + t.equal(cache.has(idA), true, '.has()'); + t.equal(cache.getAndRemove(idA), tileA, '.getAndRemove()'); + t.equal(cache.getAndRemove(idA), null, '.getAndRemove()'); + t.equal(cache.has(idA), false, '.has()'); + keysExpected(t, cache, []); + t.end(); +}); + +test('TileCache - getWithoutRemoving', (t) => { + const cache = new TileCache(10, () => { + t.fail(); + }); + t.equal(cache.add(idA, tileA), cache, '.add()'); + t.equal(cache.get(idA), tileA, '.get()'); + keysExpected(t, cache, [idA]); + t.equal(cache.get(idA), tileA, '.get()'); + t.end(); +}); + +test('TileCache - duplicate add', (t) => { + const cache = new TileCache(10, () => { + t.fail(); + }); + + cache.add(idA, tileA); + cache.add(idA, tileA2); + + keysExpected(t, cache, [idA, idA]); + t.ok(cache.has(idA)); + t.equal(cache.getAndRemove(idA), tileA); + t.ok(cache.has(idA)); + t.equal(cache.getAndRemove(idA), tileA2); + t.end(); +}); + +test('TileCache - expiry', (t) => { + const cache = new TileCache(10, (removed) => { + t.ok(cache.has(idB)); + t.equal(removed, tileA2); + t.end(); + }); + + + cache.add(idB, tileB, 0); + cache.getAndRemove(idB); + // removing clears the expiry timeout + cache.add(idB); + + cache.add(idA, tileA); + cache.add(idA, tileA2, 0); // expires immediately and `onRemove` is called. +}); + +test('TileCache - remove', (t) => { + const cache = new TileCache(10, () => {}); + + cache.add(idA, tileA); + cache.add(idB, tileB); + cache.add(idC, tileC); + + keysExpected(t, cache, [idA, idB, idC]); + t.ok(cache.has(idB)); + + cache.remove(idB); + + keysExpected(t, cache, [idA, idC]); + t.notOk(cache.has(idB)); + + t.ok(cache.remove(idB)); + + t.end(); +}); + +test('TileCache - overflow', (t) => { + const cache = new TileCache(1, (removed) => { + t.equal(removed, tileA); + }); + cache.add(idA, tileA); + cache.add(idB, tileB); + + t.ok(cache.has(idB)); + t.notOk(cache.has(idA)); + t.end(); +}); + +test('TileCache#reset', (t) => { + let called; + const cache = new TileCache(10, (removed) => { + t.equal(removed, tileA); + called = true; + }); + cache.add(idA, tileA); + t.equal(cache.reset(), cache); + t.equal(cache.has(idA), false); + t.ok(called); + t.end(); +}); + +test('TileCache#setMaxSize', (t) => { + let numRemoved = 0; + const cache = new TileCache(10, () => { + numRemoved++; + }); + cache.add(idA, tileA); + cache.add(idB, tileB); + cache.add(idC, tileC); + t.equal(numRemoved, 0); + cache.setMaxSize(15); + t.equal(numRemoved, 0); + cache.setMaxSize(1); + t.equal(numRemoved, 2); + cache.add(idD, tileD); + t.equal(numRemoved, 3); + t.end(); +}); diff --git a/test/unit/util/lru_cache.test.js b/test/unit/util/lru_cache.test.js deleted file mode 100644 index 6335d19407a..00000000000 --- a/test/unit/util/lru_cache.test.js +++ /dev/null @@ -1,107 +0,0 @@ -import { test } from 'mapbox-gl-js-test'; -import LRUCache from '../../../src/util/lru_cache'; - -test('LRUCache', (t) => { - const cache = new LRUCache(10, (removed) => { - t.equal(removed, 'dc'); - }); - t.equal(cache.getAndRemove('foo'), null, '.getAndRemove() to null'); - t.equal(cache.add('washington', 'dc'), cache, '.add()'); - t.deepEqual(cache.keys(), ['washington'], '.keys()'); - t.equal(cache.has('washington'), true, '.has()'); - t.equal(cache.getAndRemove('washington'), 'dc', '.getAndRemove()'); - t.equal(cache.getAndRemove('washington'), null, '.getAndRemove()'); - t.equal(cache.has('washington'), false, '.has()'); - t.deepEqual(cache.keys(), [], '.keys()'); - t.end(); -}); - -test('LRUCache - getWithoutRemoving', (t) => { - const cache = new LRUCache(10, () => { - t.fail(); - }); - t.equal(cache.add('washington', 'dc'), cache, '.add()'); - t.equal(cache.get('washington'), 'dc', '.get()'); - t.deepEqual(cache.keys(), ['washington'], '.keys()'); - t.equal(cache.get('washington'), 'dc', '.get()'); - t.end(); -}); - -test('LRUCache - duplicate add', (t) => { - const cache = new LRUCache(10, () => { - t.fail(); - }); - - cache.add('a', 'b'); - cache.add('a', 'c'); - - t.deepEqual(cache.keys(), ['a', 'a']); - t.ok(cache.has('a')); - t.equal(cache.getAndRemove('a'), 'b'); - t.ok(cache.has('a')); - t.equal(cache.getAndRemove('a'), 'c'); - t.end(); -}); - -test('LRUCache - remove', (t) => { - const cache = new LRUCache(10, () => {}); - - cache.add('washington', 'dc'); - cache.add('baltimore', 'md'); - cache.add('richmond', 'va'); - - t.deepEqual(cache.keys(), ['washington', 'baltimore', 'richmond']); - t.ok(cache.has('baltimore')); - - cache.remove('baltimore'); - - t.deepEqual(cache.keys(), ['washington', 'richmond']); - t.notOk(cache.has('baltimore')); - - t.ok(cache.remove('baltimore')); - - t.end(); -}); - -test('LRUCache - overflow', (t) => { - const cache = new LRUCache(1, (removed) => { - t.equal(removed, 'b'); - }); - cache.add('a', 'b'); - cache.add('c', 'd'); - - t.ok(cache.has('c')); - t.notOk(cache.has('a')); - t.end(); -}); - -test('LRUCache#reset', (t) => { - let called; - const cache = new LRUCache(10, (removed) => { - t.equal(removed, 'dc'); - called = true; - }); - cache.add('washington', 'dc'); - t.equal(cache.reset(), cache); - t.equal(cache.has('washington'), false); - t.ok(called); - t.end(); -}); - -test('LRUCache#setMaxSize', (t) => { - let numRemoved = 0; - const cache = new LRUCache(10, () => { - numRemoved++; - }); - cache.add(1, 1); - cache.add(2, 2); - cache.add(3, 3); - t.equal(numRemoved, 0); - cache.setMaxSize(15); - t.equal(numRemoved, 0); - cache.setMaxSize(1); - t.equal(numRemoved, 2); - cache.add(4, 4); - t.equal(numRemoved, 3); - t.end(); -});