Improve, optimize, and consolidate Tile LRU cache to new ES6 Map #6731

wayofthefuture · 2025-11-17T21:04:07Z

Since ES6, Map has made simple in-memory caching far easier, providing fast average-case O(1) access. Our previous tile cache relied heavily on O(n) array operations. This PR replaces that implementation with the existing BoundedLRUCache and wires it into TileCache. I evaluated several npm caching libraries, but none fit well with the specifics of MapLibre’s tile loading model or offered a good balance of size and simplicity.

The most critical operation used in TileManager, getAndRemove, previously incurred three O(n) operations per call. It is now implemented as take, which performs the same behavior in O(1). Additionally, the old tile cache maintained many concurrent timers for tiles sitting idle in the cache. The new cache no longer tracks per-tile timers; instead, expired tiles are evicted/unloaded lazily on access, avoiding the overhead of a large number of active timers.

O(N) Comparison Table

Operation	Complexity (v1 → v2)	Potential Speedup
Get	O(1) → O(1)	~1×
Set	O(1) → O(1)	~1×
getAndRemove / Take	O(N) → O(1)	Up to N× faster
Remove (by key)	O(N) → O(1)	Up to N× faster
Filter	O(N²) → O(N)	Up to N× faster
Resize (setMaxSize)	O(N²) → O(N)	Up to N× faster
Reset / Clear	O(N) → O(N)	~1×

Timers Comparison Table

Aspect	v1 (Timer-based cache)	v2 (LRU cache, no timers)
Timers per tile	1 per entry (hundreds–thousands)	0
Event-loop overhead	High (timer scheduling + callbacks)	None
GC pressure	High (many closures + timer objects)	Low
Callback churn	Thousands of callbacks	None
Memory per tile	Large (timeout + closure)	Small (Map entry only)
Latency/jank risk	High	Low

Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
Briefly describe the changes in this PR.
Write tests for all new functionality.
Add an entry to CHANGELOG.md under the ## main section.

codecov · 2025-11-17T21:15:17Z

Codecov Report

❌ Patch coverage is 97.95918% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.42%. Comparing base (a2add46) to head (6b1d576).

Files with missing lines	Patch %	Lines
src/tile/tile_manager.ts	93.54%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6731      +/-   ##
==========================================
+ Coverage   92.40%   92.42%   +0.02%     
==========================================
  Files         288      288              
  Lines       23775    23761      -14     
  Branches     5055     5050       -5     
==========================================
- Hits        21969    21961       -8     
+ Misses       1806     1800       -6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

HarelM · 2025-11-17T21:23:47Z

src/tile/bounded_lru_cache.ts

+        }
+
+        // Updating same key with a new tile - remove old tile and add new one
+        this.remove(key);


Should this call onRemove? I'm honestly asking, I don't know the right answer.

I believe so:

https://github.com/maplibre/maplibre-gl-js/blob/main/src/tile/tile_cache.ts#L82-L85

Eventually on remove would be called for the same id.

onRemove propagates back to source.unloadTile which then goes back to the worker_source.removeTile. In here, loaded is used to store tiles by uid in the worker. uid is created in tile.js using uniqueId() which is an integer count++ on each function run, so every tile object has a new and unique uid. Even though the new tile may have the same tileID, it will absolutely have a unique uid, which means it still needs to be removed from the this.loaded cache in the worker. In other words, onRemove needs to be called otherwise the tiles would collect in the worker. Do some tracing and let me know if you agree...

Ok, I think I know what should be the right approach here in order to get confident in the current solution.

We need to cover the current cache with tests so that we know exactly how it operates, which callbacks are called when and how it behaves in general (assuming this is not the tests that currently exists, if the tests cover the current functionality then we can skip this step).

After we have the tests in place, we can simply replace the implementation under the hood - no changes to logic (besides maybe the timers), without even changing the public API (take vs getAndRemove, clear vs reset etc)

After this is passing and we know that we haven't changed any logic, we can rename stuff.

I think this is the safest way to handle this.
If this is how you did it, then there's very low risk in introducing bugs, as the process above is fairly safe.

I've read the cache code start to end and a few questions rose:

Why does it have an array? Most of the usage is referring to the first element, but not all.

If the timeout were to be removed, how will this affect the code that listens to onRemove? What will happen if the tile's onRemove is not called when the tile expiries?

The functionally as covered in the current tests (not those you added, but those that exist) is far from ideal, there's even a comment about how the cache should behave but it's not covered in a test, only in a comment, so not a great start...

If you are good with keeping both 'raster' and 'vector' handled in tile_manager, then we could extract all retainment logic into tile_retainment.ts... versus having separate managers. This would cut out about 30% of the file. Just another idea...

I think you'll need to pass "this" into this file, which is an anti-pattern.
Also regarding the above name I think there are simply two "storage" types: in-view-tiles and out-of-view-tiles, the current cache us the out of view as far as I understand.
But I want to better understand other parts as well.

I think you can just pass this._tiles and retain, and change getLoadedTile to tile.hasData(), then you don't need to pass this...

Current cache is tiles that were loaded for a previous view... this._tiles is the current view. As tiles move out of view they are added to the cache...

Covering tiles -> add tile -> check the cache -> remove other this._tiles that aren't covering/substitute -> place them into cache -> repeat...

Yeah, this is also my understanding.
I wonder why there is a usage counter for a tile though...
In any case, a first step can be to move these parts to separate files as you suggested, onne for raster and one for vector.
Also renaming _tiles to inViewTiles and cache to outOfViewCache so that is clear why these two variables are needed and how they are different.

'uses' to me seems like a hack that should be deprecated... if it doesn't work without it then we are missing something...

HarelM · 2025-11-23T12:34:20Z

An initial stab at moving some code around. Not a big change overall...

Refactor some code out of tile manager #6756

wayofthefuture · 2025-11-28T03:36:45Z

Well I resolved the merge conflicts and merged your split-tile-manager branch but now the 'files changed' page looks polluted. I guess after you merge your branch into main and I update this one it will be easier to read the changes...

HarelM · 2025-11-28T09:18:43Z

I've merged it to main, so you'd have to resolve conflicts again, sorry...

# Conflicts: # src/style/style.ts # src/tile/tile_manager.test.ts # src/tile/tile_manager.ts

wayofthefuture · 2025-11-28T17:33:08Z

Got it merged... but still needs a little thought regarding remove/onRemove...

HarelM · 2025-11-28T22:40:14Z

src/tile/tile_manager.ts

        }

-        this._outOfViewCache.reset();
+        // Clear the tile cache


I don't think this comment is needed...

HarelM · 2025-11-28T22:45:05Z

src/tile/tile_manager.ts

        }

-        this._outOfViewCache.reset();
+        // Clear the tile cache


Not needed...

HarelM · 2025-11-28T22:53:33Z

I still think adding tests to current implementation is necessary to make sure we don't miss on a use case, for example maybe the cache array is used to store different wrap instances of a tile on the same key? IDK...

wayofthefuture · 2025-11-28T22:55:02Z

Ok.

Different wrap instance of tile on same key.
Tile is actually expired on removal and not used (done).
Any others?

HarelM · 2025-11-28T22:57:17Z

Current implementation, as in the old one, I know I don't fully understand the old behavior, so it's important to make sure we understand it before replacing it.

consolidate, optimize, add functionality, tests

59be754

HarelM reviewed Nov 17, 2025

View reviewed changes

wayofthefuture force-pushed the improve-lru branch from 52c774d to 59be754 Compare November 28, 2025 17:23

wayofthefuture added 2 commits November 28, 2025 11:26

Merge branch 'main' into improve-lru

c19f863

# Conflicts: # src/style/style.ts # src/tile/tile_manager.test.ts # src/tile/tile_manager.ts

constructor

0d9975c

wayofthefuture marked this pull request as ready for review November 28, 2025 17:33

public/private

6b1d576

HarelM reviewed Nov 28, 2025

View reviewed changes

src/tile/tile_manager.ts

}

this._outOfViewCache.reset();

// Clear the tile cache

Copy link

Collaborator

HarelM Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment is needed...

HarelM reviewed Nov 28, 2025

View reviewed changes

src/tile/tile_manager.ts

}

this._outOfViewCache.reset();

// Clear the tile cache

Copy link

Collaborator

HarelM Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed...

Uh oh!

Improve, optimize, and consolidate Tile LRU cache to new ES6 Map #6731

Are you sure you want to change the base?

Improve, optimize, and consolidate Tile LRU cache to new ES6 Map #6731

Uh oh!

Conversation

wayofthefuture commented Nov 17, 2025

O(N) Comparison Table

Timers Comparison Table

Uh oh!

codecov bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wayofthefuture Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HarelM commented Nov 23, 2025

Uh oh!

wayofthefuture commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HarelM commented Nov 28, 2025

Uh oh!

wayofthefuture commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HarelM commented Nov 28, 2025

Uh oh!

wayofthefuture commented Nov 28, 2025

Uh oh!

HarelM commented Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Nov 17, 2025 •

edited

Loading

wayofthefuture Nov 17, 2025 •

edited

Loading

wayofthefuture commented Nov 28, 2025 •

edited

Loading

wayofthefuture commented Nov 28, 2025 •

edited

Loading