Optimize CrossTileSymbolIndex by leveraging promoteId to automatically pair known feature symbols across tiles - #7665
Conversation
|
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... |
|
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. |
|
Thanks! Don't worry about the flakey render tests... |
| numVerticalIconVertices, | ||
| useRuntimeCollisionCircles, | ||
| 0, | ||
| crossTileID, |
There was a problem hiding this comment.
Does this has the possibility to overflow? I've seen some nasty bugs related to integer overflow when using this emplaceBack method.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
Does this already use the promoteId?
|
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
|
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... |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Here's the data flow for how crossTileID gets pre-assigned in this PR. The problemThe 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 optimizationIf 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 flowWhy this is correctThe 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 / lifecycleThe _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
|
|
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: 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. |
|
Thanks for the update, this is not covered by existing render tests, right? Maybe a different PR to add this test would make sense. |
|
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. |
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:
Outputs:
Compare to original benchmark here: #7664 this is a 2,500x speedup.
Public API changes:
Launch Checklist
CHANGELOG.mdunder the## mainsection.