Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Ask HTTP implementations not to do content decoding #3886

Open
jfirebaugh opened this issue Feb 10, 2016 · 12 comments
Open

Ask HTTP implementations not to do content decoding #3886

jfirebaugh opened this issue Feb 10, 2016 · 12 comments
Labels
Core The cross-platform C++ core, aka mbgl offline performance Speed, stability, CPU usage, memory usage, or power usage

Comments

@jfirebaugh
Copy link
Contributor

We want to store compressed resources in the offline database. Our implementation ends up decompressing and then recompressing. Instead, we should ask NSURLSession, curl, and OkHttp to give us the gzipped body back, put the compressed body directly into the database, and only then decompress it for downstream consumers.

Specifically, we need to tell our HTTP libraries to as for and pass through gzip content encoding, since that's the only format we know how to decompress.

If we get back an uncompressed body (i.e. the server doesn't support content encoding), we should still try to compress it before storing it in the database.

This will improve performance of offline downloading significantly. Right now almost all of the CPU used during offline downloads goes toward compression and decompression, neither of which is necessary.

@jfirebaugh jfirebaugh added the performance Speed, stability, CPU usage, memory usage, or power usage label Feb 10, 2016
@1ec5 1ec5 added the offline label Feb 10, 2016
@jfirebaugh jfirebaugh self-assigned this Feb 11, 2016
@jfirebaugh
Copy link
Contributor Author

  • OkHttp: If you add your own Accept-Encoding: gzip header to the request, you take responsibility for decoding.
  • Curl: "You can also opt to just include the Accept-Encoding: header in your request with CURLOPT_HTTPHEADER but then there will be no automatic decompressing when receiving data."
  • ❌ NSURLSession: Apparently not possible to take responsibility for decoding. 😭

@kkaefer
Copy link
Contributor

kkaefer commented Feb 11, 2016

This is indeed the right approach (but it still means we'll have to decompress in the SQLite source, and ideally move that to another thread).

Regarding NSURLSession, I tried http://stackoverflow.com/a/16705063/331379 and indeed got the compressed response, but of course it's a private API so we can't use it :(

@tmpsantos
Copy link
Contributor

Agreed, it is the right approach. ✅ for Qt, it is done the same way as OkHttp.

I also second @kkaefer that we should move decompress to a separate thread.

@jfirebaugh
Copy link
Contributor Author

@1ec5
Copy link
Contributor

1ec5 commented Mar 16, 2016

It’s pretty low-level, but does NSInputStream pass through gzip-encoded data?

@incanus
Copy link
Contributor

incanus commented Jul 19, 2016

I wonder if modifying NSURLSession.sessionConfiguration.requestCachePolicy would in effect sidestep this, if specifically set to not cache automatically (presuming that automatic caching has a hand in decoding for its own purposes).

@kkaefer kkaefer added the Core The cross-platform C++ core, aka mbgl label May 30, 2017
@halset
Copy link
Contributor

halset commented Jun 9, 2017

If this is ever fixed on iOS, how would this change the Mapbox APIs? Would you add a compressed-flag to mbgl::Response?

The reason I ask is that I insert lots of tiles from a SQLite file that I download from my server. This db has the same schema as the Mapbox GL cache with some of the tiles/resources compressed. I currently use a lot of cpu to decompress and recompress during import when I really should just keep the data and the compressed flag from the external SQLite file.

@jfirebaugh
Copy link
Contributor Author

There wouldn't be any changes to public APIs or the (private) database schema.

@fabian-guerra
Copy link
Contributor

@1ec5 and I asked Apple engineers about accessing the underlaying compressed file (at the networking lab). They confirmed what we already know; is not possible. They asked us why would we want that and we explained the use case to what they suggested that we should file radars in order to get it done. I don't think this is something that is going to be solved soon. What we could do is for each of us that has an apple dev account file a radar asking for this.

@1ec5
Copy link
Contributor

1ec5 commented Jun 12, 2017

What we could do is for each of us that has an apple dev account file a radar asking for this.

Also note in the radar that it's a dupe of the radar in #3886 (comment).

@stale
Copy link

stale bot commented Nov 30, 2018

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Nov 30, 2018
@friedbunny friedbunny reopened this Nov 30, 2018
@stale stale bot removed the archived Archived because of inactivity label Nov 30, 2018
@stale stale bot added the archived Archived because of inactivity label May 29, 2019
@stale
Copy link

stale bot commented May 29, 2019

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed May 29, 2019
@friedbunny friedbunny reopened this May 29, 2019
@stale stale bot removed the archived Archived because of inactivity label May 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl offline performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

8 participants