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

Streaming responses #37

Open
lbam opened this issue Mar 10, 2023 · 4 comments
Open

Streaming responses #37

lbam opened this issue Mar 10, 2023 · 4 comments

Comments

@lbam
Copy link

lbam commented Mar 10, 2023

In this line...

return row.tobytes() if row else b""

the entire MVT response from PostGIS is converted to a bytes object. That means it has to be in memory twice, once as a memoryview and once as a bytes. I'm wondering if this is necessary, since Django performs the same conversion. If the conversion were not there, a caller could intercept the memoryview and make a StreamingHttpResponse out of it, so that it only needs to be in memory once.

@StefanBrand
Copy link
Contributor

Good point, it looks as if the tobytes call could possibly be removed. I quickly ran the tests with this change and they run fine. Are you interested in contributing a PR?

@lbam
Copy link
Author

lbam commented Apr 6, 2023

I could discuss a PR with my employer, but I'd first need to have #36 resolved.

@lbam
Copy link
Author

lbam commented Apr 6, 2023

I've also done some more research and found that this is not the bottleneck, memory-wise. The tile is sent over the wire by PostgreSQL as ASCII hex. psycopg reads the entire tile into memory in that format, then decodes it into a separate buffer, so at that point the memory use is 3x the size of the tile. Fixing this issue would therefore not reduce the peak memory use. For that, the ST_AsMVT query would somehow have to chunk the resulting bytea in PostgreSQL so this library can receive it piecemeal and sent the chunks to the client in a streaming response.

@submarcos
Copy link
Owner

I could discuss a PR with my employer, but I'd first need to have #36 resolved.

This point is resolved. I'm working on a 1.0 refactor and release. If you contribute a PR I can implement it in 0.2.1 and 1.0 versions

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

No branches or pull requests

3 participants