MuPDF: fix build #1586
MuPDF: fix build #1586
Conversation
|
I could reproduce the CI failure locally after moving my system JPEG library out of the way. |
|
On a semi-related note, because cmake is hell, I've got a meson POC build system, if there's some interest. It does make composing things and experimenting much easier. |
|
Now that meson finally hit 1.0, and that we don't really care about native win32 anymore... I really, really, really wouldn't be opposed to it (despite my hangups about its approach to cross compilation), despite the potential disruption... RFC, though ;). |
|
Current state: compile the emulator on Linux, cross compile the emulator for Windows ( |
|
That's impressive ! and a bit frightening as I don't like my living room to be that revamped :)
Part of my dev worklow, after testing on the emulator, is building only libcrengine.so + libkoreader-cre.so for arm (+ their deps), and move them (only these 2 libs) over a recent koreader-kobo nightly on my device to test. |
Yes, absolutely.
Cool! |
Yeah, it does increase the base memory use, even if it potentially decreases the maximum memory needed when all the shared libraries are loaded. It does help taking advantage of LTO to further optimize and trim unused symbols. Anyway, it's optional ;) |
Well, if the performance improvment is noticable, we'll have to decide how to go with building nightlies & stable. And we would loose quite a bit of practicality with a monolithic libkoreader.so (like shipping debug/test libs to users). |
Patch the makefile link command so the build process does not try to link with system libraries (e.g. `-lpeg`), even when building the generator helpers (like `cnamedump.exe`). Since mupdf's build system is only in charge of building a static library, we can just drop the `$(LIBS)` part of the link command.
benoit-pierre
force-pushed
the
pr/fix_mupdf_build
branch
from
ee4176e
to
6a86fb6
Compare
|
Meson POC available here: koreader, koreader-base, and crengine. Notes:
TODO:
I've added a When building in return {
library = 'libs/libkoreader.so',
preloads = {
["libs/libkoreader-cre"] = "luaopen_cre",
["libs/libkoreader-djvu"] = "luaopen_djvu",
["libs/libkoreader-nnsvg"] = "luaopen_nnsvg",
["libs/libkoreader-xtext"] = "luaopen_xtext",
},
redirects = {
["czmq.1"] = true,
["freetype.6"] = true,
["harfbuzz.0"] = true,
["k2pdfopt.2"] = true,
["lept.5"] = true,
["lodepng"] = true,
["mupdf"] = true,
["sqlite3.0"] = true,
["utf8proc.2"] = true,
["z"] = true,
["zmq.4"] = true,
["zstd.1"] = true,
["SDL2-2.0.0"] = true,
["wrap-mupdf"] = true,
["blitbuffer"] = true,
},
} |
|
At a very quick glance, that looks rather sexy ;). I can certainly see the appeal of mesonifying everything to sidestep annoying upstream buildsystems, too... Is there a tool that helps generate meson.build files or was that a lot of hand-made tweaking? |
|
Oh neat.
@NiLuJe BTW -- nifty little trick, you can actually trigger a cross compilation like this: Practically everything from a cross file is not actually necessary, if that's what you were referring to. (And the one thing you officially do "need" you can still do without.) |
|
I can't judge about the build system change and pros/cons, I'm not competent enough - but it indeed looks more readable than CMake. Some practical questions, just to get an idea how complicated it would get for me :/:
|
|
And btw... who are you ?!
What next ? I'm a bit fearing you might rewrite crengine in rust :/ |
Oh, I like that, thanks! (Last I checked, there was a very strong assumption that what's in env vars should only ever affect the host, not the target, so, e.g., setting CFLAGS with arm-specific flags in the env would break pretty much anything that required running the host compiler during the build (e.g., code generation tools)). |
|
https://mesonbuild.com/Reference-tables.html#environment-variables-per-machine We support autotools-style |
On the one hand I don't know either, on the other hand a quick glance comes up with https://github.com/openstenoproject/plover which is an extremely impressive piece of software I've known about for years. @benoit-pierre I think the bigger question is… where'd you find all the time? 😆 Either that or you can do in hours what would take me at least a full working week. ;-) |
|
If you are updating MuPdf please show a little love to k2pdfopt :) |
|
You probably already saw btw, but #1203 has pretty much all of the work required in it. Specifically see https://github.com/ezdiy/libk2pdfopt |
|
Should we merge this PR quickly, for master to build ? (that is, if you approve the changes) |
100% organic made.
# manually setup for `remarkable` (no `monolibtic`) without building
▸ make KODEBUG=1 TARGET=remarkable setup
# rebuild only libkoreader-cre.so and dependencies (about ~500 targets).
▸ ninja -C build/remarkable-debug libkoreader-cre.so
# copy to target device
▸ scp build/remarkable-debug/libkoreader-cre.so …There's no This take about 140s on my (10 years old) machine, starting from a cold compile cache. As for space, compared to the current stats on master, with (almost) the same builds:
Meson impose the location of sub-projects, so
That still leaves quite a few things:
I certainly would welcome less git submodules (always a PITA to deal with).
I don't see why not.
I don't think so.
Again, "monolibtic" mode is optional, and the LTO stuff can be disabled in
That's was really more of quick way to test cross-compiling for another platform (and the
I also have no love for Windows, so I'm going to drop it.
User, yes. Reading times have recently plummeted..
I've just done the minimum for sanity's sake (see
Yeah, I've not looked into it yet. What would be helpful is a checklist of tests to ensure everything is indeed working as expected: with so many options, it's hard to know if I've hit all the code paths (like the support for highlighting). Tentative list:
|
Meson's CI regularly tests Ubuntu Bionic, which is older than those, and has GCC 7.4; in general, there's no need to worry about using the environment of a still supported, much less quite recent, distro. ... As a generic rule? Meson tries to avoid arbitrarily restricting the version of your compiler, but in order to integrate well it does sometimes need to know how to activate features. Meson usually version guards this when added, and doesn't gratuitously drop support for old compilers (and we literally cannot drop support for 7.4, since, again, we target that in our CI). There is support code as old as 4.8, which is the first version of GCC released after Meson was started (it handles warning arguments). I do not know whether anyone still relies on it, but there's no plan to drop that either, since it already exists and isn't a burden to leave in place. Any given user probably does not need GCC 2.x support. If you did, Meson would probably be a bad choice, because Meson makes no effort to support systems or toolsets that were ancient and obsolete before it was started. :P |
That's near the top of my TODO list, FWIW (I'll hopefully get to it in the next couple of months...). Not that it'll help much with the speed of LTO linking, the only really faster alternatives I know of are Clang in thin-lto mode, coupled with lld or even (or especially, I should say ^^) mold ;). |
(re: gcc 4.8) That's (nearly? can't remember the exact cutoff) the last GCC version licensed under GPL v2, so, that's what the GCC version available in (old) Android NDKs is based on (and we haven't made the move to Clang yet there). Or, even, for the same licensing reasons, some really old OS X toolchains, if you're feeling mad ;D. |
|
Sounds painful, but if you do need it and you run into issues with it, I am happy to merge untested PRs on the honor system, that support such old GCCs better. (It will probably just work with fewer optional features though.)
... for which definition of mad? 👀 |
diff --git a/setupkoenv.lua b/setupkoenv.lua
index be1b2e228..37af263d9 100644
--- a/setupkoenv.lua
+++ b/setupkoenv.lua
@@ -15,7 +15,7 @@ if ffi.os == "Windows" then
end
local ffi_load = ffi.load
-- patch ffi.load for thirdparty luajit libraries
-ffi.load = function(lib)
+ffi.load = function(lib, global)
io.write("ffi.load: ", lib, "\n")
local loaded, re = pcall(ffi_load, lib)
if loaded then return re end
@@ -27,6 +27,6 @@ ffi.load = function(lib)
error("Not able to load dynamic library: " .. lib)
else
io.write("ffi.load (assisted searchpath): ", lib_path, "\n")
- return ffi_load(lib_path)
+ return ffi_load(lib_path, global)
end
endshould fix it. |
|
Of course! That must be it. |
We *do* actually use the optional global argument sometime... Namely, for librt in ffi/posix_h. c.f., koreader/koreader-base#1586 (comment)
|
Added that to koreader/koreader#10705 because why not ;D. |
|
Confirmed that it solves it without needing any else
ffi.load = function(lib, global)
log("ffi.load: " .. lib)
return ffi_load(lib, global)
end
end
So, that means 1) there is a global table named |
|
c.f., In LuaJIT, it's the default for (Require also does full symbol resolution on load via |
|
In a way, thanks for confirming that we did indeed need it in the global namespace, because I had never actually tested this ;D. |
|
Btw, which explicite ffi.load() where then benefits from this fix so that |
|
This one: Lines 434 to 440 in a938d08 If you require'ed |
|
(I double-checked, that's our only ffi.load that sets |
|
OK, so And I assume no need to ffi.load("libc"), and that the other symbols from libc are put there by LuaJIT:
|
|
Btw, no speedup noticed in my usual benchmark test (a French Bible EPUB with > 1000 fragments), same times (30s loading + 12s rendering with no cache, 3s loading + 23s rerendering if cache rendering hash mistmatch) whether latest nightly or this monolibtic on my Kobo. |
Note that the CI kindlepw2 and kobo build use |
|
Not enough time with it to see how it grows (it's possible I've seen it at 100M earlier before I installed today's nightly, but not sure). |
|
I imagine you'll probably want to open both an ePub and a PDF to make the test fair? |
|
Given that PDF gets BB of page cached in memory, it's not easy to distinguish the memory used by the app vs. what is cached. |
We *do* actually use the optional global argument sometime... Namely, for librt in ffi/posix_h. c.f., koreader/koreader-base#1586 (comment)
We *do* actually use the optional global argument sometime... Namely, for librt in ffi/posix_h. c.f., koreader/koreader-base#1586 (comment)
We *do* actually use the optional global argument sometime... Namely, for librt in ffi/posix_h. c.f., koreader/koreader-base#1586 (comment)
|
Just a note, as I don't think there's anything we can do as it feels upstream, but newer MuPDF seems worse with some HTML than our current one:
This is the nice rendering I currently get with KOReader (including the 2nd definition that is missing in the MuPDF 1.13 above):
|
|
What's the CSS? Could be nicer by accident. :) |
|
No CSS involved. Just the html result (which has some style= attributes): |
|
It does look like the older MuPDF may be listening to display: inline and the newer isn't? Curious. |
|
Wanted to "fix" this dict.lua, so downloaded the AppImage from https://github.com/benoit-pierre/koreader/actions/runs/5578752636 v2023.04-8083-g36de7a303_2023-07-17.AppImage |
|
You confused me there — you're not talking about "my" AppImage build but some experimental Meson build. :-) We use |
|
As expected with the MuPDF update, HTML dict users can now meet the crappier HTML rendering mentionned above at #1586 (comment) :/ I guess it can only be worked around by massaging the HTML in each dict specific .lua tweak. |
Patch the makefile link command so the build process does not try to link with system libraries (e.g.
-lpeg), even when building the generator helpers (likecnamedump.exe). Since mupdf's build system is only in charge of building a static library, we can just drop the$(LIBS)part of the link command.This change is