Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use custom indexes to speed up ST_PointOnSurface queries #4292

Merged
merged 1 commit into from
Jan 28, 2021

Conversation

pnorman
Copy link
Collaborator

@pnorman pnorman commented Jan 23, 2021

Using an index for ST_PointOnSurface queries speeds up the queries
substantially by avoiding work for when the geometries are in the
bounding box but the labeling point isn't.

The impact is substantial for large geometries like administrative
areas that have a lot of matches.

For these indexes to be reliably used, the plain way && !BBOX!
condition needs to be removed from the WHERE clause, making
the index required instead of optional.

I tested this by building the new indexes on my database and browsing in Kosmtik while watching query times. Any queries which were unable to use the indexes would have stood out because I have a full planet database.

Using an index for ST_PointOnSurface queries speeds up the queries
substantially by avoiding work for when the geometries are in the
bounding box but the labeling point isn't.

The impact is substantial for large geometries like administrative
areas that have a lot of matches.

For these indexes to be reliably used, the plain way && !BBOX!
condition needs to be removed from the WHERE clause, making
the index required instead of optional.
@imagico
Copy link
Collaborator

imagico commented Jan 23, 2021

For these indexes to be reliably used, the plain way && !BBOX!
condition needs to be removed from the WHERE clause, making
the index required instead of optional.

Would it in principle be possible to make the condition depend on the presence of the index? I am not saying we necessarily should, just exploring the possibility.

We have other large and complex geometries in amenity-points like in particular place=island - but there they are mixed with tons of other very small polygons where this is not helpful.

The more fundamental solution would be to precalculate St_PointOnSurface() in a similar fashion as way_area is precalculated. That would of course make it more difficult to move to a better label location selection in the future and it would make it harder to do any label styling sensitive label placement (which was however already essentially foregone with the move of label placement from mapnik to SQL).

@pnorman
Copy link
Collaborator Author

pnorman commented Jan 24, 2021

Would it in principle be possible to make the condition depend on the presence of the index? I am not saying we necessarily should, just exploring the possibility.

It's not.

We have other large and complex geometries in amenity-points like in particular place=island - but there they are mixed with tons of other very small polygons where this is not helpful.

I don't think these are a problem, but it's hard to tell with the layer being such a big one.

The more fundamental solution would be to precalculate St_PointOnSurface() in a similar fashion as way_area is precalculated.

I don't believe osm2pgsql can do this yet.

That would of course make it more difficult to move to a better label location selection in the future

Pre-calculating would make it easier to move to a better label location selection, not harder.

@pnorman pnorman requested a review from imagico January 24, 2021 00:18
pnorman added a commit to pnorman/openstreetmap-cartographic that referenced this pull request Jan 24, 2021
This relies on the indexes in gravitystorm/openstreetmap-carto#4292
to substantially speed up z11+ generation.
@jeisenbe jeisenbe mentioned this pull request Jan 24, 2021
@pnorman
Copy link
Collaborator Author

pnorman commented Jan 27, 2021

I'm looking into if a non-partial index would help, since there are some queries that weren't changed because they don't require a name.

@pnorman
Copy link
Collaborator Author

pnorman commented Jan 28, 2021

I'm looking into if a non-partial index would help, since there are some queries that weren't changed because they don't require a name.

It doesn't seem worth it. The index takes 12h to build, is as large as the normal index on (way), and the stuff that uses st_pointonsurface without a name restriction tends to be of small objects.

@pnorman pnorman merged commit 42a6baf into gravitystorm:master Jan 28, 2021
@pnorman pnorman deleted the pointonsurface_index branch January 30, 2021 18:14
@imagico imagico mentioned this pull request Mar 13, 2021
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