-
Notifications
You must be signed in to change notification settings - Fork 105
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
Conversation
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.) |
Test builds: |
No strong opinions about it as long as it stays Android-specific in practice ;). |
It would be nice to have this on the next release. Android 18-22 seem broken since the unversioning. |
19b5bde
to
21ebda7
Compare
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). |
21ebda7
to
e5a6794
Compare
There was a problem hiding this 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?
Yep, good catch. |
e5a6794
to
f7f9341
Compare
There was a problem hiding this 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: 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.
f7f9341
to
e54c96a
Compare
We can get rid of the `libs` and `sdcv` symlink in application files. Depend on koreader/koreader-base#1920. Close #12348.
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 inassets.7z
. The final list of native libraries is: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
andlibs
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
utils/gen_linker_exports.sh
and our FFI cdecls to only keep what we needffi.loadlib(…)
instead offfi.load
to load our libraries (and a custom package loader to handle bundled native Lua modules)android-luajit-launcher
: koreader/android-luajit-launcher@master...benoit-pierre:android-luajit-launcher:monolibticffi.loadlib
moduleApart from a host of commits to facilitate bundling everything, another big change is the
ffi.loadlib
helper, replacingffi.load
andutil.ffiLoadCandidates
for loading our libraries.Example use:
On Linux, the code will attempt to load
libSDL2-2.0.so.0
inlibs/
and then from the system, thenlibSDL2-2.0.so
if that failed, and fallback to tryinglibSDL2.so
.In monolibtic mode, loading a bundled library will
ffi.load
the monolibtic library. A custom package loader handle therequire("libs/libkoreader-lfs")
case.Code size
Impact on code size (bss+data+text, ignoring executables and the cURL library):
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