From caa191b181175ca80edca5eb08ffa2afc25c6511 Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Tue, 16 Feb 2016 17:14:19 -0800 Subject: [PATCH 1/4] Simplify key generation --- index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/index.js b/index.js index 0d6f681..671033c 100644 --- a/index.js +++ b/index.js @@ -215,9 +215,7 @@ module.exports = function(tilelive, options) { console.warn(err.stack); } - var sha1 = crypto.createHash("sha1"); - sha1.update(JSON.stringify(uri)); - var key = sha1.digest("hex"); + var key = crypto.createHash("sha1").update(JSON.stringify(uri)).digest("hex"); return lock(key, function(unlock) { return tilelive.load(uri, function(err, source) { From 1a5d2343042ddcd949b58c5633008c645d27105d Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Tue, 16 Feb 2016 17:18:09 -0800 Subject: [PATCH 2/4] Remove closed sources from the cache --- CHANGELOG.md | 4 + index.js | 209 +++++++++++++++++++++++++++------------------------ 2 files changed, 115 insertions(+), 98 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 850c9c5..cdd0b84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changes +## v0.6.0 + +* Remove `close()`d sources from the cache. + ## v0.5.2 - 2/16/16 * Fix cache handling for items w/o `.length` (@petrsloup) diff --git a/index.js b/index.js index 671033c..6c6d655 100644 --- a/index.js +++ b/index.js @@ -70,103 +70,6 @@ var getContextCallbackProperties = function(context) { return properties; }; -var enableCaching = function(uri, source, locker) { - // TODO use ES6 Symbols to prevent collisions - if (source._cached) { - // already cached - - return source; - } - - var uriSha1 = crypto.createHash("sha1"); - uriSha1.update(JSON.stringify(uri)); - var uriHash = uriSha1.digest("hex"); - - var makeKey = function(name, properties) { - properties = properties || {}; - - var key = util.format("%s:%s@%j", name, uriHash, properties); - - // glue on any additional arguments using their JSON representation - key += Array.prototype.slice.call(arguments, 2).map(JSON.stringify).join(","); - - var sha1 = crypto.createHash("sha1"); - sha1.update(key); - return sha1.digest("hex"); - }; - - if (source.getTile) { - var _getTile = source.getTile.bind(source); - - source.getTile = locker(function(z, x, y, lock) { - var properties = getContextCallbackProperties(this); - - // lock neighboring tiles when metatiling (if the source is streamable) - if (source.pipe && source.metatile && source.metatile > 1) { - // TODO extract this (also used in tilelive-streaming) - // get neighboring tiles within the same metatile - var dx = x % source.metatile, - dy = y % source.metatile, - metaX = x - dx, - metaY = y - dy; - - for (var ix = metaX; ix < metaX + source.metatile; ix++) { - for (var iy = metaY; iy < metaY + source.metatile; iy++) { - // ignore the current tile - if (!(ix === x && iy === y)) { - var key = makeKey("getTile", properties, z, ix, iy); - - if (!locker.locks.get(key)) { - // lock it with an empty list of callbacks (nothing to notify) - locker.locks.set(key, []); - } - } - } - } - } - - return lock(makeKey("getTile", properties, z, x, y), function(unlock) { - return _getTile(z, x, y, unlock); - }); - }).bind(source); - } - - if (source.getGrid) { - var _getGrid = source.getGrid.bind(source); - - source.getGrid = locker(function(z, x, y, lock) { - return lock(makeKey("getGrid", getContextCallbackProperties(this), z, x, y), function(unlock) { - return _getGrid(z, x, y, unlock); - }); - }).bind(source); - } - - if (source.getInfo) { - var _getInfo = source.getInfo.bind(source); - - source.getInfo = locker(function(lock) { - return lock(makeKey("getInfo", getContextCallbackProperties(this)), function(unlock) { - return _getInfo(unlock); - }); - }).bind(source); - } - - var target = new stream.Writable({ - objectMode: true, - highWaterMark: 16 // arbitrary backlog sizing - }); - - target._write = function(tile, _, callback) { - tile.pipe(new CacheCollector(locker, makeKey).on("finish", callback)); - }; - - source.pipe(target); - - source._cached = true; - - return source; -}; - module.exports = function(tilelive, options) { tilelive = enableStreaming(tilelive); @@ -199,6 +102,116 @@ module.exports = function(tilelive, options) { } }); + var enableCaching = function(source, sourceKey) { + // TODO use ES6 Symbols to prevent collisions + if (source._cached) { + // already cached + + return source; + } + + var makeKey = function(name, properties) { + properties = properties || {}; + + var key = util.format("%s:%s@%j", name, sourceKey, properties); + + // glue on any additional arguments using their JSON representation + key += Array.prototype.slice.call(arguments, 2).map(JSON.stringify).join(","); + + var sha1 = crypto.createHash("sha1"); + sha1.update(key); + return sha1.digest("hex"); + }; + + if (source.getTile) { + var _getTile = source.getTile.bind(source); + + source.getTile = locker(function(z, x, y, lock) { + var properties = getContextCallbackProperties(this); + + // lock neighboring tiles when metatiling (if the source is streamable) + if (source.pipe && source.metatile && source.metatile > 1) { + // TODO extract this (also used in tilelive-streaming) + // get neighboring tiles within the same metatile + var dx = x % source.metatile, + dy = y % source.metatile, + metaX = x - dx, + metaY = y - dy; + + for (var ix = metaX; ix < metaX + source.metatile; ix++) { + for (var iy = metaY; iy < metaY + source.metatile; iy++) { + // ignore the current tile + if (!(ix === x && iy === y)) { + var key = makeKey("getTile", properties, z, ix, iy); + + if (!locker.locks.get(key)) { + // lock it with an empty list of callbacks (nothing to notify) + locker.locks.set(key, []); + } + } + } + } + } + + return lock(makeKey("getTile", properties, z, x, y), function(unlock) { + return _getTile(z, x, y, unlock); + }); + }).bind(source); + } + + if (source.getGrid) { + var _getGrid = source.getGrid.bind(source); + + source.getGrid = locker(function(z, x, y, lock) { + return lock(makeKey("getGrid", getContextCallbackProperties(this), z, x, y), function(unlock) { + return _getGrid(z, x, y, unlock); + }); + }).bind(source); + } + + if (source.getInfo) { + var _getInfo = source.getInfo.bind(source); + + source.getInfo = locker(function(lock) { + return lock(makeKey("getInfo", getContextCallbackProperties(this)), function(unlock) { + return _getInfo(unlock); + }); + }).bind(source); + } + + var _close = function(callback) { + return callback(); + }; + + if (source.close) { + _close = source.close.bind(source); + } + + source.close = function(callback) { + callback = callback || function() {}; + + // remove this from the source cache + lockedLoad.cache.del(sourceKey); + + return _close(callback); + } + + var target = new stream.Writable({ + objectMode: true, + highWaterMark: 16 // arbitrary backlog sizing + }); + + target._write = function(tile, _, callback) { + tile.pipe(new CacheCollector(locker, makeKey).on("finish", callback)); + }; + + source.pipe(target); + + source._cached = true; + + return source; + }; + cache.load = lockedLoad(function(uri, lock) { if (typeof uri === "string") { uri = url.parse(uri, true); @@ -222,7 +235,7 @@ module.exports = function(tilelive, options) { if (!err && options.size > 0 && useCache) { - source = enableCaching(uri, source, locker); + source = enableCaching(source, key); } return unlock(err, source); From 2f9cfb681ac3e44aa95f1a48f149eddf1044db25 Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Tue, 16 Feb 2016 17:23:57 -0800 Subject: [PATCH 3/4] Remove options.closeDelay Close immediately when sources are evicted by the cache. Other modules may be holding onto references, but they shouldn't assume that they'll remain open. --- README.md | 5 +---- index.js | 10 +++------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index b86c44a..2cdf607 100644 --- a/README.md +++ b/README.md @@ -10,12 +10,9 @@ and data produced by their respective `getTile()` functions. var tilelive = require("tilelive"), cache = require("tilelive-cache")(tilelive, { size: 10, // 10MB cache (the default) - sources: 6, // cache a maximum of 6 sources (the default); you may + sources: 6 // cache a maximum of 6 sources (the default); you may // need to change this if you're using lots of // composed sources - closeDelay: 30 // wait 30s (the default) before closing sources when - // they've been evicted; this should prevent drain errors in - // tilelive-mapnik }); // ... diff --git a/index.js b/index.js index 6c6d655..5a77e8d 100644 --- a/index.js +++ b/index.js @@ -92,13 +92,9 @@ module.exports = function(tilelive, options) { var lockedLoad = lockingCache({ max: options.sources, dispose: function(key, values) { - // don't close the source immediately in case there are pending - // references to it that haven't requested tiles yet - setTimeout(function() { - // the source will always be the first value since it's the first - // argument to unlock() - values[0].close(function() {}); - }, (options.closeDelay || 30) * 1000); + // the source will always be the first value since it's the first + // argument to unlock() + values[0].close(); } }); From 63555b695c20a30ca206f5f7f38cc510b5fb4b8f Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Tue, 16 Feb 2016 17:52:06 -0800 Subject: [PATCH 4/4] Prevent infinite recursion cache.del(sourceKey) will call close() on the source --- index.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 5a77e8d..030e4f3 100644 --- a/index.js +++ b/index.js @@ -187,7 +187,12 @@ module.exports = function(tilelive, options) { callback = callback || function() {}; // remove this from the source cache - lockedLoad.cache.del(sourceKey); + if (lockedLoad.cache.has(sourceKey)) { + // queue deletion (otherwise the cache's dispose calls this) + setImmediate(function() { + lockedLoad.cache.del(sourceKey); + }); + } return _close(callback); }