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

feat: network cleanup #616

Merged
merged 17 commits into from
Nov 14, 2023
Merged

feat: network cleanup #616

merged 17 commits into from
Nov 14, 2023

Conversation

GeopJr
Copy link
Owner

@GeopJr GeopJr commented Oct 23, 2023

libsoup can handle caching. Caching entities was more of a headache than helpful (any changes wouldn't reflect or would reflect globally). Caching images in memory for x amount of time is wasteful.

Following tootle discussions it seems that it was implemented because libsoup's wasn't reliable. Tuba migrated to libsoup3 however which might have fixed it.

TODO:

  • Check if cache actually works reliably
  • Cleanup
  • Remove abstract cache altogether
  • Cache blurhashes?
  • Check reactive entities (like pinning statuses)
  • clear cache on close?

fix: #136

@GeopJr
Copy link
Owner Author

GeopJr commented Oct 24, 2023

Libsoup does in fact hit the cache, or at least the logger claims so.

I simplified the API by getting rid of "is_loaded" and instead kept track of it locally. It also takes care of blurhash too. If a blurhash is provided, it will idly decode it and return it if the image hasn't loaded yet.

@GeopJr
Copy link
Owner Author

GeopJr commented Oct 24, 2023

Memory leak in libsoup?
is this expected?
what's going on?

image

@GeopJr GeopJr changed the title feat: stop manually caching feat: network cleanup Oct 30, 2023
@GeopJr
Copy link
Owner Author

GeopJr commented Oct 30, 2023

I think that last commit fixed #136 🤷 needs more testing

while this shouldn't(?) be happening, it occurs sometimes
it caches anything that's cacheable, not only media
memory leaks out of your control? more likely than you think. FlowBox#bind_model caused a memory leak, use helper functions instead
@GeopJr
Copy link
Owner Author

GeopJr commented Nov 8, 2023

... memory leak with FlowBox#bind_model 🤷 used helper functions instead

as the preview one would be removed by the time it's actually done
@GeopJr GeopJr marked this pull request as ready for review November 14, 2023 07:39
@GeopJr
Copy link
Owner Author

GeopJr commented Nov 14, 2023

I'll go ahead and merge it. There's no way to find out what might be wrong unless it starts getting used outside of this PR

@GeopJr GeopJr merged commit 43b2580 into main Nov 14, 2023
5 checks passed
@GeopJr GeopJr deleted the feat/stop-manually-caching branch November 14, 2023 09:10
@LukaszH77
Copy link
Contributor

I used it for some time and it worked fine.

@GeopJr
Copy link
Owner Author

GeopJr commented Nov 14, 2023

It has some problems here and there, especially:

  • memory leaks became more prominent
  • sometimes it just gives up and doesn't recover - I can reproduce it sometimes on the custom emoji picker, where it will stop midway and then no other images will load at all

Overall this change is actually positive despite its shortcomings. From a user standpoint, it saves on bandwidth by caching images and metadata locally (.cache/Tuba/soup/...) and doesn't clog the main connections by using a separate session for images. From a dev standpoint, it finally gets rid of the whole entity cache headache and should allow me to move forward with other plans.

Thanks for testing it as always 💛🎺

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.

[Bug]: Stream timeline doesn't refresh after the sleep of the computer
2 participants