Revert "Render unpaved roads different"

[sommerluk]

Reverts #3357

[imagico]

Could you explain the reasoning behind this and your plan to proceed with this change?

[sommerluk]

Well, in the detailed testing that @rrzefox did with this change, it has a performance impact on higher zoom level.

I have played around a little bit with a different SQL query reducing the amount of actually rendered globalboundingboxes and the SQL query gets more complicated. And I run into an SQL syntax problem that I do not understand (has apparently something to do with bbox)

As @tomhughes for the moment did not made a statement in #3386 about how much impact that would have on openstreetmap.org servers, and as the next release is planed for 21 september 2018 and as I do not have much time currently to proceed with investigating the SQL query, I preferred to stay on the same side and avoid performance problems like the last time. And avoid to feel in hurry.

I’ve detected also a minor bug in this pull request. (Unpaved railway=platform that are at the same time bridges do not render in the correct colour if layer=* is different from 0).

Plans to proceed? My idea would be:

• Fix the railway=platform bug
• Adding cache feature also for turning circles and aero-ways. Maybe this is enough to get better performance.
• Asking help from @rrzefox for testing this (as my local setup has proven not to be representative for performance testing)
• If the performance result are acceptable for OSMF servers, merge it. If not, than taking a closer look to the SQL query.

But I’m open for other suggestions…

[sommerluk]

The easy fix for the railway=platform bug is rendering unpaved platforms (both railway=platform and highway=platform) like paved ones.

[imagico]

My recommendation in #3357 (comment) was to see if you can reproduce the performance difference in a test environment and if yes to try pinpointing what exactly makes the difference.

[sommerluk]

My recommendation in #3357 (comment) was to see if you can reproduce the performance difference in a test environment

I’ve tried to reproduce this locally and was not able to. Measuring the time for tile rendering using the script from @giggls gives me the same performance with or without this PR.

As @talaj said:

The dst-over is not executed if the polygon is not rendered. It would most likely help to render it only if there are corresponding roads rendered.

This was the starting point for looking at SQL query tuning…

[imagico]

I’ve tried to reproduce this locally and was not able to.

If you can't reproduce it in a test environment i would avoid drawing any broader conclusions or making decisions based on a single observation without information what part of the process is slower exactly.

If you can't reproduce it in a test environment you can essentially also rule out any universal performance issues in mapnik with the code used (including ones due to unnecessary geometries being included in rendering) because what mapnik receives to render will be the same no matter if you are on a global database or just with a small test data set.

[sommerluk]

Okay. At https://github.com/sommerluk/openstreetmap-carto/tree/unpaved24 is what I’m currently working on.

The problem with platform rendering was that the globalboundingbox for bridges is the result of a SELECT DISTINCT only in the highway part, but for platform we have both highway=platform and railway=platform. An SQL that catches this case has to span over both, highway and railway part. As this could be complicate, for the moment I do no special unpaved rendering for platforms.

For performance, as the bridges layer with will potentially be rendered various times (at difference to road-fill or roads-low-zoom), and as this layer has yet a SELECT DISTINCT, I’ve done now a better filtering. For performance measuring I’ve tried to eliminate factors like other programs that in in parallel. With the new code, I get more or less the same performance (base on single-tile-measurement) that without unpaved rendering (while the previous approach was 10% slower).