From a190cdf1297cc2c92acca94b2e13f952d7dd4406 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 30 Oct 2020 10:47:53 +0000 Subject: [PATCH 1/3] fix: cache preloaded CIDs We 'preload' most CIDs we interact with on the network. In some cases this can mean preloading the same CID over and over again which is not necessary. This PR adds a LRU cache to the preloader with a default size of 1000. The cache is used to avoid re-preloading the same CID over and over again until it drops out of the cache. We use a cache that will evict CIDs over time to have some sort of upper bound on memory usage. Fixes #3307 --- packages/ipfs-core/src/preload.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/ipfs-core/src/preload.js b/packages/ipfs-core/src/preload.js index 57d24fc76a..ce304b12ee 100644 --- a/packages/ipfs-core/src/preload.js +++ b/packages/ipfs-core/src/preload.js @@ -6,6 +6,9 @@ const CID = require('cids') const shuffle = require('array-shuffle') const AbortController = require('native-abort-controller') const preload = require('./runtime/preload-nodejs') +/** @type {typeof import('hashlru').default} */ +// @ts-ignore - hashlru has incorrect typedefs +const hashlru = require('hashlru') const log = Object.assign( debug('ipfs:preload'), @@ -14,12 +17,14 @@ const log = Object.assign( /** * @param {Object} [options] - * @param {boolean} [options.enabled] - * @param {string[]} [options.addresses] + * @param {boolean} [options.enabled] - Whether to preload anything + * @param {string[]} [options.addresses] - Which preload servers to use + * @param {number} [options.cache] - How many CIDs to cache */ const createPreloader = (options = {}) => { options.enabled = Boolean(options.enabled) options.addresses = options.addresses || [] + options.cache = options.cache || 1000 if (!options.enabled || !options.addresses.length) { log('preload disabled') @@ -34,6 +39,9 @@ const createPreloader = (options = {}) => { let requests = [] const apiUris = options.addresses.map(toUri) + // Avoid preloading the same CID over and over again + const cache = hashlru(options.cache) + /** * @param {string|CID} path * @returns {Promise} @@ -46,6 +54,14 @@ const createPreloader = (options = {}) => { path = new CID(path).toString() } + if (cache.has(path)) { + // we've preloaded this recently, don't preload it again + return + } + + // make sure we don't preload this again any time soon + cache.set(path, true) + const fallbackApiUris = shuffle(apiUris) let success = false const now = Date.now() From 885354382c0b12cec12dcd029411651db9078a74 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 30 Oct 2020 11:14:33 +0000 Subject: [PATCH 2/3] chore: add test for caching of cids --- packages/ipfs-core/test/preload.spec.js | 39 +++++++++++++++++-------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/packages/ipfs-core/test/preload.spec.js b/packages/ipfs-core/test/preload.spec.js index c2c69fe575..f565a69822 100644 --- a/packages/ipfs-core/test/preload.spec.js +++ b/packages/ipfs-core/test/preload.spec.js @@ -26,6 +26,19 @@ describe('preload', () => { after(() => cleanup()) afterEach(() => MockPreloadNode.clearPreloadCids()) + it('should not preload content multiple times', async function () { + this.timeout(50 * 1000) + const { cid } = await ipfs.add(uint8ArrayFromString(nanoid()), { preload: false }) + + await all(ipfs.cat(cid)) + await MockPreloadNode.waitForCids(cid) + + // should not preload the second time + await MockPreloadNode.clearPreloadCids() + await all(ipfs.cat(cid)) + await expect(MockPreloadNode.waitForCids(cid)).to.eventually.be.rejectedWith('Timed out waiting for CIDs to be preloaded') + }) + it('should preload content added with add', async function () { this.timeout(50 * 1000) const res = await ipfs.add(uint8ArrayFromString(nanoid())) @@ -112,7 +125,7 @@ describe('preload', () => { }, { path: 'dir0/file2', content: uint8ArrayFromString(nanoid()) - }], { wrapWithDirectory: true })) + }], { wrapWithDirectory: true, preload: false })) const wrappingDir = res.find(file => file.path === '') expect(wrappingDir).to.exist() @@ -147,12 +160,12 @@ describe('preload', () => { const [parent, link] = await Promise.all([createNode(), createNode()]) + await MockPreloadNode.clearPreloadCids() const cid = await ipfs.object.patch.addLink(parent.cid, { Name: 'link', Hash: link.cid, Tsize: link.node.size }) - await MockPreloadNode.waitForCids(cid) }) @@ -171,6 +184,7 @@ describe('preload', () => { }] }) + await MockPreloadNode.clearPreloadCids() const cid = await ipfs.object.patch.rmLink(parentCid, { name: 'link' }) await MockPreloadNode.waitForCids(cid) }) @@ -178,6 +192,7 @@ describe('preload', () => { it('should preload content added with object.patch.setData', async function () { this.timeout(50 * 1000) const originalCid = await ipfs.object.put({ Data: uint8ArrayFromString(nanoid()), Links: [] }) + await MockPreloadNode.clearPreloadCids() const patchedCid = await ipfs.object.patch.setData(originalCid, uint8ArrayFromString(nanoid())) await MockPreloadNode.waitForCids(patchedCid) }) @@ -185,13 +200,15 @@ describe('preload', () => { it('should preload content added with object.patch.appendData', async function () { this.timeout(50 * 1000) const originalCid = await ipfs.object.put({ Data: uint8ArrayFromString(nanoid()), Links: [] }) + await MockPreloadNode.clearPreloadCids() const patchedCid = await ipfs.object.patch.appendData(originalCid, uint8ArrayFromString(nanoid())) await MockPreloadNode.waitForCids(patchedCid) }) it('should preload content retrieved with object.get', async function () { this.timeout(50 * 1000) - const cid = await ipfs.object.new({ preload: false }) + const cid = await ipfs.object.put({ Data: uint8ArrayFromString(nanoid()), Links: [] }, { preload: false }) + await MockPreloadNode.clearPreloadCids() await ipfs.object.get(cid) await MockPreloadNode.waitForCids(cid) }) @@ -205,6 +222,7 @@ describe('preload', () => { it('should preload content retrieved with block.get', async function () { this.timeout(50 * 1000) const block = await ipfs.block.put(uint8ArrayFromString(nanoid()), { preload: false }) + await MockPreloadNode.clearPreloadCids() await ipfs.block.get(block.cid) await MockPreloadNode.waitForCids(block.cid) }) @@ -212,6 +230,7 @@ describe('preload', () => { it('should preload content retrieved with block.stat', async function () { this.timeout(50 * 1000) const block = await ipfs.block.put(uint8ArrayFromString(nanoid()), { preload: false }) + await MockPreloadNode.clearPreloadCids() await ipfs.block.stat(block.cid) await MockPreloadNode.waitForCids(block.cid) }) @@ -228,39 +247,35 @@ describe('preload', () => { const obj = { test: nanoid() } const opts = { format: 'dag-cbor', hashAlg: 'sha2-256', preload: false } const cid = await ipfs.dag.put(obj, opts) + await MockPreloadNode.clearPreloadCids() await ipfs.dag.get(cid) await MockPreloadNode.waitForCids(cid) }) it('should preload content retrieved with files.ls', async () => { - const res = await ipfs.add({ path: `/t/${nanoid()}`, content: uint8ArrayFromString(nanoid()) }) + const res = await ipfs.add({ path: `/t/${nanoid()}`, content: uint8ArrayFromString(nanoid()) }, { preload: false }) const dirCid = res.cid - await MockPreloadNode.waitForCids(dirCid) await MockPreloadNode.clearPreloadCids() await all(ipfs.files.ls(`/ipfs/${dirCid}`)) await MockPreloadNode.waitForCids(`/ipfs/${dirCid}`) }) it('should preload content retrieved with files.ls by CID', async () => { - const res = await ipfs.add({ path: `/t/${nanoid()}`, content: uint8ArrayFromString(nanoid()) }) + const res = await ipfs.add({ path: `/t/${nanoid()}`, content: uint8ArrayFromString(nanoid()) }, { preload: false }) const dirCid = res.cid - await MockPreloadNode.waitForCids(dirCid) - await MockPreloadNode.clearPreloadCids() await all(ipfs.files.ls(dirCid)) await MockPreloadNode.waitForCids(dirCid) }) it('should preload content retrieved with files.read', async () => { - const { cid } = await ipfs.add(uint8ArrayFromString(nanoid())) - await MockPreloadNode.waitForCids(cid) + const { cid } = await ipfs.add(uint8ArrayFromString(nanoid()), { preload: false }) await MockPreloadNode.clearPreloadCids() await ipfs.files.read(`/ipfs/${cid}`) await MockPreloadNode.waitForCids(`/ipfs/${cid}`) }) it('should preload content retrieved with files.stat', async () => { - const { cid: fileCid } = await ipfs.add(uint8ArrayFromString(nanoid())) - await MockPreloadNode.waitForCids(fileCid) + const { cid: fileCid } = await ipfs.add(uint8ArrayFromString(nanoid()), { preload: false }) await MockPreloadNode.clearPreloadCids() await ipfs.files.stat(`/ipfs/${fileCid}`) await MockPreloadNode.waitForCids(`/ipfs/${fileCid}`) From 119027c978f80892c80cdd10b23099fa1113f36d Mon Sep 17 00:00:00 2001 From: Alex Potsides Date: Fri, 30 Oct 2020 12:44:22 +0000 Subject: [PATCH 3/3] chore: apply suggestions from code review Co-authored-by: Vasco Santos --- packages/ipfs-core/src/preload.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ipfs-core/src/preload.js b/packages/ipfs-core/src/preload.js index ce304b12ee..53425347d3 100644 --- a/packages/ipfs-core/src/preload.js +++ b/packages/ipfs-core/src/preload.js @@ -17,9 +17,9 @@ const log = Object.assign( /** * @param {Object} [options] - * @param {boolean} [options.enabled] - Whether to preload anything - * @param {string[]} [options.addresses] - Which preload servers to use - * @param {number} [options.cache] - How many CIDs to cache + * @param {boolean} [options.enabled = false] - Whether to preload anything + * @param {string[]} [options.addresses = []] - Which preload servers to use + * @param {number} [options.cache = 1000] - How many CIDs to cache */ const createPreloader = (options = {}) => { options.enabled = Boolean(options.enabled)