Further optimize cross tile symbol index `findMatches` #6641

bradymadden97 · 2025-10-31T18:23:55Z

Issue

When zooming in or out over a source that has a large number of symbols with the same label (or no label), maplibre will hit an extremely slow path trying to find and pair symbols across tiles (for fading behavior). This will result in noticable stalls and frame drops while zooming. This is also documented in the linked issues below.

Demo of issue

Screen.Recording.2025-10-31.at.3.10.30.PM.1.mov

(this is using https://jsbin.com/girobohihu/1/edit?html,console,output which is the simplest repro I could create. I've seen much worse stalls in production and the linked issues have some examples as well). The TLDR of the issue is:

Many symbols near eachother
All with same label or no label at all
Zoom in/ out quickly
See stallling

Background

Essentially, as I understand it, what findMatches is doing is iterating through all the symbols on the current tile, and all the symbols on a future tile (at a different zoom level, depending on if we're zooming in or out) and trying to pair as many symbols from the starting tile and the destination tile as possible. When symbols are "paired", the symbol on the destination tile will not "fade in". If the symbols did always "fade in" it gives a weird experience while zooming. The video below mocks what it would look like if we never paired any symbols while zooming:

Screen.Recording.2025-10-31.at.2.17.04.PM.1.mov

You can see the symbol flash every time we enter a new zoom level, as it's reloaded and unpaired/transitioned from the previous zoom level.

Compare that to the current state:

Screen.Recording.2025-10-31.at.2.18.43.PM.1.mov

So I think we established the purpose of the code, now let's talk about the pathologically slow case. The slow case happens when we have a bunch of symbols on the origin tile that need to be matched with a bunch of symbols on the destination tile. This happens when all symbols have the same label, as the label is what is used to match symbols. However, unlabeled symbols are also essentially treated as being "the same label" for this matching purpose, so a simple pathologically slow case can occur any time we just have a bunch of symbols with no label in the same tile.

We tried to optimize this exact case in #1755 by building a spatial index per unique symbol key (aka label, or no label) (source). But then we ultimately re-run the range query - which is the slow path as highlighted in some of the linked issues - for every single symbol instance (of which there are many) [source].

There was a PR to attempt to exit early from these range queries after we've found 1 result (see here), but I actually don't think this solution would solve this optimally. You wouldn't necessarily want to only fetch 1 result from your range query, as it's possible that first result would have already been paired with a different symbol instance at the same zoom level with the same key. So then the question becomes: "How many do you fetch?", and there is no answer that can be correct besides "All of them", which is what we're currently doing, and is currently slow.

Changes in this PR

That brings us to the changes in this PR. The idea here is to find a way to avoid running the slow range query N times, where N is the number of symbol instances with the same symbol key. We want to run the range query for a given symbol key once, and then pair as many of the results with as many symbol instances as possible.

Effectively the pseduo code changes from:

// assuming all symbols have the same key

for symbol in symbols:
  let entries = index.range(symbol.location);
  for entry in entries:
    if entry.isNotAlreadyPaired:
      symbol.pair = entry
      break;

to

// assuming all symbols have the same key

for location in locationToSymbols:
  let entries = index.range(location);
  loop until entries or location.symbols is exhausted:
    location.symbols[index] = entries[index];
    index++

Before vs. After

Running performance profiles on this example https://jsbin.com/girobohihu/1/edit?html,console,output

The code to run locally (which is where the screenshots themselves came from) is here:

pnpm run dev:repro slow-cross-tile-symbol-index

Manual Regression Test

I performed a manual regression test with labels to confirm our matching behavior still works with this new algorithm. You can see 52 does not fade in/out which implies it is being matched across tiles:

Screen.Recording.2025-10-31.at.3.15.05.PM.2.mov

Benchmark

Wrote a small benchmark to demonstrate the speedup. There is a third comparison, to an internal fork we have that had some additional optimizations beyond what is currently in main, but this improvement exceeds the improvement on that fork.

This was ran last month, against 5.7, but there have been no changes to this file since then, besides #6635 which was just a rename, so they should still be valid.

let X = 10 // 100, 1000, 2000, 3000, 10_000, etc.
export class CrossTileSymbolIndexX extends Benchmark {

    bench() {
        const index = new Index();

        const mainID = new OverscaledTileID(6, 0, 6, 8, 8);
        const childID = new OverscaledTileID(7, 0, 7, 16, 16);

        const mainInstances: any[] = [];
        const childInstances: any[] = [];

        for (let i = 0; i < X; i++) {
            mainInstances.push(makeSymbolInstance(0, 0, ''));
            childInstances.push(makeSymbolInstance(0, 0, ''));
        }
        const mainTile = makeTile(mainID, mainInstances);
        const childTile = makeTile(childID, childInstances);
        index.addLayer(styleLayer as any, [mainTile], 0);
        index.addLayer(styleLayer as any, [childTile], 0);
    }
};

Iterations	Maplibre 5.7	Maplibre 5.7 + PR	Speedup
1k			5.7: 233x Pal: 20x
2k		Not run, see 3,000	See 3,000
3k			5.7: 661x Pal: 55x
10k	Times out / never finishes		5.7: n/a Pal: 111x

Related issues

Launch Checklist

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.
Link to related issues.
Include before/after visuals or gifs if this PR includes visual changes.
Write tests for all new functionality.
Document any changes to public APIs.
Post benchmark scores.
Add an entry to CHANGELOG.md under the ## main section.

codecov · 2025-10-31T18:35:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.22%. Comparing base (d0a0839) to head (3d218f3).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6641      +/-   ##
==========================================
+ Coverage   92.21%   92.22%   +0.01%     
==========================================
  Files         285      285              
  Lines       23663    23697      +34     
  Branches     5022     5030       +8     
==========================================
+ Hits        21820    21854      +34     
  Misses       1843     1843

☔ 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-01T14:19:50Z

Thanks for taking the time to solve this!
Is there a chance to write a test to cover this scenario or this is a "proper" refactoring so no changes to logic besides performance optimization?

HarelM · 2025-11-01T14:26:24Z

src/symbol/cross_tile_symbol_index.ts

+                // used to key indexes. For each symbol index key, build a reverse map
+                // of the scaled coordinates back to a set of SymbolInstance that
+                // resolve to that coordinate.
+                const scaledCoordinateId = scaledSymbolCoord.x << 16 | scaledSymbolCoord.y;


Do we have to use numbers as keys?

Couldn't figure out another way to use the coordinates themselves as keys in a relatively efficient way

There's the tileID.key that is usually used, can it be used here?

This coordinate is within a given tile, so not equivalent to tileID.key I don't think.

Can we use strings maybe? I'm terrified of number overflows when I see this kind of code? How worse is the performance if this is string instead of number?

Changed to doubly nested map by x/y coordinates if that's better 964916c

src/symbol/cross_tile_symbol_index.ts

HarelM · 2025-11-01T14:29:01Z

The explanation in the PR is just amazing! Thanks for all the work on this, it really helps reviewing this change.

…//github.com/bradymadden97/maplibre-gl-js into bmadden/cross-tile-symbol-index-optimization

… bmadden/cross-tile-symbol-index-optimization

bradymadden97 · 2025-11-01T20:34:41Z

Is there a chance to write a test to cover this scenario or this is a "proper" refactoring so no changes to logic besides performance optimization?

Found a small bug in the logic that the updates to the test caught. Fixed in the latest version. Test also passes on main. I think this update to the test should cover the refactor logic.

HarelM · 2025-11-03T11:57:19Z

Can you add a changelog entry please?

src/symbol/cross_tile_symbol_index.ts

HarelM · 2025-11-03T12:11:16Z

I've added a few minor comments. the map of maps is a bit confusing to understand so I would recommend adding some docs and export some part of the method to smaller methods with good documentation.
Other than that, this looks good!
Thanks for all the work on this!

…//github.com/bradymadden97/maplibre-gl-js into bmadden/cross-tile-symbol-index-optimization

bradymadden97 · 2025-11-10T04:09:35Z

Ok I refactored as suggested to try and pull the indexed and non-indexed codepaths apart from each other. Let me know if it's a bit clearer what's going on now.

Will follow up with some tests specifically for indexed path tomorrow as suggested.

src/symbol/cross_tile_symbol_index.ts

…//github.com/bradymadden97/maplibre-gl-js into bmadden/cross-tile-symbol-index-optimization

bradymadden97 · 2025-11-10T17:19:44Z

Added a test suite that mimics the non-indexed code paths but should hit the indexed code paths.

CHANGELOG.md

src/symbol/cross_tile_symbol_index.test.ts

HarelM · 2025-11-10T20:48:44Z

src/symbol/cross_tile_symbol_index.test.ts

+
+        // does not match Windsor because of different key
+        const windsorInstance = childInstances.find(i => i.key === 'Windsor');
+        expect(windsorInstance.crossTileID).toEqual(131);


This test has too many expect calls, I would recommend splitting this test and use beforeEach to arrange all the relevant information if it is similar between the tests.
The comments above each paragraph is a good indication that this needs to be split and the comment should probably be the test name.

Was copying the structure of the section of tests above, which do have interleaved expects and do not use beforeEach. I would suggest the structures remain the same actually, in my opinion. Do you agree / would you prefer me to include beforeEach refactor for the existing tests in the other decribe block as well?

I think it would be better to make the file similar to other files and also make sure it is consistent within the same file, meaning change to use beforeEach etc.
Having said that, I don't want to force you to rewrite the entire file, so at least change the tests you added, it would be great if you would change the other tests as well, but I would understand if you decide not to.

src/symbol/cross_tile_symbol_index.ts

HarelM · 2025-11-10T20:56:56Z

src/symbol/cross_tile_symbol_index.ts

-                    }
+            const xMap = instancesByScaledCoordinate.get(scaledSymbolCoord.x);
+            if (xMap) {
+                const yMap = xMap.get(scaledSymbolCoord.y);


is yMap a map or an array? I don't see method related to Map when using yMap, am I missing anything?

Does this rename improve it or not? 5c2c793

It's a doubly nested map (since I need to group instances by their x,y coordinates). Happy to change to other suggestions, or clarify if the reasoning still doesn't make sense

instancesByScaledCoordinate is a map, xMap is a map, but yMap is an array, am I reading it wrong?

I don't think 5c2c793 make is more readable unfortunately.
maybe just rename yMap to instancesArray? or simply instances?
Man this is so confusing...
I think the names should match the below for loops, but I'm not sure they do...

HarelM · 2025-11-10T21:11:58Z

src/symbol/cross_tile_symbol_index.ts

+        // at the same location.
+        for (const [x, yMap] of instancesByScaledCoordinate.entries()) {
+            for (const [y, instances] of yMap.entries()) {
+                const indexes = entry.index.range(


I tried to compare this to the previous code and I'm not sure I understand why there will be less calls to range, can you elaborate on this?

Absolutely! So the situation where this is faster is because we're now running a range query only once for each coordinate position that has any symbol instances within it. Then we are matching all symbol instances at that position with any results from the range query.

Previously, we would independently run a range query for every single symbol instance, and then match a result from that range query with the symbol instance.

A pathological case would be 10,000 symbol instances at the exact same coordinate. Previously that would be 10,000 range queries. Now that would be 1 range query.

Based on that algorithm, it appears to never be more range queries than before, but in bad cases it can be significantly less.

But this is for x and y that are exactly the same, right? If all the x and y are in slightly different places then it would behave as before, right?
I'm just trying to make sure I understand what you mean and where this will improve performance.

HarelM · 2025-11-10T21:12:51Z

Sorry for dragging this PR, I just really want to understand what you did here...

bradymadden97 · 2025-11-10T21:24:00Z

Sorry for dragging this PR, I just really want to understand what you did here...

Not a problem at all, I similarly want to make the code understandable so thank you for the feedback. Happy to keep iterating until it's in a good place.

bradymadden97 · 2025-11-10T21:28:55Z

package-lock.json

      "dev": true,
-      "license": "MIT",
-      "peer": true
+      "license": "MIT"


Not sure what's going on with these generated changes.

I would advise to simply revet this file...

Probably different version of node...

bradymadden97 added 2 commits October 31, 2025 13:32

Optimize cross tile index searching

efaaea5

newline

c834bd2

bradymadden97 changed the title ~~Bmadden/cross tile symbol index optimization~~ Further optimize cross tile symbol index findMatches Oct 31, 2025

bradymadden97 marked this pull request as ready for review October 31, 2025 19:18

This was referenced Oct 31, 2025

Bad performance with many overlapping symbols #6192

Open

Zoom perfomance with symbol layer type #3084

Open

Optimize cross tile symbol index bradymadden97/maplibre-gl-js#1

Closed

HarelM reviewed Nov 1, 2025

View reviewed changes

src/symbol/cross_tile_symbol_index.ts Outdated Show resolved Hide resolved

bradymadden97 added 7 commits November 1, 2025 13:42

clarify comments

1c0d4f2

Fix iteration logic

4600214

Revert

6dda944

Merge branch 'bmadden/cross-tile-symbol-index-optimization' of https:…

77a9904

…//github.com/bradymadden97/maplibre-gl-js into bmadden/cross-tile-symbol-index-optimization

Add check to test

91ad4b2

Merge branch 'bmadden/add-check-to-cross-tile-symbol-index-test' into…

88ae4e0

… bmadden/cross-tile-symbol-index-optimization

Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization

0c0f587

bradymadden97 requested a review from HarelM November 1, 2025 20:35

Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization

392f7d3

HarelM reviewed Nov 3, 2025

View reviewed changes

src/symbol/cross_tile_symbol_index.ts Outdated Show resolved Hide resolved

HarelM reviewed Nov 3, 2025

View reviewed changes

src/symbol/cross_tile_symbol_index.ts Outdated Show resolved Hide resolved

bradymadden97 added 4 commits November 5, 2025 10:15

Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization

c1dc44f

Merge branch 'bmadden/cross-tile-symbol-index-optimization' of https:…

99c7530

…//github.com/bradymadden97/maplibre-gl-js into bmadden/cross-tile-symbol-index-optimization

update for comments & add changelog

b0e7909

Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization

1b92449

refactor for clarity

1134558

indent

6574aee

HarelM reviewed Nov 10, 2025

View reviewed changes

src/symbol/cross_tile_symbol_index.ts Outdated Show resolved Hide resolved

HarelM reviewed Nov 10, 2025

View reviewed changes

src/symbol/cross_tile_symbol_index.ts Outdated Show resolved Hide resolved

Remove double space

4cf39a2

HarelM reviewed Nov 10, 2025

View reviewed changes

src/symbol/cross_tile_symbol_index.ts Outdated Show resolved Hide resolved

bradymadden97 added 7 commits November 10, 2025 10:53

Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization

f4d0304

rename and types

4beaad2

Merge branch 'bmadden/cross-tile-symbol-index-optimization' of https:…

386288a

…//github.com/bradymadden97/maplibre-gl-js into bmadden/cross-tile-symbol-index-optimization

longer comment

578fdd8

pull out

20eca82

xy map

964916c

add tests specifically for indexing

4989773

bradymadden97 requested a review from HarelM November 10, 2025 17:19

bradymadden97 commented Nov 10, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

a954ea8

HarelM reviewed Nov 10, 2025

View reviewed changes

src/symbol/cross_tile_symbol_index.test.ts Outdated Show resolved Hide resolved

HarelM reviewed Nov 10, 2025

View reviewed changes

src/symbol/cross_tile_symbol_index.ts Outdated Show resolved Hide resolved

HarelM reviewed Nov 10, 2025

View reviewed changes

rename

5c2c793

bradymadden97 added 2 commits November 10, 2025 16:25

Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization

880833a

pull out to method

f729a85

bradymadden97 commented Nov 10, 2025

View reviewed changes

toBeDefined

3d218f3

Uh oh!

Further optimize cross tile symbol index findMatches #6641

Are you sure you want to change the base?

Further optimize cross tile symbol index findMatches #6641

Conversation

bradymadden97 commented Oct 31, 2025 • edited by HarelM Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Demo of issue

Background

Changes in this PR

Before vs. After

Manual Regression Test

Benchmark

Related issues

Launch Checklist

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

HarelM commented Nov 1, 2025

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!

Uh oh!

HarelM commented Nov 1, 2025

Uh oh!

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

Uh oh!

Uh oh!

HarelM commented Nov 3, 2025

Uh oh!

Uh oh!

Uh oh!

HarelM commented Nov 3, 2025

Uh oh!

bradymadden97 commented Nov 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bradymadden97 commented Nov 10, 2025

Uh oh!

Uh oh!

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!

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 10, 2025

Uh oh!

bradymadden97 commented Nov 10, 2025

Further optimize cross tile symbol index `findMatches` #6641

Further optimize cross tile symbol index `findMatches` #6641

bradymadden97 commented Oct 31, 2025 •

edited by HarelM

Loading

codecov bot commented Oct 31, 2025 •

edited

Loading

bradymadden97 commented Nov 1, 2025 •

edited

Loading