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

The SILE you know, but Rust-ier. #1762

Merged
merged 56 commits into from
Sep 12, 2023
Merged

Conversation

alerque
Copy link
Member

@alerque alerque commented Apr 6, 2023

I've tried a couple times before to start a Rust rewrite or wrapper or submodule system or various other experiments. They all went sideways for one reason or another, but I know a lot more about Rust now and the ecosystem has grown. Notably mlua has a lot more bells and whistles now.

My main goal for starters is to try wrapping the SILE we already have in a Rust based CLI that acts as it's own Lua interpreter and just runs all the code we already have. Once that works, the next step will be to setup building new modules such that code can be written in Rust and loaded into and called from Lua. After that, we should be able to write code in either language without hampering people's ability to modify/hack on/re-implement anything in Lua.

Once that PoC works, getting paths and builds worked out to be compatible with LuaRocks on the system and whatever version of Lua that runs so that 3rd party modules and document expectations just keep working will be a little tricky, but already evidently possibly.

Incidentally this approach does not obsolete the idea that we've never quite achieved of using SILE as a Lua module. That sort of works, but is really hackish to use since the CLI still has to be installed anyway. Already working on this I can see the core will have to be properly modular for this to work, so it will force the internal API to be sensible thus making the use as a Lua library more sensible.

@alerque
Copy link
Member Author

alerque commented Apr 6, 2023

As of posting this PR, the new CLI doesn't accept any arguments except --help and --version and without those arguments is hard coded to render tests/absmin.xml.

The good news is that it does work end to end: it loads up and parses the input, processes it, and outputs a PDF! It does not work to compile some more complicated things such as the manual yet which causes a stack overflow somewhere.

@ctrlcctrlv
Copy link
Member

Really cool idea. A lot of the things SILE does would make good Rust crates. 👍

@alerque
Copy link
Member Author

alerque commented Apr 18, 2023

If nothing else this is going to force me to finish modularization the Lua interfaces. Rust really doesn't like sharing variables around or functions causing side effects! Also that work will make SILE much more ready for use as a Lua library if that's what anybody needs. Already it's shaken out quite a few bugs and bad assumptions about context in the Lua code.

So far it will typeset quite a few of the test cases, but stack overflows when trying to work with our trace stack (e.g. when handling warnings).

I'm really eager to get even just the outer wrapper spun up and benchmark it so I know if this is a viable way to run at all. If it is it would provide some packaging opportunities (e.g. embedding the source files in a single binary and running on Windows!) that we've been lacking.

And yes, beyond the initial outer wrapper the next thing to do with be to re-implement a few key modules in Rust and load them and see how they affect speed.

@alerque alerque force-pushed the riir branch 2 times, most recently from 0013265 to 56a3d09 Compare April 22, 2023 19:01
@alerque
Copy link
Member Author

alerque commented Apr 23, 2023

This now works to compile the manual (in release mode --disable-debug only, with debugging enabled it turns up issues in our existing C modules). I benchmarked it (10 runs each with Lua 5.4 and LuaJIT, Rust vs. Lua CLIs) and the results are that they run almost the same speed, just 8% or so off at max. The Rust CLI startup time is more which affects small docs more than large ones, but usually runs slightly faster overall after it starts up. But honestly the difference isn't far enough out of the noise to matter either way. The additional flexibility of shipping our own Lua interpreter is going to be more than enough advantage, plus with being able to write modules in Rust we should be able to start chopping away at the speed.

Compiling long documents like the manual is faster in the Rust CLI:

$ export FONTCONFIG_FILE=/home/caleb/projects/sile-typesetter/sile/fontconfig.conf
$ hyperfine --warmup 6 './sile -q documentation/sile.sil' './sile-lua -q documentation/sile.sil'
Benchmark 1: ./sile -q documentation/sile.sil
  Time (mean ± σ):     23.134 s ±  0.331 s    [User: 22.744 s, System: 0.302 s]
  Range (min … max):   22.637 s … 23.663 s    10 runs

Benchmark 2: ./sile-lua -q documentation/sile.sil
  Time (mean ± σ):     24.922 s ±  0.296 s    [User: 23.881 s, System: 0.951 s]
  Range (min … max):   24.528 s … 25.426 s    10 runs

Summary
  './sile -q documentation/sile.sil' ran
    1.08 ± 0.02 times faster than './sile-lua -q documentation/sile.sil'

Compiling tiny documents like our minimum working example is faster under plain Lua:

$ hyperfine --warmup 2 './sile tests/absmin.xml' './sile-lua tests/absmin.xml'
Benchmark 1: ./sile tests/absmin.xml
  Time (mean ± σ):     112.9 ms ±   3.7 ms    [User: 92.0 ms, System: 19.8 ms]
  Range (min … max):   109.0 ms … 126.0 ms    25 runs

Benchmark 2: ./sile-lua tests/absmin.xml
  Time (mean ± σ):     111.2 ms ±   3.3 ms    [User: 89.4 ms, System: 20.7 ms]
  Range (min … max):   107.6 ms … 119.3 ms    26 runs

Summary
  './sile-lua tests/absmin.xml' ran
    1.02 ± 0.04 times faster than './sile tests/absmin.xml'

@alerque alerque force-pushed the riir branch 2 times, most recently from 14175ba to 84b7f2d Compare April 24, 2023 09:32
@ctrlcctrlv
Copy link
Member

I think I can explain the speed difference. From my review of the code it seems like you run Lua in the main thread. I'd put the interpreter in another thread, actually I'd put it in the global namespace (static mut).

I'd also take full advantage of the async feature of mlua, and when we're doing our startup, do that asynchronously, like the real interpreter does, not just feed it a ton of Lua code to parse JIT.

If there's a way to pre-JIT compile (what an oxymoron lol) in luajit, I'd look into that too.

@alerque
Copy link
Member Author

alerque commented Apr 25, 2023

All good points. I can think of a couple others: we could snapshot the Lua VM after SILE init and after loading all the (embeded? amalgumated?) resources, then just reuse that for actual processing. Also since we sit outside the VM we can snapshot runs part way through and go back and replay them (for example to generate TOCs).

In any event starting out within a couple percentage points with no optimization at all makes me pretty confident this is going to be the way to go moving forward.

I do want to work things out so that we can play nice with system-installed 3rd party packages, right now this PR only works with ./configure --without-system-luarocks, but I'm sure I can also make it work --with.

@alerque
Copy link
Member Author

alerque commented Apr 25, 2023

There, now it works --with-system-luarocks or --without. Next up, read LUA_PATH at run time so that users that only install plugins locally to a project or their user can use luarocks path to setup for their SILE run...

@alerque alerque added this to the v0.15.0 milestone Apr 27, 2023
@alerque alerque added the tooling Build tooling, release management, and packaging processes label Apr 27, 2023
@ctrlcctrlv
Copy link
Member

ctrlcctrlv commented Apr 28, 2023

@alerque I tried to build this and had another thought.

We should take the opportunity of this Rust release to draw a line in the sand and distribute Lua with SILE.

And then only distribute LuaJIT.

I was talking to @MatthewBlanchard about it, whom I consider to be a Lua expert, and it just makes sense. LuaJIT bytecode is stable minor release to minor release. It means we could distribute the bytecode with SILE.

Patch for ya.

diff --git a/Cargo.toml b/Cargo.toml
index 8c502d6b..92232355 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -33,7 +33,7 @@ required-features = ["cli"]
 
   [dependencies.mlua]
   version = "0.8"
-  features = [ "macros", "vendored" ]
+  features = [ "macros", "vendored", "luajit" ]
 
 [build-dependencies]
 
@@ -57,11 +57,6 @@ required-features = ["cli"]
 
 [features]
 default = ["cli", "bash", "elvish", "fish", "manpage", "powershell", "zsh"]
-lua54 = ["mlua/lua54"]
-lua53 = ["mlua/lua53"]
-lua52 = ["mlua/lua52"]
-lua51 = ["mlua/lua51"]
-luajit = ["mlua/luajit"]
 completions = ["cli", "clap_complete"]
 cli = ["clap"]
 bash = ["completions"]
diff --git a/Makefile.am b/Makefile.am
index 6f5da94e..637d6b9b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -82,13 +82,6 @@ else
 CARGO_RELEASE_ARGS = --release --locked
 endif
 
-if LUAJIT
-MLUAVER = luajit
-else
-MLUAVER = lua$(LUA_SHORT_VERSION)
-endif
-CARGO_FEATURE_ARGS = --features $(MLUAVER)
-
 CARGO_ENV = CARGO_TARGET_DIR=@abs_top_builddir@/target SILE_PATH=$(SILE_PATH)
 RUST_BIN = @abs_top_builddir@/target/@RUST_TARGET_SUBDIR@/$(PACKAGE_NAME)
 _RUST_OUT = @abs_top_builddir@/target/@RUST_TARGET_SUBDIR@/.cargo_out_dir
diff --git a/build-aux/ax_lua.m4 b/build-aux/ax_lua.m4
index e9b95225..eed2fab2 100644
--- a/build-aux/ax_lua.m4
+++ b/build-aux/ax_lua.m4
@@ -7,10 +7,7 @@ AC_DEFUN([AX_PROG_LUA],
   dnl Make LUA a precious variable.
   AC_ARG_VAR([LUA], [The Lua interpreter, e.g. /usr/bin/lua5.1])
 
-  dnl Find a Lua interpreter.
-  AM_COND_IF([LUAJIT],
-	  [_ax_lua_interpreter_list="luajit luajit-2.1.0-beta3 luajit-2.0.5 luajit-2.0.4 luajit-2.0.3"],
-	  [_ax_lua_interpreter_list="lua lua5.4 lua54 lua5.3 lua53 lua5.2 lua52 lua5.1 lua51 lua5.0 lua50"])
+  _ax_lua_interpreter_list="luajit luajit-2.1.0-beta3 luajit-2.0.5 luajit-2.0.4 luajit-2.0.3"
 
   m4_if([$1], [],
   [ dnl No version check is needed. Find any Lua interpreter.
@@ -78,14 +75,12 @@ AC_DEFUN([AX_PROG_LUA],
 	AC_SUBST([LUA_VERSION], [$ax_cv_lua_version])
 	AC_SUBST([LUA_SHORT_VERSION], [`echo "$LUA_VERSION" | $SED 's|\.||'`])
 
-	AM_COND_IF([LUAJIT], [
-		AC_CACHE_CHECK([for $ax_display_LUA jit version], [ax_cv_luajit_version],
-			[ ax_cv_luajit_version=`$LUA -e 'print(jit and jit.version:match "(%d+%..+)")'` ])
-		AS_IF([test "x$ax_cv_luajit_version" = 'x'],
-			[AC_MSG_ERROR([invalid Lua version number])])
-		AC_SUBST([LUAJIT_VERSION], [$ax_cv_luajit_version])
-		AC_SUBST([LUAJIT_SHORT_VERSION], [`echo "$LUAJIT_VERSION" | $SED 's|\.|§|;s|\..*||;s|§|.|'`])
-	])
+	AC_CACHE_CHECK([for $ax_display_LUA jit version], [ax_cv_luajit_version],
+		[ ax_cv_luajit_version=`$LUA -e 'print(jit and jit.version:match "(%d+%..+)")'` ])
+	AS_IF([test "x$ax_cv_luajit_version" = 'x'],
+		[AC_MSG_ERROR([invalid Lua version number])])
+	AC_SUBST([LUAJIT_VERSION], [$ax_cv_luajit_version])
+	AC_SUBST([LUAJIT_SHORT_VERSION], [`echo "$LUAJIT_VERSION" | $SED 's|\.|§|;s|\..*||;s|§|.|'`])
 
     dnl The following check is not supported:
     dnl At times (like when building shared libraries) you may want to know
@@ -245,21 +240,17 @@ AC_DEFUN([AX_LUA_HEADERS],
   AC_ARG_VAR([LUA_INCLUDE], [The Lua includes, e.g. -I/usr/include/lua5.1])
 
   dnl  Some default directories to search.
-  AM_COND_IF([LUAJIT],
-	  [_ax_lua_include_list="/usr/include/luajit-$LUAJIT_VERSION /usr/include/luajit-$LUAJIT_SHORT_VERSION /usr/local/include/luajit-$LUAJIT_VERSION /usr/local/include/luajit-$LUAJIT_SHORT_VERSION"],
-	  [_ax_lua_include_list="/usr/include/lua$LUA_VERSION /usr/include/lua/$LUA_VERSION /usr/include/lua$LUA_SHORT_VERSION /usr/local/include/lua$LUA_VERSION /usr/local/include/lua-$LUA_VERSION /usr/local/include/lua/$LUA_VERSION /usr/local/include/lua$LUA_SHORT_VERSION"])
+  _ax_lua_include_list="/usr/include/luajit-$LUAJIT_VERSION /usr/include/luajit-$LUAJIT_SHORT_VERSION /usr/local/include/luajit-$LUAJIT_VERSION /usr/local/include/luajit-$LUAJIT_SHORT_VERSION"
 
   dnl Try to find the headers.
   _ax_lua_saved_cppflags=$CPPFLAGS
   CPPFLAGS="$CPPFLAGS $LUA_INCLUDE"
-  AC_CHECK_HEADERS([lua.h lualib.h lauxlib.h luaconf.h])
-  AM_COND_IF([LUAJIT], [AC_CHECK_HEADERS([luajit.h])])
+  AC_CHECK_HEADERS([luajit.h lua.h lualib.h lauxlib.h luaconf.h])
   CPPFLAGS=$_ax_lua_saved_cppflags
 
   dnl Try some other directories if LUA_INCLUDE was not set.
   AS_IF([test "x$LUA_INCLUDE" = 'x' &&
 		 test "x$ac_cv_header_lua_h" != "xyes" ||
-		 test "x$with_luajit" = "xyes" &&
 		 test "x$ac_cv_header_luajit_h" != 'xyes'],
     [ dnl Try some common include paths.
 	for _ax_include_path in $_ax_lua_include_list; do
@@ -276,8 +267,7 @@ AC_DEFUN([AX_LUA_HEADERS],
 
         _ax_lua_saved_cppflags=$CPPFLAGS
         CPPFLAGS="$CPPFLAGS -I$_ax_include_path"
-        AC_CHECK_HEADERS([lua.h lualib.h lauxlib.h luaconf.h])
-		AM_COND_IF([LUAJIT], [AC_CHECK_HEADERS([luajit.h])])
+        AC_CHECK_HEADERS([luajit.h lua.h lualib.h lauxlib.h luaconf.h])
         CPPFLAGS=$_ax_lua_saved_cppflags
 
         AS_IF([test "x$ac_cv_header_lua_h" = 'xyes'],
@@ -397,17 +387,16 @@ AC_DEFUN([AX_LUA_LIBS],
     dnl Try to find the Lua libs.
     _ax_lua_saved_libs=$LIBS
     LIBS="$LIBS $LUA_LIB"
-	AM_COND_IF([LUAJIT],
-			[AC_SEARCH_LIBS([lua_load],
-				[luajit$LUA_VERSION luajit$LUA_SHORT_VERSION luajit-$LUA_VERSION luajit-$LUA_SHORT_VERSION luajit],
-				[_ax_found_lua_libs='yes'],
-				[_ax_found_lua_libs='no'],
-				[$_ax_lua_extra_libs])],
-			[AC_SEARCH_LIBS([lua_load],
-				[lua$LUA_VERSION lua$LUA_SHORT_VERSION lua-$LUA_VERSION lua-$LUA_SHORT_VERSION lua],
-				[_ax_found_lua_libs='yes'],
-				[_ax_found_lua_libs='no'],
-				[$_ax_lua_extra_libs])])
+	AC_SEARCH_LIBS([lua_load],
+		[luajit$LUA_VERSION luajit$LUA_SHORT_VERSION luajit-$LUA_VERSION luajit-$LUA_SHORT_VERSION luajit],
+		[_ax_found_lua_libs='yes'],
+		[_ax_found_lua_libs='no'],
+		[$_ax_lua_extra_libs])
+	AC_SEARCH_LIBS([lua_load],
+		[lua$LUA_VERSION lua$LUA_SHORT_VERSION lua-$LUA_VERSION lua-$LUA_SHORT_VERSION lua],
+		[_ax_found_lua_libs='yes'],
+		[_ax_found_lua_libs='no'],
+		[$_ax_lua_extra_libs])
 	LIBS=$_ax_lua_saved_libs
 
     AS_IF([test "x$ac_cv_search_lua_load" != 'xno' &&
diff --git a/configure.ac b/configure.ac
index 8302eeae..c0a3a1f5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -45,11 +45,6 @@ AC_ARG_ENABLE([font-variations],
                              [Disable support for OpenType variations and variable fonts that requires HarfBuzz subsetter library]))
 AM_CONDITIONAL([FONT_VARIATIONS], [test "x$enable_font_variations" != "xno"])
 
-AC_ARG_ENABLE([linklua],
-              AS_HELP_STRING([--disable-linklua],
-                             [Don’t link lua library with dylibs]))
-AM_CONDITIONAL([LINKLUA], [test "x$enable_linklua" != "xno"])
-
 # TODO: Refactor fontconfig check to allow Appkit/DirectWrite as alternatives, maybe default to off on Darwin
 # AC_ARG_WITH([fontconfig],
 #             AS_HELP_STRING([--without-fontconfig],
@@ -72,11 +67,6 @@ AC_ARG_WITH([system-luarocks],
 AM_CONDITIONAL([SYSTEM_LUAROCKS], [test "x$with_system_luarocks" = "xyes"])
 AC_SUBST([SYSTEM_LUAROCKS])
 
-AC_ARG_WITH([luajit],
-            AS_HELP_STRING([--with-luajit],
-                           [Run under LuaJIT instead of Lua]))
-AM_CONDITIONAL([LUAJIT], [test "x$with_luajit" = "xyes"])
-
 AC_ARG_WITH([manual],
             AS_HELP_STRING([--with-manual],
                            [Rebuild manual and install to system’s PDF documentation directory]))
@@ -153,28 +143,14 @@ AM_COND_IF([DEPENDENCY_CHECKS], [
         fi
     ])
 
-    case $host_os in
-        msys)
-            LUA_VERSION=5.3 # By fiat. This is obviously not good.
-            ;;
-        *)
-            AX_PROG_LUA([5.1])
-            ;;
-    esac
-
+    AX_PROG_LUA([5.1])
     AX_LUA_HEADERS
     AX_LUA_LIBS
 
     AM_COND_IF([SYSTEM_LUAROCKS], [
-        AS_IF([test "$LUA_SHORT_VERSION" -lt 52], [
-            AM_COND_IF([LUAJIT], [], [
-                AX_LUA_MODULE([bit32], [bit32])
-            ])
-        ])
+	AX_LUA_MODULE([bit32], [bit32])
         AX_LUA_MODULE([cassowary], [cassowary])
-        AS_IF([test "$LUA_SHORT_VERSION" -lt 53],
-            AX_LUA_MODULE([compat53], [compat53])
-        )
+        AX_LUA_MODULE([compat53], [compat53])
         AX_LUA_MODULE([cosmo], [cosmo])
         AX_LUA_MODULE([cldr], [cldr])
         AX_LUA_MODULE([fluent], [fluent])
diff --git a/src/Makefile.am b/src/Makefile.am
index 70f72913..8924071b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,19 +1,14 @@
 ACLOCAL_AMFLAGS = -I ../build-aux
 
-if LINKLUA
-MY_LUA_LIB=$(LUA_LIB)
-UNDEFINED=-no-undefined
-else
-MY_LUA_LIB=
-UNDEFINED=
-endif
-
 if SYSTEM_LIBTEXPDF
 LIBTEXPDF_LIB=-ltexpdf
 else
 LIBTEXPDF_LIB=../libtexpdf/libtexpdf.la
 endif
 
+MY_LUA_LIB=
+UNDEFINED=
+
 pkglib_LTLIBRARIES = justenoughharfbuzz.la justenoughlibtexpdf.la justenoughfontconfig.la fontmetrics.la svg.la
 justenoughharfbuzz_la_SOURCES = justenoughharfbuzz.c hb-utils.c hb-utils.h
 justenoughharfbuzz_la_LDFLAGS = -module -avoid-version -shared $(UNDEFINED)

@alerque
Copy link
Member Author

alerque commented Apr 29, 2023

Long term I think that's likely where this is headed and I'll keep the patch around, but there are a couple things holding me back:

  1. When possible I want at least one transition release where we help people migrate. Sometimes in the interest of making major progress I'd consider just moving on, but so far I haven't run into anything at all that makes it even hard to take this one step at a time and experiment with the Rust entry point with included Lua interpreter while still including a pure Lua entry point.

  2. I still want to mess around with typed Lua variants (teal, luau, see Look into running SILE under Ravi / Luau / Pluto #725 and Consider typechecking libraries that can be disabled #871) both for our own upstream development and making it easier on 3rd party package developers to write correct code and test their addons.

The possibility of embedding precompiled bypecode versions of all our core distribution code while still letting people modify any port of the source in Lua is tantalizing. Before I draw any lines in the sand though I'd want an actual measurable PoC that showed what the advantages were going to be that we can weigh against the disadvantages. So far even when messing with this I haven't run into any major disadvantages to continuing to support multiple Lua versions. The existing Lua version detection and --with-luajit flag works to match the system Lua version to the one we're building already, which has some advantage for installing 3rd party libraries.

@ctrlcctrlv
Copy link
Member

I see, that makes sense. We can surely come back to it. It'd help with performance a lot, I'm sure, but as a fellow hacker I know that no PoC can defeat the draw of testing out Lua variants. Let me know what the results on those tests are then I'll try ;o)

@alerque alerque force-pushed the riir branch 4 times, most recently from cb5f818 to 7894109 Compare May 27, 2023 11:06
@alerque alerque force-pushed the riir branch 2 times, most recently from c213ec1 to 5c563ff Compare June 2, 2023 21:13
@alerque alerque mentioned this pull request Aug 19, 2023
4 tasks
@alerque alerque changed the base branch from master to develop September 12, 2023 08:25
@alerque alerque marked this pull request as ready for review September 12, 2023 08:25
@alerque
Copy link
Member Author

alerque commented Sep 12, 2023

Per #1864 I'm merging a few PRs to a develop branch following something like the gitflow model for a while so that the breaking changes and major refactoring can get a workout and multiple PRs pending the next major release can be aired out together.

I'll open separate issues for the bits of this PR considered incomplete.

@alerque alerque merged commit 6c8e041 into sile-typesetter:develop Sep 12, 2023
9 of 16 checks passed
@alerque alerque deleted the riir branch September 12, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Build tooling, release management, and packaging processes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants