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 “cache-feature: true” for better performance with attachments #3838

Merged
merged 1 commit into from
Aug 21, 2019

Conversation

sommerluk
Copy link
Collaborator

Fixes #3756

Changes proposed in this pull request:

  • Add cache-feature: true for all layers that use attachments to get better performance. We have quite a lot of them…

Test rendering with links to the example places: Should trigger no rendering change.

This PR would need performance testing, but I am not able to do that.

Another question: What about adding cache-feature: true to all layers? If this would be performance-neutral, it might be worth to do so, to prevent adding new attachments while forgetting about adding cache-feature: true?

@pnorman
Copy link
Collaborator

pnorman commented Aug 10, 2019

Another question: What about adding cache-feature: true to all layers? If this would be performance-neutral, it might be worth to do so, to prevent adding new attachments while forgetting about adding cache-feature: true?

I would expect it to boost RAM usage if done to all layers, but I have no idea how much.

I'm loading up a DB to run low zoom tests on, but if they're inconclusive, then I'll need to do high-zoom testing too. My current test setup use mapproxy, so because it creates the map object each render that flaw might overwhelm any performance differences, and I might need to go to renderd

@sommerluk
Copy link
Collaborator Author

For comparison: https://github.com/sommerluk/openstreetmap-carto/tree/cache02 provides code that uses cache-features for all layers.

@pnorman pnorman self-requested a review August 12, 2019 02:39
Copy link
Collaborator

@pnorman pnorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent of this branch took 158m31.916s while the branch took 148m17.483s. That's a 7% gain for z0-z10 pre-rendering, and we have relatively few attachments there. I expect higher gains at higher zooms, but don't feel the need to quantify exact gains.

I'm against adding cache-features to all layers regardless of attachment status without quantifying memory changes.

@pnorman pnorman merged commit a039bd0 into gravitystorm:master Aug 21, 2019
@sommerluk
Copy link
Collaborator Author

Thanks for testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set “cache-features: true” in project.mml?
3 participants