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

Monolibtic koreader library #1920

Merged
merged 4 commits into from
Oct 1, 2024
Merged

Conversation

benoit-pierre
Copy link
Contributor

@benoit-pierre benoit-pierre commented Aug 31, 2024

After fixing the libraries sonames on Android in #1907 (to fix koreader/koreader#12348), the dynamic linker on API level 18 (and 19 too I assume) craps out during symbol resolution (cf. comment).

The only way I could get around those type of issues on my meson branch was to bundle (almost) everything into one big library, and this PR does just that.

At first I tried a partial solution, not bundling in native Lua modules, but of course this just resulted in the dynamic loader having another symbol resolution fit when attempting to load common/ssl.so… So you get the full monty, native Lua modules included, no shared objects in assets.7z. The final list of native libraries is:

    71864  1981-01-01 01:01   lib/arm64-v8a/lib7z.so
  1014704  1981-01-01 01:01   lib/arm64-v8a/libc++_shared.so
     4728  1981-01-01 01:01   lib/arm64-v8a/libioctl.so
 18639648  1981-01-01 01:01   lib/arm64-v8a/libkoreader-monolibtic.so
    19584  1981-01-01 01:01   lib/arm64-v8a/libluajit-launcher.so
   689520  1981-01-01 01:01   lib/arm64-v8a/libluajit.so
  1908280  1981-01-01 01:01   lib/arm64-v8a/libsdcv.so

For now, monolibtic mode is only enabled for Android (but I tested the emulator and kindlepw2 builds too).

On Android, there's no need for the sdcv and libs symlinks in the application files directory anymore: both executable and libraries are directly loaded from the application native libraries directory (fixing this corner case).

Implementation

ffi.loadlib module

Apart from a host of commits to facilitate bundling everything, another big change is the ffi.loadlib helper, replacing ffi.load and util.ffiLoadCandidates for loading our libraries.

Example use:

function util.loadSDL2()
    if libSDL2 == nil then
        local ok
        ok, libSDL2 = pcall(ffi.loadlib,
            "SDL2-2.0", 0,
            "SDL2-2.0", nil,
            "SDL2", nil
        )
        if not ok then
            print("SDL2 not loaded:", libSDL2)
            libSDL2 = false
        end
    end
    return libSDL2 or nil
end

On Linux, the code will attempt to load libSDL2-2.0.so.0 in libs/ and then from the system, then libSDL2-2.0.so if that failed, and fallback to trying libSDL2.so.

In monolibtic mode, loading a bundled library will ffi.load the monolibtic library. A custom package loader handle the require("libs/libkoreader-lfs") case.

Code size

Impact on code size (bss+data+text, ignoring executables and the cURL library):

build master monolibtic
Android (ARM) 20.0 MB 15.1 MB (-4.9 MB, -24.5 %)
Android (ARM64) 28.2 MB 21.2 MB (-7.0 MB, -24.8 %)
Android (x86) 28.8 MB 25.0 MB (-3.8 MB, -13.2 %)
Android (x86_64) 31.5 MB 27.1 MB (-4.4 MB, -14.0 %)
AppImage 30.7 MB 26.7 MB (-4.0 MB, -13.0 %)
Kindle PW2 18.7 MB 16.1 MB (-2.7 MB, -13.9 %)

Impact on memory use: a slight increase at startup (starting with the FM, +0-2 MB), but a reduction after opening a few documents of different types (-2-8 MB).

TODO

If the solution is accepted, split the change into smaller PRs for merging: preliminary cleanups, ffi.loadlib module / helper, FFI cdecls cleanups, cURL static build, other static libraries support, final monolitic commit.


This change is Reviewable

@poire-z
Copy link
Contributor

poire-z commented Aug 31, 2024

For now, monolibtic mode is only enabled for Android

I don't mind for Android, but I hope this won't go further - I would like to keep my kobo nightlies with smaller libraries I can quickly compile independantly without having to rebuild the whole thing.

(And on principle, I don't like the idea of a single monolithic library, but what do I know.)
Also, I remember that when I tested it months ago on my Kobo, it wasn't at all faster than a current nightly.

@benoit-pierre
Copy link
Contributor Author

@NiLuJe
Copy link
Member

NiLuJe commented Sep 1, 2024

No strong opinions about it as long as it stays Android-specific in practice ;).

@pazos
Copy link
Member

pazos commented Sep 18, 2024

It would be nice to have this on the next release. Android 18-22 seem broken since the unversioning.

@benoit-pierre
Copy link
Contributor Author

It's not the unversioned libraries that caused the issue, but one or the combination of the libk2pfopt / mupdf / tesseract / wrap-mupdf optimizations (statically building / merging some of them).

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.

Reviewed 58 of 58 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @benoit-pierre)


thirdparty/cmake_modules/koreader_thirdparty_libs.cmake line 199 at r1 (raw file):

target_link_libraries(pthread INTERFACE Threads::Threads)

# srell

Wrong comment for SQLite?

@benoit-pierre
Copy link
Contributor Author

thirdparty/cmake_modules/koreader_thirdparty_libs.cmake line 199 at r1 (raw file):

target_link_libraries(pthread INTERFACE Threads::Threads)

# srell

Wrong comment for SQLite?

Yep, good catch.

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.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @benoit-pierre)

Bundle all libraries and native Lua modules into one big library. This
is automatically enabled on Android to avoid the vagaries of the dynamic
loader behavior on older versions (particularly KitKat, and Jelly Bean).

In monolibtic mode, `ffi.loadlib` will automatically redirect loading
a bundled library to use our jumbo one. Similarly, a custom package
loader takes care of bundled native Lua modules.

This also allow for some specific optimizations: we can exports only
the symbols needed (via `utils/gen_linker_exports.sh`), reducing code
size (especially on Android, where `-ffunction-sections` is used), and
potentially optimizing some inter-libraries function calls.
@Frenzie Frenzie merged commit 168d001 into koreader:master Oct 1, 2024
2 of 3 checks passed
@benoit-pierre benoit-pierre deleted the monolibtic branch October 1, 2024 18:46
Frenzie pushed a commit to koreader/koreader that referenced this pull request Oct 2, 2024
We can get rid of the `libs` and `sdcv` symlink in application files.

Depend on koreader/koreader-base#1920.

Close #12348.
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.

Crash at startup on Tolino Vision 5
5 participants