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

fix most freetype/harfbuzz related memory leaks #1618

Conversation

benoit-pierre
Copy link
Contributor

@benoit-pierre benoit-pierre commented Jun 11, 2023

There's still one small leak remaining due to using FT_Done_Library instead of FT_Done_FreeType to handle the Freetype's library lifecycle: its memory object is not freed on exit.

This is also the precursor to some memory optimizations.


This change is Reviewable

There's still one small leak remaining due to using `FT_Done_Library`
instead of `FT_Done_FreeType` to handle the Freetype's library
lifecycle: its memory object is not freed on exit.
@poire-z
Copy link
Contributor

poire-z commented Jun 11, 2023

fix most freetype/harfbuzz related memory leaks

Btw, how much of that amount to the well known KOReader memory leaks (cf koreader/koreader#5998 (comment)) ?
Does your mesonic monolibtic build still get its memory usage increasing as obviously as ours ?

Copy link
Contributor

@poire-z poire-z left a comment

Choose a reason for hiding this comment

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

A bit over my head, so it must be ok :)

@benoit-pierre
Copy link
Contributor Author

Good question, memory does increase as more features get used, menus opened, but I'm not sure it's leaking. Honestly, I don't have to restart on my Kindle anymore thanks to:

  • those changes coupled with the aforementioned fonts related memory optimizations: it shaves 30 MB from Koreader's memory use after browsing in the file manager, opening a (previously opened) EPUB, and triggering the various bottom settings panels
  • using my meson' size optimized build (buildtype = 'minsize'), combined with a pass on all libraries to ensure GNU visibility support is used, as well as trimming unnecessary features and statically linking when applicable: it shaves about 10 MB from the combined BSS+data+code sections

monolibtic is currently broken (because of the visibility patches), but technically fixable.

Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

I'm not up to snuff on the ft/hb API, but this looks sound ;).

@Frenzie Frenzie merged commit 3cdd235 into koreader:master Jun 12, 2023
2 checks passed
@benoit-pierre benoit-pierre deleted the pr/fix_most_freetype/harfbuzz_related_memory_leaks branch June 12, 2023 20:30
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.

None yet

4 participants