Omikhleia / resilient.sile

Advanced book classes and packages for the SILE typesetting system
MIT License
21 stars 4 forks source link

Lua 5.1 compatibility #70

Closed Omikhleia closed 8 months ago

Omikhleia commented 8 months ago

Closes #71

Stage 1:

Stage 2:

Stage 3 (release smoke tests)

Omikhleia commented 8 months ago

Luacheck CI update from https://github.com/Omikhleia/markdown.sile/pull/101 yields:

    packages/resilient/bookmatters/init.lua:80:94: accessing undefined field pow of global math
    packages/resilient/poetry/init.lua:181:40: accessing undefined field log10 of global math

Yet they seem to be ok:

SILE v0.14.13 (LuaJIT 2.1.1700008891)
> math.pow(2,3)
8
> math.log10(10)
1
Omikhleia commented 8 months ago

Not seeing any noticeable performance improvement, so this SILE v0.14.13 (LuaJIT 2.1.1700008891) (from the initial docker file in https://github.com/sile-typesetter/sile/discussions/1923) is weird.

With -q -e 'print(SILE.lua_version); os.exit()' it reports 5.1 -- perhaps just running that, actually.

Omikhleia commented 8 months ago

Luacheck CI update from Omikhleia/markdown.sile#101 yields:

    packages/resilient/bookmatters/init.lua:80:94: accessing undefined field pow of global math
    packages/resilient/poetry/init.lua:181:40: accessing undefined field log10 of global math

math.pow(x,y) is deprecated in Lua 5.3 (https://www.lua.org/manual/5.3/manual.html#8.2) --> x^y is apparently the way to go and works in 5.1 too.

math.log10(x) was deprecated in Lua 5.2 (https://www.lua.org/manual/5.2/manual.html#8.2), which says to use math.log(x,10). Not working in 5.1 though, as shown below, so I guess one has to go for math.log(x)/math.log(10)....

$ lua5.2
Lua 5.2.4  Copyright (C) 1994-2015 Lua.org, PUC-Rio
> print(math.log10(5))
0.69897000433602
> print(math.log(5,10))
0.69897000433602

$ lua5.1
Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> print(math.log10(5))
0.69897000433602
> print(math.log(5,10))
1.6094379124341
> -- Doh, ignoring the base in Lua 5.1 !!!!!!!!!!!!!
> print(math.log(5)/math.log(10))
0.69897000433602

This language is insane.

alerque commented 8 months ago

Yes, math in Lua is kind of insane. You haven't even scratched the surface of weirdness when you get into 32 bit vs. 64 bit numbers, bit-shift operators, etc. There isn't a clear succession of VMs because multiple parties that don't agree have continued developing the language in several directions.

For log10() it has been deprecated but not removed in any VM I know of, so you can add a luacheck exception for it and just use it for now, or as you noted do your own division:

$ for LUA in lua5.4 lua5.1 luajit; do $LUA -e 'print(math.log10(5))'; done
0.69897000433602
0.69897000433602
0.69897000433602
$ for LUA in lua5.4 lua5.1 luajit; do $LUA -e 'print(math.log(5)/math.log(10))'; done
0.69897000433602
0.69897000433602
0.69897000433602
alerque commented 8 months ago

Not seeing any noticeable performance improvement,

Between Lua 5.4 and LuaJIT? I've benchmarked the difference hundreds of times over the last couple years and LuaJIT is always consistently way faster. And not just 5% or 10% faster but 200–250%.

On the system I happen to be on right now with SILE master (not develop), the difference in rendering the SILE manual is 37.5s for Lua 5.4 vs. 18.9s for LuaJIT (or 16.2s for the OpenResty fork of LuaJIT). That's a bit over twice as fast.

The startup time for both is about the same so the difference on a one page document won't be huge, but the longer documents get the more obvious the difference should be.

alerque commented 8 months ago

With -q -e 'print(SILE.lua_version); os.exit()' it reports 5.1 -- perhaps just running that, actually.

The SILE.lua_version variable is reporting the ABI version, and LuaJIT is ABI 5.1 (hence why it can load and use the Rocks installed by luarocks --lua-version 5.1). You can see SILE.lua_isjit for a boolean status.

Omikhleia commented 8 months ago

The SILE.lua_version variable is reporting the ABI version, and LuaJIT is ABI 5.1 (hence why it can load and use the Rocks installed by luarocks --lua-version 5.1). You can see SILE.lua_isjit for a boolean status.

Thanks.

resilient -q -e 'print(SILE.lua_version); print(SILE.lua_isjit); os.exit()'
5.1
true

I'll do some perf comparisons after a clean reboot.

alerque commented 8 months ago

SILE requires and provides the compat53 patches. That should allow math.log() to accept a base in SILE running under Lua 5.1 even though plain Lua 5.1 does not.

Omikhleia commented 8 months ago

On a second thought, we'll want to avoid troubles later, so will enforce linting on vendored libraries, and ideally propose fixes upstream = affecting markdown.sile (for djot.lua and lunamark) and resilient.sile (tinyyaml though the situation is less clear here, see provided README)

EDIT: Acceptable warnings ignored in tinyyaml + opportunity to include a bug fix.

Omikhleia commented 8 months ago

Naive black-box performances - Let's go biblical.

So let's take a big file, compiling the Segond Bible... Same initial conditions = no .toc and .ref files, resilient style file and master simplified to ensure same fonts, only standard desktop running on the laptop, near to no network activity. Both tests are performed in sequence and use docker images (no other docker running on the host).

Lua 5.4 (custom made image, built some time ago with my rocks already installed, and a few extra fonts we are not going to use anyway)

$ time silex -u inputters.silm perfo/lsg.silm 
SILE v0.14.11.r21-g3b0ea5a (Lua 5.4)
<perfo/USX/001GEN.usx> as xml
[1] [2] [3] [4] 
...
[1709] [1710] [1711] <perfo/USX/066REV.usx> as xml
[1712] [1713] [1714] [1715] [1716] [1717] [1718] [1719] [1720] [1721] [1722] [1723] [1724] [1725] [1726] [1727] [1728] [1729] [1730] [1731] [1732] [1733] [1734] [1735] 
! Warning: table of contents has changed, please rerun SILE to update it.

real    32m21,037s
user    0m0,177s
sys 0m0,135s

LuaJIT 2.1 (made as stated above)

time resilient -u inputters.silm perfo/lsg.silm 
SILE v0.14.13 (LuaJIT 2.1.1700008891)
<perfo/USX/001GEN.usx> as xml
[1] [2] [3] [4] 
...
[1726] [1727] [1728] [1729] [1730] [1731] [1732] [1733] [1734] [1735] 
! Warning: table of contents has changed, please rerun SILE to update it.

real    20m44,838s
user    0m0,160s
sys 0m0,111s

Of course this is not a general test -- but on these two runs on big XML files with collated notes and references,

Indeed better (x 1.56 boost).

Omikhleia commented 8 months ago

At some point though (near page 1200), underfull/overfull message start to differ...

Omikhleia commented 8 months ago

At some point though (near page 1200), underfull/overfull message start to differ...

Actually started much earlier.

image

At some points, likely chapters (= bible books, on a new page) it gets in sync, and the sequences repeat.

image

Could be that I don't have the exact same version of Gentium Plus in both images. Could be floating number differences... Or something more subtle.

It's a problem for having reproducible output for a given book (and anything > 1pt starts getting embarrassing), but I am not going to care here for now: this is acceptable for a pre-alpha software.

alerque commented 8 months ago

It would be pretty easy to check whether the difference is the font, the SILE version (you are pitting v0.14.11 against v0.14.13 here) or the Lua version. I very much suspect the SILE version is the culprit, with a second guess being the font version difference. I have yet to run into anything at all that successfully runs without errors but gives different metrics between Lua versions. I don't know what math you're doing in libraries apart from what SILE does, but I'm pretty confident SILE's math isn't being thrown off.

I have a lot of tooling for comparing builds done with various different options like different Lua versions or SILE versions, but I don't have your input content or font version to compare with.

Omikhleia commented 8 months ago

Yep, the test is not perfect (a 0.11 + cherry-picked patches that make it close to 0.13 but not so, vs. a real 0.13, both built at different times so maybe no getting the exact same font and tooling, etc.) -- Most important I guess is the perf. boost, which I assume to be somehow "correct" and cool. The rest would need a better testing procedure, admittedly!

Omikhleia commented 8 months ago

Note for future investigation: the "end" sequence might be easier to extract than things that occurs in the middle of books:

image

Everything was again in sync until Revelations, so it could be interesting to process just these few books

EDIT: they were in sync since [1629] [1630] [1631] [1632] [1633] [1634] <perfo/USX/051COL.usx> as xml -- so over nearly 100p.

Omikhleia commented 8 months ago

Merging so I can start fetching dev luarocks and do some more tests.

Omikhleia commented 7 months ago

For the record: