Cross tile index performance #16127

pozdnyakov · 2020-01-20T15:12:34Z

Before this change cross tile symbol indexing was a performance bottle neck, for the overscaled tiles in particular. This pull request optimizes the cross tile symbol indexing code path:

find matches only among the buckets that have the same leader Id.
for overscaled tiles, index only the symbols inside the viewport
multiple minor optimizations and code simplifications

Fixes https://github.com/mapbox/mapbox-gl-native-team/issues/133

astojilj

I did a quick performance check. The patch improves the performance (1024x768 mbgl-glfw release build on 3.2 GHz 6-Core Intel Core i7 macMini):
a) when there's no pitch, dropped frames take up to 50ms. This was several seconds before.
b) with 60 degree pitch, dropped frames usually take up to 9000ms.

@pozdnyakov, agree with conclusion that this, due to the severity of the issue and even though it is addressing the symptoms and not the root cause (overscaling tiles and buckets we loose the granularity and performance of tiled approach) is worth having as an intermediate fix.

src/mbgl/text/cross_tile_symbol_index.cpp

Only buckets with the same leader id participate in `TileLayerIndex::findMatches()` in order to improve its performace. `TileLayerIndex` constness is fixed.

So that it can retain ownership of the given parameters.

…iles For overscaled tiles the viewport might be showing only a small part of the tile, so we filter out the off-screen symbols to improve the performance.

alexshalamov · 2020-01-22T11:58:10Z

src/mbgl/text/cross_tile_symbol_index.cpp

+            if (isInVewport(tileMatrix, symbolInstance.anchor.point)) {
+                symbolInstance.crossTileID = 0u;
+            } else {
+                symbolInstance.crossTileID = SymbolInstance::invalidCrossTileID();


Would this delay symbol placement by 300ms for out-of-viewport symbols when we pan overscaled tile?

it won't, as we reassign those ids at every frame

astojilj

👍

springmeyer · 2020-01-22T15:40:00Z

src/mbgl/text/cross_tile_symbol_index.cpp

-    const auto& thisZoomIndexes = indexes[tileID.overscaledZ];
+namespace {
+
+bool isInVewport(const mat4& posMatrix, const Point<float>& point) {


@pozdnyakov should this be spelled isInViewport?

yeah 😬 thanks for the catch, will fix it in a following patch..

pozdnyakov added performance Speed, stability, CPU usage, memory usage, or power usage Core The cross-platform C++ core, aka mbgl labels Jan 20, 2020

pozdnyakov requested review from astojilj, alexshalamov and ansis January 20, 2020 15:12

pozdnyakov self-assigned this Jan 20, 2020

astojilj reviewed Jan 21, 2020

View reviewed changes

src/mbgl/text/cross_tile_symbol_index.cpp Outdated Show resolved Hide resolved

astojilj reviewed Jan 21, 2020

View reviewed changes

src/mbgl/text/cross_tile_symbol_index.cpp Outdated Show resolved Hide resolved

pozdnyakov force-pushed the mikhail_cross_tile_index_performance branch 2 times, most recently from 78fcd82 to a7cacfc Compare January 21, 2020 14:52

pozdnyakov marked this pull request as ready for review January 21, 2020 14:52

pozdnyakov requested a review from 1ec5 as a code owner January 21, 2020 14:52

pozdnyakov added 11 commits January 22, 2020 10:48

[core] Consider symbol bucket leader id in cross-tile symbol indexing

302b13b

Only buckets with the same leader id participate in `TileLayerIndex::findMatches()` in order to improve its performace. `TileLayerIndex` constness is fixed.

[core] Pass placement commit parameters in constructor

c53ffce

[core] Pass std::shared_ptr<UpdateParameters> to the render orchestrator

54d4da2

So that it can retain ownership of the given parameters.

[core] Pass UpdateParameters to Placement constructor

43011ad

[core] Cache showCollisionBoxes in Placement

d0fa782

[core] Avoid repeated calculations in symbol placement

6fe66c0

[core] Simplify CrossTileSymbolIndex::addLayer()

13c7770

[core] Simplify Placement::addLayer()

8da64eb

[core] Do not index and place the off-screen symbols for overscaled t…

ffa1cb3

…iles For overscaled tiles the viewport might be showing only a small part of the tile, so we filter out the off-screen symbols to improve the performance.

Add CrossTileSymbolLayerIndex.offscreenSymbols test

b905f26

Add change log entry

688f545

pozdnyakov force-pushed the mikhail_cross_tile_index_performance branch from a7cacfc to 688f545 Compare January 22, 2020 11:38

alexshalamov reviewed Jan 22, 2020

View reviewed changes

astojilj approved these changes Jan 22, 2020

View reviewed changes

alexshalamov approved these changes Jan 22, 2020

View reviewed changes

pozdnyakov merged commit 04edb1d into master Jan 22, 2020

pozdnyakov deleted the mikhail_cross_tile_index_performance branch January 22, 2020 12:30

springmeyer reviewed Jan 22, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross tile index performance #16127

Cross tile index performance #16127

pozdnyakov commented Jan 20, 2020 •

edited

Loading

astojilj left a comment

alexshalamov Jan 22, 2020

pozdnyakov Jan 22, 2020

astojilj left a comment

springmeyer Jan 22, 2020

pozdnyakov Jan 22, 2020

Cross tile index performance #16127

Cross tile index performance #16127

Conversation

pozdnyakov commented Jan 20, 2020 • edited Loading

astojilj left a comment

Choose a reason for hiding this comment

alexshalamov Jan 22, 2020

Choose a reason for hiding this comment

pozdnyakov Jan 22, 2020

Choose a reason for hiding this comment

astojilj left a comment

Choose a reason for hiding this comment

springmeyer Jan 22, 2020

Choose a reason for hiding this comment

pozdnyakov Jan 22, 2020

Choose a reason for hiding this comment

pozdnyakov commented Jan 20, 2020 •

edited

Loading