MBVectorTileDecoder from CompiledStyleSet is slow (xml / json)

[farfromrefug]

I just realized that generating a MBVectorTileDecoder from CompiledStyleSet is really slow. I tested with that theme https://github.com/farfromrefug/new_carto_theme as a zip and it took almost 1.5s. Now i also transformed that json style to xml and it dropped to 500ms

TBH i think it should be faster in both cases. Seems to me that now my theme is pretty small. I remember you telling me that json style parsing was now really fast. It seems pretty slow right now.

Would be really good to have this really faster

[mtehver]

@farfromrefug That is really strange. How did you measure it?

I did a quick measurement with SDK 4.4.2 and the latest style snapshot from your GitHub repo:

                long x0 = System.nanoTime();

                BinaryData styleAsset = AssetUtils.loadAsset("new_carto_theme.zip");
                AssetPackage styleAssetPackage = new ZippedAssetPackage(styleAsset);
                CompiledStyleSet styleSet = new CompiledStyleSet(styleAssetPackage);
                MBVectorTileDecoder tileDecoder = new MBVectorTileDecoder(styleSet);
                TileDataSource dataSource = new CartoOnlineTileDataSource("carto.streets");
                VectorTileLayer currentLayer = new VectorTileLayer(dataSource, tileDecoder);

                long x1 = System.nanoTime();
                Log.debug("Layer construction time: " + (x1 - x0) / 1000000);

I got 320ms on my Pixel 4a, which uses midrange SoC/CPU. This is not exactly great, but usable. I also have some optimisations that should reduce loading time by additional 20-30% for complex styles.

The zipped file I used is CartoCSS style, not compiled style.

[farfromrefug]

@mtehver I do exactly the same thing (except i use 'osm' style which has image patterns). I tried right now with streets (CartoCSS style) and it took 1.3s so could be the same time exactly. BTW i only count the time of the MBVectorTileDecoder creation from CompiledStyleSet which is where the parsing is done. One thing is that i test on a Samsung a3 2017 which is for sure slower

[mtehver]

@farfromrefug Ok, I guess ARM A53 cores are really that much slower than A76 cores then. The latest develop branch snapshot should improve performance by 20-30%, but I will do additional profiling.

[farfromrefug]

@mtehver tested your latest changes and am now at 950ms! this is already a big win :D Thanks a lot

[mtehver]

I did some further profiling and added few optimizations and the latest develop snapshot should cut another 10% of the loading time. In addition I created a PR with some style changes that make parsing/compiling of the style faster (https://github.com/farfromrefug/new_carto_theme/pull/4).

With the latest SDK changes and optimized style I got the loading time down to 125ms on my Pixel 4a. That seems to be good enough.

[farfromrefug]

@mtehver awesome will test that! Could you explain a few of your optimisations on the style? :

• using [zoom=15] instead of [zoom>=15] when targeting zoom >= 15 && zoom <16 is an optimisation? not sure i see why
• #park i see why, it was adding rules for many cases not related to park. Duplicates but makes sense
• you moved ['mapnik::geometry_type'=3] for #transportation can you explain the optimisation? (so that i dont do it again)

[mtehver]

@farfromrefug Regarding [zoom=X], instead of [zoom>=X]. This seemed to make measurable difference, but I did not have time to investigate this further. Also, I think in case of POI layer, it makes things easier to read.

Regarding 'transportation' layer, by explicitly checking for correct geometry_type, I fixed a few potential cases where line symbolizers were used for polygon geometry. The implemented changes reduced the number of rules in generated XML file.

[farfromrefug]

@mtehver so i did ran some tests with your latest changes:

• loading the new theme with your PR took 505ms
• loading the same theme compiled in xml took 313ms
• thanks to your PR on the theme xml style files dropped from 1.4Mb to 800kb ....

So clearly we can say that on the side of the SDK loading of style we are good ! Now loading cartoCSS is still quite a lot slower than loading XML (no big deal here). I will end up saying that still improvements can be made in the XML generation which would make files smaller but also loading faster(less rules):

• i see a lot of duplicate rules which could be joined based on MinScaleDenominator and MaxScaleDenominator. Like joining rule1 and rule2 if rule1.MaxScaleDenominator === rule2.MinScaleDenominator.
• that simple change, looking at my xmls, could reduce the file size by 20 to 40%

[farfromrefug]

@mtehver i implemented the cleanup of the xml files and it makes quite a difference. As explained the idea is to join together rules which correspond to consecutive zoom levels. Changes:

• file size:
  • before: 870kb
  • after: 339kb
• loading time:
  • before: 380ms
  • after: 250ms

As a reference loading the corresponding JSON style takes 520ms.

It seems like quite a big improvement to me. I think this should be done in css2xml. For reference here is the code i use to group/filter https://github.com/farfromrefug/alpimaps/blob/master/simply_xml.js#L27

[mtehver]

@farfromrefug The script seems to do unsafe reordering of the rules, unfortunately. But you are right, there are cases when the current SDK fails to merge identical rules even when it is safe to do. I have improved this a bit in develop branch.

In addition I added 'symbolizer cache' to compiled style parsing, this should improve loading times of compiled styles considerably. On my Windows desktop machine (R9 5900x) loading of CSS style takes roughly 55ms, loading of compiled style now takes 20ms. I used 'outdoors' style for testing.

[farfromrefug]

@mtehver awesome! can you elaborate a bit on the unsafe reordering? Seems to be working great here.

[mtehver]

@farfromrefug The following reordering is unsafe (wrong), for example:

(zoomrange=1..1, filter="[class]=1 && [subclass]=2", symbolizers=...)
(zoomrange=2..2, filter="[class]=1 && [ramp]=1", symbolizers=...)
(zoomrange=2..2, filter="[class]=1 && [subclass]=2", symbolizers=...)

to

(zoomrange=1..2, filter="[class]=1 && [subclass]=2", symbolizers=...)
(zoomrange=2..2, filter="[class]=1 && [ramp]=1", symbolizers=...)

(here the 3rd rule was merged into the 1st)

[farfromrefug]

@mtehver thanks for the explanation. So i guess it means the order is important. Thanks for all your work / time!