Skip to content

perf(terrain): avoid allocating tile IDs and matrices that are then discarded - #8173

Closed
Alchez wants to merge 1 commit into
maplibre:mainfrom
Alchez:perf/terrain-coords-allocation
Closed

perf(terrain): avoid allocating tile IDs and matrices that are then discarded#8173
Alchez wants to merge 1 commit into
maplibre:mainfrom
Alchez:perf/terrain-coords-allocation

Conversation

@Alchez

@Alchez Alchez commented Aug 16, 2026

Copy link
Copy Markdown

When using terrain, the tile manager is allocating a tile ID and a 16-element matrix for every renderable terrain tile, but most of it is wasted compute that adds up.

flowchart TD
    subgraph after["After — decide, then allocate"]
        A1[next terrain tile] --> A2{same, child<br/>or parent?}
        A2 -->|no| A3[continue<br/>nothing allocated]
        A2 -->|yes| A4[clone tile ID<br/>copy scratch matrix]
        A3 --> A1
        A4 --> A1
    end
    subgraph before["Before — allocate, then decide"]
        B1[next terrain tile] --> B2[clone tile ID<br/>allocate Float64Array 16]
        B2 --> B3{same, child<br/>or parent?}
        B3 -->|no| B4[continue<br/>both discarded]
        B3 -->|yes| B5[keep]
        B4 --> B1
        B5 --> B1
    end
Loading

After applying the fix, every frame should save about 0.15-1.05ms of compute (which is about 1-7% of a 60FPS budget).

Bench results:

renderable tiles before (hz) after (hz) ratio
15 8,076 221,123 27.4×
27 2,386 105,658 44.3×
64 373 27,834 74.6×

The factors seem big, but they're only in-function improvements, and this function isn't called very many times for a frame, so effective improvement is still only the range noted above.


Related to:

Launch Checklist

  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Write tests for all new functionality.
  • If you changed code in a file that has a benchmark file next to it (*.bench.ts), post before/after results of npm run bench (the compare workflow is in test/bench/README.md).
  • Add an entry to CHANGELOG.md under the ## main section.
  • Confirm you have read our AI policy here.

Assisted-By: Claude Opus 5

@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.92%. Comparing base (4529c6e) to head (6b191d5).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8173   +/-   ##
=======================================
  Coverage   93.91%   93.92%           
=======================================
  Files         290      290           
  Lines       24915    24916    +1     
  Branches     6575     6575           
=======================================
+ Hits        23399    23402    +3     
+ Misses       1516     1514    -2     

☔ View full report in Codecov by Harness.
📢 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

HarelM commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

This doesn't sound like a big perf gain. can you share the bench results?

@Alchez

Alchez commented Aug 17, 2026

Copy link
Copy Markdown
Author

@HarelM sorry I forgot to post the bench results. I'll add it to the description.

This doesn't sound like a big perf gain.

By itself, I agree that it's not a significant improvement. But I'd argue for two things:

  • It's worth going in as in incremental improvement anyway, which may compound with other perf improvements.
  • This change also makes this function consistent with the rest of the library. In the same file, the following function already filters before allocating:

_getTerrainCoordsForTileRanges(
tileID: OverscaledTileID,
terrainTileRanges: {[zoom: string]: CanonicalTileRange}
): Record<string, OverscaledTileID> {
const coords: Record<string, OverscaledTileID> = {};
for (const key of this._renderableTilesKeys) {
const terrainTileID = this._tiles[key].tileID;
if (!this._isWithinTileRanges(terrainTileID, terrainTileRanges)) {
continue;
}
const coord = tileID.clone();
const mat = createMat4f64();
if (terrainTileID.canonical.z === tileID.canonical.z) {

@HarelM

HarelM commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Since this has the risk of creating issue in the future I don't see this as a real perf improvement worth pursuing. I prefer to focus on things that will make the terrain work faster on mobile devices, which is where it currently suffers...

@Alchez

Alchez commented Aug 17, 2026

Copy link
Copy Markdown
Author

@HarelM okay, that's fair. I'll close this one. I do have another separate terrain improvement that may be more applicable for mobile performance. I'll raise it shortly.

@Alchez Alchez closed this Aug 17, 2026
@Alchez
Alchez deleted the perf/terrain-coords-allocation branch August 17, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants