Skip to content

Optimize CrossTileSymbolIndex by leveraging promoteId to automatically pair known feature symbols across tiles - #7665

Draft
bradymadden97 wants to merge 10 commits into
maplibre:mainfrom
bradymadden97:bmadden/cross-tile-id-from-feature
Draft

Optimize CrossTileSymbolIndex by leveraging promoteId to automatically pair known feature symbols across tiles#7665
bradymadden97 wants to merge 10 commits into
maplibre:mainfrom
bradymadden97:bmadden/cross-tile-id-from-feature

Conversation

@bradymadden97

@bradymadden97 bradymadden97 commented May 20, 2026

Copy link
Copy Markdown
Contributor

Original context: #6641

When there are a large number of symbols with the same label (or no label) on the same tile, we currently run a very slow algorithm to try to match symbols to a "cross tile symbol" at the next highest or lowest zoom level as you zoom in or out. This is to prevent symbols from fading out and back in as one zoom level's tile is faded out and another fades in.

Thanks to the great comment here, #6641 is OBE and this approach can be used, which is much simpler. If a feature has a promoteId, just use that for matching across tiles. If not, it continues using the existing (slow) case.

(In my case, I know my features have IDs, so this optimization is perfect).

Without promoteId - you can clearly see the huge stall when a bunch of symbols share a tile:

Screen.Recording.2026-05-20.at.3.53.09.PM480.mov

With promoteId - the stall completely goes away:

Screen.Recording.2026-05-20.at.3.52.34.PM480good.mov

I wrote a benchmark in #7664. Now re-ran with these changes:

Ran:

npx st --no-cache -H localhost --port 9966 .
node --no-warnings --experimental-transform-types test/bench/run-benchmarks.ts --compare '' CrossTileSymbolIndex

Outputs:

Starting headless chrome at: http://localhost:9966/test/bench/versions/index.html?compare=
                      bmadden/cross-tile-id-from-feature-dirty ef4ed4d 
 CrossTileSymbolIndex                                       0.3233 ms 

Compare to original benchmark here: #7664 this is a 2,500x speedup.


Public API changes:

  • This PR does not change any public APIs. The promoteId option already exists and is already documented. The optimization is entirely internal — the same inputs produce the same visual output, just faster. No new options, methods, or behavior changes are exposed to users.

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.
  • Confirm you have read our AI policy here.

@bradymadden97
bradymadden97 marked this pull request as ready for review May 21, 2026 14:11
@HarelM

HarelM commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Can you point me to where the cross tile id is assigned? Maybe draw a small diagram of the data flow? I'm having a hard time understanding how this works...
Not that I doubt the excellent work you did, just trying to understand what the limitation of this, maybe memory concerned etc...

@bradymadden97

Copy link
Copy Markdown
Contributor Author

Will do. One note on memory - these maps are defined at the source level (during one iteration of this PR they were a global map), so I think that's an improvement as now they get gc'd away with the source if it gets removed.

I'll see if I can get to your Q before end of the week.

@HarelM

HarelM commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks! Don't worry about the flakey render tests...

numVerticalIconVertices,
useRuntimeCollisionCircles,
0,
crossTileID,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this has the possibility to overflow? I've seen some nasty bugs related to integer overflow when using this emplaceBack method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think only in the same way the existing (global) crossTileID (here) has the possibility to overflow. Except this is a counter per feature per source, so you'd need 2^32 features in a source.

const variableAnchorOffset = getTextVariableAnchorOffset(layer, feature, canonical);
const [textAnchorOffsetStartIndex, textAnchorOffsetEndIndex] = addTextVariableAnchorOffsets(bucket.textAnchorOffsets, variableAnchorOffset);

const crossTileID = feature.id != null && layer.id && crossTileIDs ? getCrossTileID(`${feature.id}-${layer.id}`, crossTileIDs) : 0;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this already use the promoteId?

@HarelM

HarelM commented May 21, 2026

Copy link
Copy Markdown
Collaborator

I see, so this is basically assigning ids in case it's possible, the rest is handled as it was before when ids can be used.
So the only concern I have is with the integer overflow and the lifecycle of this map - i.e. when it is getting cleaned.

@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.14%. Comparing base (a93eb96) to head (4bba6d4).
⚠️ Report is 53 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7665      +/-   ##
==========================================
+ Coverage   95.09%   95.14%   +0.04%     
==========================================
  Files         289      288       -1     
  Lines       24488    24497       +9     
  Branches     6462     6470       +8     
==========================================
+ Hits        23288    23308      +20     
+ Misses       1200     1189      -11     

☔ 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 May 23, 2026

Copy link
Copy Markdown
Collaborator

A new version of KDbush was just released, I'm not sure if it's relevant to this PR but you were trying to improve performance there, so I thought it would be worth mentioning...

bradymadden97 and others added 2 commits June 7, 2026 14:32
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bradymadden97

Copy link
Copy Markdown
Contributor Author

Here's the data flow for how crossTileID gets pre-assigned in this PR.

The problem

The existing CrossTileSymbolIndex runs on the main thread after tile parsing. When a new bucket arrives, it tries to match each symbol to symbols from other zoom levels by spatial proximity (anchor position + label key). When many symbols share the same key (e.g. all have the same label or no label), this becomes very slow — it's doing pairwise coordinate comparisons or KDBush range queries across potentially thousands of symbols.

The optimization

If we know a feature's identity (via promoteId → feature.id), we can assign a stable crossTileID during layout on the worker thread, before the main-thread matching ever runs. The same feature parsed in tile z/x/y and tile z+1/x'/y' gets the same ID, so no spatial matching is needed.

Data flow

┌─────────────────────────────────────────────────────────────┐
│  WORKER THREAD                                              │
│                                                             │
│  GeoJSONWorkerSource                                        │
│    ._crossTileIDs: Map<string, number>  ◄── lives here,     │
│    │                                        persists across │
│    │                                        all tile parses │
│    │                                                        │
│    ▼                                                        │
│  _parseWorkerTile()                                         │
│    │  only passes map if params.promoteId is set            │
│    ▼                                                        │
│  WorkerTile.parse()                                         │
│    │                                                        │
│    ▼                                                        │
│  performSymbolLayout({ ..., crossTileIDs })                 │
│    │                                                        │
│    ▼                                                        │
│  addSymbol()                                                │
│    │                                                        │
│    │  feature.id != null && crossTileIDs?                   │
│    │  ┌─YES──────────────────────────┐                      │
│    │  │ key = `${feature.id}-${layer.id}`                   │
│    │  │ crossTileID = getCrossTileID(key, map)              │
│    │  │   └─ map.get(key) ?? (map.size + 1)                 │
│    │  └──────────────────────────────┘                      │
│    │  ┌─NO───────────────────────────┐                      │
│    │  │ crossTileID = 0  (old path)  │                      │
│    │  └──────────────────────────────┘                      │
│    │                                                        │
│    ▼                                                        │
│  bucket.symbolInstances.emplaceBack(..., crossTileID, ...)  │
│    (stored as Uint32)                                       │
│                                                             │
└──────────────────────────┬──────────────────────────────────┘
                           │  tile result sent to main thread
                           ▼
┌─────────────────────────────────────────────────────────────┐
│  MAIN THREAD                                                │
│                                                             │
│  CrossTileSymbolIndex.addBucket()                           │
│    │                                                        │
│    ▼                                                        │
│  symbolInstance.crossTileID ??= 0                           │
│    (was `= 0` before — this preserves pre-assigned IDs)     │
│    │                                                        │
│    ▼                                                        │
│  findMatches() loop:                                        │
│    if (symbolInstance.crossTileID) → skip ✓  (fast path)    │
│    if (!symbolInstance.crossTileID) → spatial match (old)   │
│    │                                                        │
│    ▼                                                        │
│  Any remaining symbols with crossTileID === 0               │
│  get a new ID from CrossTileIDs.generate()  (old path)      │
│                                                             │
└─────────────────────────────────────────────────────────────┘

Why this is correct

The same feature appearing in multiple tiles (e.g. at different zoom levels, or in adjacent tiles) will always produce the same "${feature.id}-${layer.id}" key, so it gets the same crossTileID from the map. This is exactly what the main-thread spatial matching was trying to achieve — just without the expensive coordinate comparisons.

Memory / lifecycle

The _crossTileIDs map lives on GeoJSONWorkerSource — one per GeoJSON source. It grows with unique featureId × layerId combinations (not tiles), never shrinks, and gets GC'd when the source is removed. For a source with 100K features and a few symbol layers, it's a few MB at most.

The map must be append-only — if we evicted entries, two tiles containing the same feature could end up with different IDs.

Limitations

  • Only works when promoteId is set (features need stable IDs). Without it, falls through to the existing spatial matching.
  • Only implemented for GeoJSONWorkerSource in this PR — VectorTileWorkerSource could get the same treatment in a follow-up.
  • The worker-side counter and main-thread CrossTileIDs counter are independent ID spaces, but that's fine — symbols with pre-assigned IDs skip findMatches entirely, so the two spaces never interact.

@bradymadden97

Copy link
Copy Markdown
Contributor Author

Found a possible regression with multi-point / line geometries:

For symbol-placement: 'line', a single feature generates multiple anchors, and addSymbol() is called once per anchor. Since the key is "${feature.id}-${layer.id}" (no anchor disambiguation), all anchors from the same feature get the same crossTileID.

In placeLayerBucketPart() at line 463:

if (seenCrossTileIDs[symbolInstance.crossTileID]) return;

This means only the first anchor would be placed; the rest would be skipped as "duplicates." For line-placement labels (e.g., road names repeated along a highway), this would suppress all but one label per feature.

Will need to look into testing this a bit more I think.

@bradymadden97
bradymadden97 marked this pull request as draft June 7, 2026 18:51
@HarelM

HarelM commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the update, this is not covered by existing render tests, right? Maybe a different PR to add this test would make sense.
Thanks for looking into this.
It's worth updating the PR name to indicate this is only relevant for geojson source some how, as this might be implied by promoteId, but it's not obvious from the title.

@HarelM

HarelM commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Version 6 was just released so if you would like to push this forward let me know. Currently it's in draft, when it will be moved to ready for review I'll do another sweep. Thanks.

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