From 187fb52d27a3476dd9042bffb09e31b28a6cc271 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Wed, 25 Sep 2024 20:10:52 +0200 Subject: [PATCH 01/14] Initial pass at fixing the memory cache --- src/base/TilesRendererBase.js | 20 ++++++++++++++------ src/three/TilesRenderer.js | 2 +- src/utilities/LRUCache.js | 21 ++++++++++++++------- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/base/TilesRendererBase.js b/src/base/TilesRendererBase.js index ffb42cf11..e38b3e999 100644 --- a/src/base/TilesRendererBase.js +++ b/src/base/TilesRendererBase.js @@ -560,6 +560,8 @@ export class TilesRendererBase { const extension = getUrlExtension( uri ); const addedSuccessfully = lruCache.add( tile, t => { + console.log('REMOVAL', tile.content ) + // Stop the load if it's started if ( t.__loadingState === LOADING ) { @@ -737,15 +739,21 @@ export class TilesRendererBase { stats.parsing --; tile.__loadingState = LOADED; - // if the cache is full due to newly loaded memory then lets discard this tile - it will - // be loaded again later from the disk cache if needed. - if ( lruCache.isFull() ) { + // If the memory of the item hasn't been registered yet + if ( lruCache.getMemoryUsage( tile ) === null ) { - lruCache.remove( tile ); + if ( lruCache.isFull() && lruCache.computeMemoryUsageCallback( tile ) > 0 ) { - } else { + // And if the cache is full due to newly loaded memory then lets discard this tile - it will + // be loaded again later from the disk cache if needed. + lruCache.remove( tile ); + + } else { + + // Otherwise update the item to the latest known value + lruCache.updateMemoryUsage( tile ); - lruCache.updateMemoryUsage( tile ); + } } diff --git a/src/three/TilesRenderer.js b/src/three/TilesRenderer.js index 7dfc5a818..21c283f42 100644 --- a/src/three/TilesRenderer.js +++ b/src/three/TilesRenderer.js @@ -76,7 +76,7 @@ export class TilesRenderer extends TilesRendererBase { this._eventDispatcher = new EventDispatcher(); this._upRotationMatrix = new Matrix4(); - this.lruCache.getMemoryUsageCallback = tile => tile.cached.bytesUsed || 0; + this.lruCache.computeMemoryUsageCallback = tile => tile.cached.bytesUsed ?? null; // flag indicating whether frustum culling should be disabled this._autoDisableRendererCulling = true; diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index bb14c1d2e..f3f517e07 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -54,7 +54,7 @@ class LRUCache { this.bytesMap = new Map(); this._unloadPriorityCallback = null; - this.getMemoryUsageCallback = () => 0; + this.computeMemoryUsageCallback = () => null; const itemSet = this.itemSet; this.defaultPriorityCallback = item => itemSet.get( item ); @@ -68,6 +68,12 @@ class LRUCache { } + getMemoryUsage( item ) { + + return this.bytesMap.get( item ) ?? null; + + } + add( item, removeCb ) { if ( this.markUnusedQueued ) { @@ -98,8 +104,9 @@ class LRUCache { itemSet.set( item, Date.now() ); callbacks.set( item, removeCb ); - const bytes = this.getMemoryUsageCallback( item ); - this.cachedBytes += bytes; + // computeMemoryUsageCallback can return "null" if memory usage is not known, yet + const bytes = this.computeMemoryUsageCallback( item ); + this.cachedBytes += bytes || 0; bytesMap.set( item, bytes ); return true; @@ -116,7 +123,7 @@ class LRUCache { if ( itemSet.has( item ) ) { - this.cachedBytes -= bytesMap.get( item ); + this.cachedBytes -= bytesMap.get( item ) || 0; bytesMap.delete( item ); callbacks.get( item )( item ); @@ -145,9 +152,9 @@ class LRUCache { } - this.cachedBytes -= bytesMap.get( item ); + this.cachedBytes -= bytesMap.get( item ) || 0; - const bytes = this.getMemoryUsageCallback( item ); + const bytes = this.computeMemoryUsageCallback( item ); bytesMap.set( item, bytes ); this.cachedBytes += bytes; @@ -244,7 +251,7 @@ class LRUCache { while ( true ) { const item = itemList[ removedNodes ]; - const bytes = bytesMap.get( item ); + const bytes = bytesMap.get( item ) || 0; // note that these conditions ensure we keep one tile over the byte cap so we can // align with the the isFull function reports. From 43cdd0bc7fc9aa69ef2c7869908f41f563133676 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Thu, 26 Sep 2024 19:02:42 +0200 Subject: [PATCH 02/14] Account for loading state in lru cache unload logic --- src/base/TilesRendererBase.js | 5 +++++ src/base/constants.js | 3 ++- src/utilities/LRUCache.js | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/base/TilesRendererBase.js b/src/base/TilesRendererBase.js index e38b3e999..71025b327 100644 --- a/src/base/TilesRendererBase.js +++ b/src/base/TilesRendererBase.js @@ -52,6 +52,11 @@ const lruPriorityCallback = ( a, b ) => { // dispose of deeper tiles first return a.__depthFromRenderedParent > b.__depthFromRenderedParent ? 1 : - 1; + } else if ( a.__loadingState !== b.__loadingState ) { + + // dispose of tiles that are further along in the loading process + return a.__loadingState > b.__loadingState ? - 1 : 1; + } else if ( a.__lastFrameVisited !== b.__lastFrameVisited ) { // dispose of least recent tiles first diff --git a/src/base/constants.js b/src/base/constants.js index ee656c21b..043b7288b 100644 --- a/src/base/constants.js +++ b/src/base/constants.js @@ -1,8 +1,9 @@ +// FAILED is negative so lru cache priority sorting will unload it first +export const FAILED = - 1; export const UNLOADED = 0; export const LOADING = 1; export const PARSING = 2; export const LOADED = 3; -export const FAILED = 4; // https://en.wikipedia.org/wiki/World_Geodetic_System // https://en.wikipedia.org/wiki/Flattening diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index f3f517e07..acfde74c1 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -266,6 +266,7 @@ class LRUCache { // don't unload any used tiles unless we're above our size cap if ( ! doContinue + || usedSet.has( item ) || removedNodes >= unused && this.cachedBytes - removedBytes - bytes <= maxBytesSize && itemList.length - removedNodes <= maxSize From 84cec94e8168e6e1bdf2232532c2b5eff1780bce Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 09:45:35 +0200 Subject: [PATCH 03/14] Remove nodes after the fact --- src/utilities/LRUCache.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index acfde74c1..5bb256060 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -248,6 +248,11 @@ class LRUCache { let removedNodes = 0; let removedBytes = 0; + + // TODO: unload up to "max bytes" limit as possible + // TODO: unload up to "min bytes" limit as possible + // TODO: unload up to "min nodes" limit as possible + while ( true ) { const item = itemList[ removedNodes ]; @@ -279,15 +284,17 @@ class LRUCache { removedBytes += bytes; removedNodes ++; + } + + itemList.splice( 0, removedNodes ).forEach( item => { + bytesMap.delete( item ); callbacks.get( item )( item ); itemSet.delete( item ); callbacks.delete( item ); + this.cachedBytes -= bytesMap.get( item ) || 0; - } - - itemList.splice( 0, removedNodes ); - this.cachedBytes -= removedBytes; + } ); // if we didn't remove enough nodes or we still have excess bytes and there are nodes to removed // then we want to fire another round of unloading From dc5fcfa30bc18fc5c5f9b8e05827e567574c378f Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 10:11:01 +0200 Subject: [PATCH 04/14] Simplify LRUCache eviction logic --- src/utilities/LRUCache.js | 53 +++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index 5bb256060..b973d3785 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -249,32 +249,41 @@ class LRUCache { let removedNodes = 0; let removedBytes = 0; - // TODO: unload up to "max bytes" limit as possible - // TODO: unload up to "min bytes" limit as possible - // TODO: unload up to "min nodes" limit as possible - - while ( true ) { - + // evict up to the max node or bytes size + while ( + this.cachedBytes - removedBytes > maxBytesSize || + itemList.length - removedNodes > maxSize + ) { + + // TODO: allow for disposing actively-loading items if they are above the + // maximum limits const item = itemList[ removedNodes ]; const bytes = bytesMap.get( item ) || 0; + if ( + usedSet.has( item ) || + this.cachedBytes - removedBytes - bytes < maxBytesSize + ) { - // note that these conditions ensure we keep one tile over the byte cap so we can - // align with the the isFull function reports. + break; - // base while condition - const doContinue = - removedNodes < nodesToUnload - || removedBytes < bytesToUnload - || this.cachedBytes - removedBytes - bytes > maxBytesSize - || itemList.length - removedNodes > maxSize; + } + + removedBytes += bytes; + removedNodes ++; - // don't unload any used tiles unless we're above our size cap + } + + // evict up to the min size + while ( + removedBytes < bytesToUnload || + removedNodes < nodesToUnload + ) { + + const item = itemList[ removedNodes ]; + const bytes = bytesMap.get( item ) || 0; if ( - ! doContinue - || usedSet.has( item ) - || removedNodes >= unused - && this.cachedBytes - removedBytes - bytes <= maxBytesSize - && itemList.length - removedNodes <= maxSize + usedSet.has( item ) || + this.cachedBytes - removedBytes - bytes < minBytesSize ) { break; @@ -286,13 +295,15 @@ class LRUCache { } + // remove the nodes itemList.splice( 0, removedNodes ).forEach( item => { + this.cachedBytes -= bytesMap.get( item ) || 0; + bytesMap.delete( item ); callbacks.get( item )( item ); itemSet.delete( item ); callbacks.delete( item ); - this.cachedBytes -= bytesMap.get( item ) || 0; } ); From b5dc52a7efe1cf45c0cae18a2be3868ada360fa2 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 10:31:42 +0200 Subject: [PATCH 05/14] Add "loading" field for cache tiles --- src/base/TilesRendererBase.js | 3 +++ src/utilities/LRUCache.js | 47 +++++++++++++++++++++++++++++------ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/base/TilesRendererBase.js b/src/base/TilesRendererBase.js index 71025b327..91c6b3ceb 100644 --- a/src/base/TilesRendererBase.js +++ b/src/base/TilesRendererBase.js @@ -615,6 +615,7 @@ export class TilesRendererBase { // Track a new load index so we avoid the condition where this load is stopped and // another begins soon after so we don't parse twice. + lruCache.setLoading( tile, true ); tile.__loadIndex ++; const loadIndex = tile.__loadIndex; const controller = new AbortController(); @@ -653,6 +654,7 @@ export class TilesRendererBase { console.error( `TilesRenderer : Failed to load tile at url "${ tile.content.uri }".` ); console.error( e ); tile.__loadingState = FAILED; + lruCache.setLoading( tile, false ); } else { @@ -743,6 +745,7 @@ export class TilesRendererBase { stats.parsing --; tile.__loadingState = LOADED; + lruCache.setLoading( tile, false ); // If the memory of the item hasn't been registered yet if ( lruCache.getMemoryUsage( tile ) === null ) { diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index b973d3785..4758681bb 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -52,6 +52,7 @@ class LRUCache { this.unloadingHandle = - 1; this.cachedBytes = 0; this.bytesMap = new Map(); + this.loadingSet = new Set(); this._unloadPriorityCallback = null; this.computeMemoryUsageCallback = () => null; @@ -120,6 +121,7 @@ class LRUCache { const itemList = this.itemList; const bytesMap = this.bytesMap; const callbacks = this.callbacks; + const loadingSet = this.loadingSet; if ( itemSet.has( item ) ) { @@ -133,6 +135,7 @@ class LRUCache { usedSet.delete( item ); itemSet.delete( item ); callbacks.delete( item ); + loadingSet.delete( item ); return true; @@ -142,6 +145,25 @@ class LRUCache { } + setLoading( item, value ) { + + const { itemSet, loadingSet } = this; + if ( itemSet.has( item ) ) { + + if ( value === true ) { + + loadingSet.add( item ); + + } else { + + loadingSet.delete( item ); + + } + + } + + } + updateMemoryUsage( item ) { const itemSet = this.itemSet; @@ -203,6 +225,7 @@ class LRUCache { itemList, itemSet, usedSet, + loadingSet, callbacks, bytesMap, minBytesSize, @@ -226,9 +249,19 @@ class LRUCache { const usedB = usedSet.has( b ); if ( usedA === usedB ) { - // Use the sort function otherwise - // higher priority should be further to the left - return - unloadPriorityCallback( a, b ); + const loadingA = loadingSet.has( a ); + const loadingB = loadingSet.has( b ); + if ( loadingA === loadingB ) { + + // Use the sort function otherwise + // higher priority should be further to the left + return - unloadPriorityCallback( a, b ); + + } else { + + return loadingA ? 1 : - 1; + + } } else { @@ -255,12 +288,10 @@ class LRUCache { itemList.length - removedNodes > maxSize ) { - // TODO: allow for disposing actively-loading items if they are above the - // maximum limits const item = itemList[ removedNodes ]; const bytes = bytesMap.get( item ) || 0; if ( - usedSet.has( item ) || + usedSet.has( item ) && ! loadingSet.has( item ) || this.cachedBytes - removedBytes - bytes < maxBytesSize ) { @@ -300,10 +331,12 @@ class LRUCache { this.cachedBytes -= bytesMap.get( item ) || 0; - bytesMap.delete( item ); callbacks.get( item )( item ); + bytesMap.delete( item ); itemSet.delete( item ); callbacks.delete( item ); + loadingSet.delete( item ); + usedSet.delete( item ); } ); From ad33d51bf8647a2b09cd661d891ec41a0652d849 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 10:50:25 +0200 Subject: [PATCH 06/14] Handle unloading correctly --- src/base/TilesRendererBase.js | 5 ++--- src/utilities/LRUCache.js | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/base/TilesRendererBase.js b/src/base/TilesRendererBase.js index 91c6b3ceb..4814823a0 100644 --- a/src/base/TilesRendererBase.js +++ b/src/base/TilesRendererBase.js @@ -615,7 +615,6 @@ export class TilesRendererBase { // Track a new load index so we avoid the condition where this load is stopped and // another begins soon after so we don't parse twice. - lruCache.setLoading( tile, true ); tile.__loadIndex ++; const loadIndex = tile.__loadIndex; const controller = new AbortController(); @@ -654,7 +653,7 @@ export class TilesRendererBase { console.error( `TilesRenderer : Failed to load tile at url "${ tile.content.uri }".` ); console.error( e ); tile.__loadingState = FAILED; - lruCache.setLoading( tile, false ); + lruCache.setLoaded( tile, true ); } else { @@ -745,7 +744,7 @@ export class TilesRendererBase { stats.parsing --; tile.__loadingState = LOADED; - lruCache.setLoading( tile, false ); + lruCache.setLoaded( tile, true ); // If the memory of the item hasn't been registered yet if ( lruCache.getMemoryUsage( tile ) === null ) { diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index 4758681bb..8a19cbef9 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -52,7 +52,7 @@ class LRUCache { this.unloadingHandle = - 1; this.cachedBytes = 0; this.bytesMap = new Map(); - this.loadingSet = new Set(); + this.loadedSet = new Set(); this._unloadPriorityCallback = null; this.computeMemoryUsageCallback = () => null; @@ -121,7 +121,7 @@ class LRUCache { const itemList = this.itemList; const bytesMap = this.bytesMap; const callbacks = this.callbacks; - const loadingSet = this.loadingSet; + const loadedSet = this.loadedSet; if ( itemSet.has( item ) ) { @@ -135,7 +135,7 @@ class LRUCache { usedSet.delete( item ); itemSet.delete( item ); callbacks.delete( item ); - loadingSet.delete( item ); + loadedSet.delete( item ); return true; @@ -145,18 +145,18 @@ class LRUCache { } - setLoading( item, value ) { + setLoaded( item, value ) { - const { itemSet, loadingSet } = this; + const { itemSet, loadedSet } = this; if ( itemSet.has( item ) ) { if ( value === true ) { - loadingSet.add( item ); + loadedSet.add( item ); } else { - loadingSet.delete( item ); + loadedSet.delete( item ); } @@ -225,7 +225,7 @@ class LRUCache { itemList, itemSet, usedSet, - loadingSet, + loadedSet, callbacks, bytesMap, minBytesSize, @@ -249,9 +249,9 @@ class LRUCache { const usedB = usedSet.has( b ); if ( usedA === usedB ) { - const loadingA = loadingSet.has( a ); - const loadingB = loadingSet.has( b ); - if ( loadingA === loadingB ) { + const loadedA = loadedSet.has( a ); + const loadedB = loadedSet.has( b ); + if ( loadedA === loadedB ) { // Use the sort function otherwise // higher priority should be further to the left @@ -259,7 +259,7 @@ class LRUCache { } else { - return loadingA ? 1 : - 1; + return loadedA ? 1 : - 1; } @@ -291,7 +291,7 @@ class LRUCache { const item = itemList[ removedNodes ]; const bytes = bytesMap.get( item ) || 0; if ( - usedSet.has( item ) && ! loadingSet.has( item ) || + usedSet.has( item ) && loadedSet.has( item ) || this.cachedBytes - removedBytes - bytes < maxBytesSize ) { @@ -335,7 +335,7 @@ class LRUCache { bytesMap.delete( item ); itemSet.delete( item ); callbacks.delete( item ); - loadingSet.delete( item ); + loadedSet.delete( item ); usedSet.delete( item ); } ); From 3c4688d8f2539cc5164eabadf9b814ea56576ba3 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 10:53:37 +0200 Subject: [PATCH 07/14] Remove log --- src/base/TilesRendererBase.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/base/TilesRendererBase.js b/src/base/TilesRendererBase.js index 4814823a0..2db14dab1 100644 --- a/src/base/TilesRendererBase.js +++ b/src/base/TilesRendererBase.js @@ -565,8 +565,6 @@ export class TilesRendererBase { const extension = getUrlExtension( uri ); const addedSuccessfully = lruCache.add( tile, t => { - console.log('REMOVAL', tile.content ) - // Stop the load if it's started if ( t.__loadingState === LOADING ) { From f644a1d248be81accd44ed12a80bfdcaca7dad30 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 20:43:22 +0200 Subject: [PATCH 08/14] Don't try to dispose of any items above the max cache sizes unless we have tiles that are still loading --- src/utilities/LRUCache.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index 8a19cbef9..8c8433286 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -233,13 +233,14 @@ class LRUCache { } = this; const unused = itemList.length - usedSet.size; + const unloaded = itemList.length - loadedSet.size; const excessNodes = Math.max( Math.min( itemList.length - minSize, unused ), 0 ); const excessBytes = this.cachedBytes - minBytesSize; const unloadPriorityCallback = this.unloadPriorityCallback || this.defaultPriorityCallback; let needsRerun = false; - const hasNodesToUnload = excessNodes > 0 && unused > 0 || itemList.length > maxSize; - const hasBytesToUnload = unused && this.cachedBytes > minBytesSize || this.cachedBytes > maxBytesSize; + const hasNodesToUnload = excessNodes > 0 && unused > 0 || unloaded && itemList.length > maxSize; + const hasBytesToUnload = unused && this.cachedBytes > minBytesSize || unloaded && this.cachedBytes > maxBytesSize; if ( hasBytesToUnload || hasNodesToUnload ) { // used items should be at the end of the array From 2606015ad623e23296d0e6ecd806e39a13ed39f6 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 20:45:17 +0200 Subject: [PATCH 09/14] Comment --- src/utilities/LRUCache.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index 8c8433286..ee9d14cf9 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -145,6 +145,9 @@ class LRUCache { } + // Marks whether tiles in the cache have been completely loaded or not. Tiles that have not been completely + // loaded are subject to being disposed early even if the are marked as used if the cache is full above its + // max size limits. setLoaded( item, value ) { const { itemSet, loadedSet } = this; From 7eb8bd61717d63b642712fc85ce3ef0c593eb7d7 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 20:54:06 +0200 Subject: [PATCH 10/14] comments --- src/base/TilesRendererBase.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/base/TilesRendererBase.js b/src/base/TilesRendererBase.js index 2db14dab1..d1090e7b1 100644 --- a/src/base/TilesRendererBase.js +++ b/src/base/TilesRendererBase.js @@ -54,7 +54,7 @@ const lruPriorityCallback = ( a, b ) => { } else if ( a.__loadingState !== b.__loadingState ) { - // dispose of tiles that are further along in the loading process + // dispose of tiles that are earlier along in the loading process first return a.__loadingState > b.__loadingState ? - 1 : 1; } else if ( a.__lastFrameVisited !== b.__lastFrameVisited ) { @@ -744,7 +744,8 @@ export class TilesRendererBase { tile.__loadingState = LOADED; lruCache.setLoaded( tile, true ); - // If the memory of the item hasn't been registered yet + // If the memory of the item hasn't been registered yet then that means the memory usage hasn't + // been accounted for by the cache yet so we need to check if it fits or if we should remove it. if ( lruCache.getMemoryUsage( tile ) === null ) { if ( lruCache.isFull() && lruCache.computeMemoryUsageCallback( tile ) > 0 ) { From 75bfab6d3d355d9c780985b806204956157358f1 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sat, 28 Sep 2024 21:00:51 +0200 Subject: [PATCH 11/14] Comments --- src/utilities/LRUCache.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index ee9d14cf9..2744d7c35 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -246,7 +246,7 @@ class LRUCache { const hasBytesToUnload = unused && this.cachedBytes > minBytesSize || unloaded && this.cachedBytes > maxBytesSize; if ( hasBytesToUnload || hasNodesToUnload ) { - // used items should be at the end of the array + // used items should be at the end of the array, "unloaded" items in the middle of the array itemList.sort( ( a, b ) => { const usedA = usedSet.has( a ); @@ -286,7 +286,8 @@ class LRUCache { let removedNodes = 0; let removedBytes = 0; - // evict up to the max node or bytes size + // evict up to the max node or bytes size, keeping one more item over the max bytes limit + // so the "full" function behaves correctly. while ( this.cachedBytes - removedBytes > maxBytesSize || itemList.length - removedNodes > maxSize From 7301bf8b35b3f8a373f9c1c811f4e0f20c850f65 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sun, 29 Sep 2024 12:08:10 +0200 Subject: [PATCH 12/14] Fix tests, ensure we only exit eviction loops when both size conditions are met --- src/utilities/LRUCache.js | 9 ++++++--- test/LRUCache.test.js | 14 +++++++------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index 2744d7c35..d7b4e3573 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -297,7 +297,8 @@ class LRUCache { const bytes = bytesMap.get( item ) || 0; if ( usedSet.has( item ) && loadedSet.has( item ) || - this.cachedBytes - removedBytes - bytes < maxBytesSize + this.cachedBytes - removedBytes - bytes < maxBytesSize && + itemList.length - removedNodes <= maxSize ) { break; @@ -309,7 +310,8 @@ class LRUCache { } - // evict up to the min size + // evict up to the min node or bytes size, keeping one more item over the min bytes limit + // so we're meeting it while ( removedBytes < bytesToUnload || removedNodes < nodesToUnload @@ -319,7 +321,8 @@ class LRUCache { const bytes = bytesMap.get( item ) || 0; if ( usedSet.has( item ) || - this.cachedBytes - removedBytes - bytes < minBytesSize + this.cachedBytes - removedBytes - bytes < minBytesSize && + removedNodes >= nodesToUnload ) { break; diff --git a/test/LRUCache.test.js b/test/LRUCache.test.js index 41705847e..20db965e2 100644 --- a/test/LRUCache.test.js +++ b/test/LRUCache.test.js @@ -52,10 +52,10 @@ describe( 'LRUCache', () => { cache.add( {}, () => {} ); expect( cache.isFull() ).toEqual( true ); - cache.unloadUnusedContent( null ); + cache.unloadUnusedContent(); expect( cache.isFull() ).toEqual( true ); cache.markAllUnused(); - cache.unloadUnusedContent( null ); + cache.unloadUnusedContent(); expect( cache.isFull() ).toEqual( false ); @@ -124,7 +124,7 @@ describe( 'LRUCache', () => { cache.minBytesSize = 5; cache.maxBytesSize = 25; cache.unloadPercent = 1; - cache.getMemoryUsageCallback = () => 4; + cache.computeMemoryUsageCallback = () => 4; for ( let i = 0; i < 10; i ++ ) { @@ -138,8 +138,8 @@ describe( 'LRUCache', () => { cache.markAllUnused(); cache.unloadUnusedContent(); - expect( cache.itemList.length ).toEqual( 1 ); - expect( cache.cachedBytes ).toEqual( 4 ); + expect( cache.itemList.length ).toEqual( 2 ); + expect( cache.cachedBytes ).toEqual( 8 ); } ); @@ -149,7 +149,7 @@ describe( 'LRUCache', () => { cache.minBytesSize = 10; cache.maxBytesSize = 25; cache.unloadPercent = 1; - cache.getMemoryUsageCallback = () => 1; + cache.computeMemoryUsageCallback = () => 1; const items = new Array( 10 ).fill().map( () => ( { priority: 1 } ) ); for ( let i = 0; i < 10; i ++ ) { @@ -165,7 +165,7 @@ describe( 'LRUCache', () => { expect( cache.cachedBytes ).toEqual( 10 ); expect( cache.itemList.length ).toEqual( 10 ); - cache.getMemoryUsageCallback = () => 4; + cache.computeMemoryUsageCallback = () => 4; for ( let i = 0; i < 10; i ++ ) { cache.updateMemoryUsage( items[ i ] ); From 47fc178bda7c61bc14e932fdcc579072d2df13e4 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Sun, 29 Sep 2024 12:13:16 +0200 Subject: [PATCH 13/14] Update tests --- test/LRUCache.test.js | 67 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/test/LRUCache.test.js b/test/LRUCache.test.js index 20db965e2..6bdef35c7 100644 --- a/test/LRUCache.test.js +++ b/test/LRUCache.test.js @@ -181,4 +181,71 @@ describe( 'LRUCache', () => { } ); + it( 'should allow for unloading "used" items if they are unloaded and above the max bytes size threshold.', () => { + + const cache = new LRUCache(); + cache.minBytesSize = 0; + cache.maxBytesSize = 5; + cache.unloadPercent = 1; + cache.computeMemoryUsageCallback = () => null; + + // insert items with unknown memory quantity + const items = new Array( 10 ).fill().map( () => ( { priority: 1 } ) ); + for ( let i = 0; i < 10; i ++ ) { + + cache.add( items[ i ], () => {} ); + + } + + expect( cache.isFull() ).toEqual( false ); + + // update all items to have a memory quantity that's over the cache limit and update the items + cache.computeMemoryUsageCallback = () => 1; + cache.itemList.forEach( item => cache.updateMemoryUsage( item ) ); + + expect( cache.isFull() ).toEqual( true ); + expect( cache.itemList.length ).toEqual( 10 ); + + cache.unloadUnusedContent(); + expect( cache.isFull() ).toEqual( true ); + expect( cache.itemList.length ).toEqual( 5 ); + + } ); + + it( 'should not unload "used" items if they are loaded and above the max bytes size threshold.', () => { + + const cache = new LRUCache(); + cache.minBytesSize = 0; + cache.maxBytesSize = 5; + cache.unloadPercent = 1; + cache.computeMemoryUsageCallback = () => null; + + // insert items with unknown memory quantity + const items = new Array( 10 ).fill().map( () => ( { priority: 1 } ) ); + for ( let i = 0; i < 10; i ++ ) { + + cache.add( items[ i ], () => {} ); + + } + + expect( cache.isFull() ).toEqual( false ); + + // update all items to have a memory quantity that's over the cache limit and update the items + cache.computeMemoryUsageCallback = () => 1; + cache.itemList.forEach( item => { + + cache.updateMemoryUsage( item ); + cache.setLoaded( item, true ); + + } ); + + expect( cache.isFull() ).toEqual( true ); + expect( cache.itemList.length ).toEqual( 10 ); + + cache.unloadUnusedContent(); + expect( cache.isFull() ).toEqual( true ); + expect( cache.itemList.length ).toEqual( 10 ); + + } ); + } ); From 16421672df2b820adb3e3ed53178db4af1a89411 Mon Sep 17 00:00:00 2001 From: Garrett Johnson Date: Mon, 30 Sep 2024 17:16:10 +0900 Subject: [PATCH 14/14] Update LRUCache.js --- src/utilities/LRUCache.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js index d7b4e3573..f6b13039c 100644 --- a/src/utilities/LRUCache.js +++ b/src/utilities/LRUCache.js @@ -146,8 +146,8 @@ class LRUCache { } // Marks whether tiles in the cache have been completely loaded or not. Tiles that have not been completely - // loaded are subject to being disposed early even if the are marked as used if the cache is full above its - // max size limits. + // loaded are subject to being disposed early if the cache is full above its max size limits, even if they + // are marked as used. setLoaded( item, value ) { const { itemSet, loadedSet } = this;