bleakgrey / tootle

GTK-based Mastodon client for Linux
GNU General Public License v3.0
400 stars 61 forks source link

Cache PixBufs #17

Closed martensitingale closed 5 years ago

martensitingale commented 6 years ago

When a new toot loads or the user sends a toot, initially the "no avatar" image is shown for the user's avatar, and only later is the user's proper avatar shown. This is the case even when caching is enabled in settings and when sending a toot from the user's own account (which is the easiest way to reproduce this bug).

My guess is that relying on Soup to cache avatars still requires sending a request to validate the HTTP cache and waiting for its response to return the locally cached content. In cases like these where tootle has already downloaded the users' avatars, it would be simpler and more efficient to avoid involving Soup at all; this could also enable sharing pixbufs and decrease total memory usage compared to creating a new avatar pixbuf for every status of a given user.

bleakgrey commented 6 years ago

Well then Soup's cache is literally pointless if it's downloading an image just to see whether it has changed. However if it's just checking the headers or hashes or whatever then I'm okay with that.

The reason why I used Soup.Cache in the first place is that I want to keep my code as simple as possible. I can't maintain enormous codebase and reinventing the wheel feels a waste of time which I don't have much anyway.

martensitingale commented 6 years ago

In my testing it looks like tootle (with Soup caching enabled) doesn't obtain avatars from the cache at all (see log below).

Keeping a data structure with knowledge that the app should have (a mapping from users to their avatar pixbuf) doesn't seem like it reinvents a wheel or imposes significant complexity.

I don't mind implementing this change if you don't have time. Would you accept a pull request when I do?

HTTP log of a single toot of mine being viewed twice:

> GET /api/v1/statuses/100064157047337545/context HTTP/1.1
> Soup-Debug-Timestamp: 1526859031
> Soup-Debug: SoupSession 1 (0x555576db5100), SoupMessage 129 (0x55557746eb80), SoupSocket 1 (0x555577411cf0)
> Host: anti.energy
> Authorization: Bearer 
> Accept-Encoding: gzip, deflate
> Connection: Keep-Alive

< HTTP/1.1 200 OK
< Soup-Debug-Timestamp: 1526859031
< Soup-Debug: SoupMessage 129 (0x55557746eb80)
< Date: Sun, 20 May 2018 23:30:29 GMT
< Content-Type: application/json; charset=utf-8
< Transfer-Encoding: chunked
< Connection: keep-alive
< Server: Mastodon
< X-Frame-Options: DENY
< X-Content-Type-Options: nosniff
< X-XSS-Protection: 1; mode=block
< X-RateLimit-Limit: 300
< X-RateLimit-Remaining: 293
< X-RateLimit-Reset: 2018-05-20T23:35:00.234833Z
< Vary: Accept-Encoding, Origin
< Content-Encoding: gzip
< ETag: W/"3cafcca51abeeac42f2757dd3fc820aa"
< Cache-Control: max-age=0, private, must-revalidate
< X-Request-Id: 558ca0ba-455b-43a1-97b1-8a9c10de617a
< X-Runtime: 0.034693
< Strict-Transport-Security: max-age=31536000; includeSubDomains

> GET /system/accounts/avatars/000/000/159/original/4a5fda568790b2bf.png HTTP/1.1
> Soup-Debug-Timestamp: 1526859031
> Soup-Debug: SoupSession 1 (0x555576db5100), SoupMessage 130 (0x555577e55760), SoupSocket 1 (0x555577411cf0)
> Host: anti.energy
> Authorization: Bearer 
> Accept-Encoding: gzip, deflate
> Connection: Keep-Alive

< HTTP/1.1 200 OK
< Soup-Debug-Timestamp: 1526859031
< Soup-Debug: SoupMessage 130 (0x555577e55760)
< Server: nginx/1.6.2
< Date: Sun, 20 May 2018 23:30:29 GMT
< Content-Type: image/png
< Content-Length: 35191
< Last-Modified: Thu, 13 Apr 2017 02:30:40 GMT
< Connection: keep-alive
< ETag: "58eee2d0-8977"
< Strict-Transport-Security: max-age=31536000; includeSubDomains
< Accept-Ranges: bytes

> GET /api/v1/statuses/100064157047337545/context HTTP/1.1
> Soup-Debug-Timestamp: 1526859036
> Soup-Debug: SoupSession 1 (0x555576db5100), SoupMessage 131 (0x555578157d80), SoupSocket 1 (0x555577411cf0)
> Host: anti.energy
> Authorization: Bearer 
> Accept-Encoding: gzip, deflate
> Connection: Keep-Alive

< HTTP/1.1 200 OK
< Soup-Debug-Timestamp: 1526859036
< Soup-Debug: SoupMessage 131 (0x555578157d80)
< Date: Sun, 20 May 2018 23:30:34 GMT
< Content-Type: application/json; charset=utf-8
< Transfer-Encoding: chunked
< Connection: keep-alive
< Server: Mastodon
< X-Frame-Options: DENY
< X-Content-Type-Options: nosniff
< X-XSS-Protection: 1; mode=block
< X-RateLimit-Limit: 300
< X-RateLimit-Remaining: 292
< X-RateLimit-Reset: 2018-05-20T23:35:00.462574Z
< Vary: Accept-Encoding, Origin
< Content-Encoding: gzip
< ETag: W/"15ee8096c9295f9bd6cf4456e55f92ce"
< Cache-Control: max-age=0, private, must-revalidate
< X-Request-Id: ecf53f9e-30ba-4c7a-87db-d35d8ed0b243
< X-Runtime: 0.026682
< Strict-Transport-Security: max-age=31536000; includeSubDomains

> GET /system/accounts/avatars/000/000/159/original/4a5fda568790b2bf.png HTTP/1.1
> Soup-Debug-Timestamp: 1526859036
> Soup-Debug: SoupSession 1 (0x555576db5100), SoupMessage 132 (0x55557750c0c0), SoupSocket 1 (0x555577411cf0)
> Host: anti.energy
> Authorization: Bearer 
> Accept-Encoding: gzip, deflate
> Connection: Keep-Alive

< HTTP/1.1 200 OK
< Soup-Debug-Timestamp: 1526859036
< Soup-Debug: SoupMessage 132 (0x55557750c0c0)
< Server: nginx/1.6.2
< Date: Sun, 20 May 2018 23:30:34 GMT
< Content-Type: image/png
< Content-Length: 35191
< Last-Modified: Thu, 13 Apr 2017 02:30:40 GMT
< Connection: keep-alive
< ETag: "58eee2d0-8977"
< Strict-Transport-Security: max-age=31536000; includeSubDomains
< Accept-Ranges: bytes
bleakgrey commented 6 years ago

Woah, so caching does nothing except using disk space... This isn't helping at all lol. Will check soup docs tomorrow, maybe I'm just using it wrong.

Sure, I happily accept pull requests. Please make it a separate Manager class, see account and network classes for references.

I think Tootle should cache all pixbufs since we also have attachments and, hopefully, instance emojis soon.

bleakgrey commented 6 years ago

Also posting http debug logs publicly isn't a great idea, you just exposed your auth token. Please invalidate it in your account settings.

martensitingale commented 6 years ago

I've already invalidated the token. :)

martensitingale commented 6 years ago

I have a working implementation of caching now and it speeds things up nicely. I still need to implement a size limit (I'll use the existing preference) and eviction policy (I was thinking fewest-references-first so frequest posters' avatars are more likely to stay cached). When I find time I'll split it into commits and push to my branch and get a PR going.

bleakgrey commented 6 years ago

Sounds awesome! Thanks for helping.

No idea why Soup's cache not working though. It seems to be always rewriting files regardless of their state. I had an idea that it's something in the request headers that makes caching policy crazy, but it seems like it's not the problem.

Looks like I'll have to remove Soup caching for now since it's practically not helpful :/