Preserving unknown keys in cached font table
Hi, while looking into a [luaotfload bug][bug] caused by the fontloader being confused when LuaTeX overwrites the `parameters` in a cached font table, I wondered why `write_lua_parameters` always creates a new table instead of altering a potentially preexisting table. Changing this should lead to some small performance improvement because Lua no longer has to create a new table every time a cached font table is queried and more importantly it allows additional string keys in `parameters` to be preserved. Best regards Marcel [bug]: https://github.com/u-fischer/luaotfload/issues/3 A patch implementing the suggested change: --- source/texk/web2c/luatexdir/font/luafont.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source/texk/web2c/luatexdir/font/luafont.c b/source/texk/web2c/luatexdir/font/luafont.c index e64aaffc1..68e74ffc5 100644 --- a/source/texk/web2c/luatexdir/font/luafont.c +++ b/source/texk/web2c/luatexdir/font/luafont.c @@ -349,7 +349,11 @@ static void write_lua_parameters(lua_State * L, int f) { int k; lua_push_string_by_name(L,parameters); - lua_newtable(L); + lua_pushvalue(L, -1); + if (LUA_TNIL == lua_rawget(L, -3)) { + lua_pop(L, 1); + lua_newtable(L); + } for (k = 1; k <= font_params(f); k++) { switch (k) { case slant_code: -- 2.19.0
On 9/25/2018 9:33 PM, Marcel Krüger wrote:
Hi,
while looking into a [luaotfload bug][bug] caused by the fontloader being confused when LuaTeX overwrites the `parameters` in a cached font table, I wondered why `write_lua_parameters` always creates a new table instead of altering a potentially preexisting table.
Changing this should lead to some small performance improvement because Lua no longer has to create a new table every time a cached font table is queried and more importantly it allows additional string keys in `parameters` to be preserved.
sounds more like an issue of luaotfload ... it should manage the parameters well we're not going to change the current behaviour in the engine which is creating these tables (same for math constant etc) from mem (changing such things might solve your specific issue but introduce issues for others) Hans
Best regards Marcel
[bug]: https://github.com/u-fischer/luaotfload/issues/3
A patch implementing the suggested change:
--- source/texk/web2c/luatexdir/font/luafont.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/source/texk/web2c/luatexdir/font/luafont.c b/source/texk/web2c/luatexdir/font/luafont.c index e64aaffc1..68e74ffc5 100644 --- a/source/texk/web2c/luatexdir/font/luafont.c +++ b/source/texk/web2c/luatexdir/font/luafont.c @@ -349,7 +349,11 @@ static void write_lua_parameters(lua_State * L, int f) { int k; lua_push_string_by_name(L,parameters); - lua_newtable(L); + lua_pushvalue(L, -1); + if (LUA_TNIL == lua_rawget(L, -3)) { + lua_pop(L, 1); + lua_newtable(L); + } for (k = 1; k <= font_params(f); k++) { switch (k) { case slant_code:
-- ----------------------------------------------------------------- Hans Hagen | PRAGMA ADE Ridderstraat 27 | 8061 GH Hasselt | The Netherlands tel: 038 477 53 69 | www.pragma-ade.nl | www.pragma-pod.nl -----------------------------------------------------------------
---- On Tue, 25 Sep 2018 23:26:10 +0200 Hans Hagen
On 9/25/2018 9:33 PM, Marcel Krüger wrote:
Hi,
while looking into a [luaotfload bug][bug] caused by the fontloader being confused when LuaTeX overwrites the `parameters` in a cached font table, I wondered why `write_lua_parameters` always creates a new table instead of altering a potentially preexisting table.
Changing this should lead to some small performance improvement because Lua no longer has to create a new table every time a cached font table is queried and more importantly it allows additional string keys in `parameters` to be preserved.
sounds more like an issue of luaotfload ... it should manage the parameters well
I do not think luaotfload is responsible especially because Ulrike Fischer got a similar problem on ConTeXt with \starttext foo \ctxlua{for ii,vv in font.each() do end } bar \stoptext She proposed that the ConTeXt fontloader might benefit from overloading `font.each()` similar to `font.getfont` if the engine should not be changed. In some personal experiments local function fonteach_next(max, f) repeat f = f + 1 if f > max then return end until font.frozen(f) ~= nil return f, font.getfont(f) or font.fonts[f] end font.each = function() return fonteach_next, font.max(), 0 end seemed to fix the problem.
we're not going to change the current behaviour in the engine which is creating these tables (same for math constant etc) from mem (changing such things might solve your specific issue but introduce issues for others)
While I agree that keeping compatibility is important I do not really understand how this would lead to issues. Especially the parameters known to LuaTeX are still updated, so the only change I can see here would be if someone relies on LuaTeX dropping some table values... Is there a problem I missed? Best regards Marcel
----------------------------------------------------------------- Hans Hagen | PRAGMA ADE Ridderstraat 27 | 8061 GH Hasselt | The Netherlands tel: 038 477 53 69 | www.pragma-ade.nl | www.pragma-pod.nl -----------------------------------------------------------------
On 9/26/2018 12:30 AM, Marcel Krüger wrote: > ---- On Tue, 25 Sep 2018 23:26:10 +0200 Hans Hagenwrote ---- > > On 9/25/2018 9:33 PM, Marcel Krüger wrote: > > > Hi, > > > > > > while looking into a [luaotfload bug][bug] caused by the fontloader > > > being confused when LuaTeX overwrites the `parameters` in a > > > cached font table, I wondered why `write_lua_parameters` always > > > creates a new table instead of altering a potentially preexisting table. > > > > > > Changing this should lead to some small performance improvement > > > because Lua no longer has to create a new table every time a cached > > > font table is queried and more importantly it allows additional string > > > keys in `parameters` to be preserved. > > > > sounds more like an issue of luaotfload ... it should manage the > > parameters well > > I do not think luaotfload is responsible especially because Ulrike Fischer > got a similar problem on ConTeXt with > > \starttext > foo > \ctxlua{for ii,vv in font.each() do end } > bar > \stoptext hm, we never use font.each in context > She proposed that the ConTeXt fontloader might benefit from overloading > `font.each()` similar to `font.getfont` if the engine should not be changed. she hasn't reported an issue on the context list (yet) > In some personal experiments > > local function fonteach_next(max, f) > repeat > f = f + 1 > if f > max then return end > until font.frozen(f) ~= nil usage and therefore freezing can happen out of order so you can miss some > return f, font.getfont(f) or font.fonts[f] > end > font.each = function() return fonteach_next, font.max(), 0 end > > seemed to fix the problem. i assume that you just want to run over fonts ... in that case i can overload font.each in the context font loader (but different) in a more efficient way than querying tex > > we're not going to change the current behaviour in the engine which is > > creating these tables (same for math constant etc) from mem (changing > > such things might solve your specific issue but introduce issues for > > others) > > While I agree that keeping compatibility is important I do not really understand > how this would lead to issues. > > Especially the parameters known to LuaTeX are still updated, so the only change > I can see here would be if someone relies on LuaTeX dropping some table values... > > Is there a problem I missed? - why only parameters and not mathconstants, every character (which can have subtables), and other properties - why overload set values at the lua end with values at the tex end (they can differe on purpose) - better would be to set a metatable but that would add an extra lookup chain every time one does font.each - do you really want to use font.each and overwrite parameters with maybe values different from what is expected at the other end (can have fuzzy side effects) - you probably loose way more than this one variable with font.each - basically what font.each does is give you back what tex sees (which can be way less than what lives at the lua end) - so you solve one of your issues, then later another pops up (because some other assumed value is not coming from the tex end, and within a few years we have chaos in intercepts and heuristics - the example of 'factor' not being in the tex parameters table is an example of where usage can differ from stock tex usage etc Hans ----------------------------------------------------------------- Hans Hagen | PRAGMA ADE Ridderstraat 27 | 8061 GH Hasselt | The Netherlands tel: 038 477 53 69 | www.pragma-ade.nl | www.pragma-pod.nl -----------------------------------------------------------------
---- On Wed, 26 Sep 2018 10:51:57 +0200 Hans Hagen
On 9/26/2018 12:30 AM, Marcel Krüger wrote:
[...] She proposed that the ConTeXt fontloader might benefit from overloading `font.each()` similar to `font.getfont` if the engine should not be changed.
she hasn't reported an issue on the context list (yet)
In some personal experiments
local function fonteach_next(max, f) repeat f = f + 1 if f > max then return end until font.frozen(f) ~= nil
usage and therefore freezing can happen out of order so you can miss some
RIght, but it is only comparing to nil so it shouldn't check if the font is frozen, it is just used as a way to check if the font is valid.
return f, font.getfont(f) or font.fonts[f] end font.each = function() return fonteach_next, font.max(), 0 end
seemed to fix the problem.
i assume that you just want to run over fonts ... in that case i can overload font.each in the context font loader (but different) in a more efficient way than querying tex
That would certainly fix the original problem but I still think that it would be nicer to change the engine here. Keeping almost everything in cached font tables except for the parameters table seems counterintuitive.
[...]
- why only parameters and not mathconstants, every character (which can have subtables), and other properties
- you probably loose way more than this one variable with font.each
- basically what font.each does is give you back what tex sees (which can be way less than what lives at the lua end)
- so you solve one of your issues, then later another pops up (because some other assumed value is not coming from the tex end, and within a few years we have chaos in intercepts and heuristics
This is only about cached font tables. The code handling them is `font_to_lua` is if (font_cache_id(f) > 0) { /*tex Fetch the table from the registry if it was saved there by |font_from_lua|. */ lua_rawgeti(L, LUA_REGISTRYINDEX, font_cache_id(f)); /*tex Font dimenensions can be changed from \TEX\ code. */ write_lua_parameters(L, f); return 1; } So AFAICT the table is not modified (and no entries are lost) except by `write_lua_parameters`. So I only want to change the handling of `parameters` because no values are lost in the other fields.
- why overload set values at the lua end with values at the tex end (they can differe on purpose)
- do you really want to use font.each and overwrite parameters with maybe values different from what is expected at the other end (can have fuzzy side effects)
I am not sure what you mean here. But I think it makes sense to reflect changes in the TeX font parameters in the Lua table. I would think that everyone who uses the font parameters table expects the values to conform to the font dimensions in TeX.
- better would be to set a metatable but that would add an extra lookup chain every time one does font.each
I think an extra lookup would be a small price to pay if the alternative is that font.each is effectivly useless.
- the example of 'factor' not being in the tex parameters table is an example of where usage can differ from stock tex usage
I think this is more an argument to not change the parameters table at all. Especially *because* the usage of the table can differ from stock TeX usage it makes sense not to drop unknown entries. Best regards Marcel
etc
Hans
----------------------------------------------------------------- Hans Hagen | PRAGMA ADE Ridderstraat 27 | 8061 GH Hasselt | The Netherlands tel: 038 477 53 69 | www.pragma-ade.nl | www.pragma-pod.nl -----------------------------------------------------------------
On Wed, Sep 26, 2018 at 12:08 PM Marcel Krüger
---- On Wed, 26 Sep 2018 10:51:57 +0200 Hans Hagen
wrote ---- On 9/26/2018 12:30 AM, Marcel Krüger wrote:
[...] She proposed that the ConTeXt fontloader might benefit from overloading `font.each()` similar to `font.getfont` if the engine should not be changed.
she hasn't reported an issue on the context list (yet)
for the record: %%% test.tex Test: \directlua{ local function recdump(k,v) if type(v)=='table' then print(" >>",k) for k1, v1 in pairs(v) do recdump(k1,v1) end else print(" ",k,v) end end for ii,vv in font.each() do recdump(ii,vv) end} \bye mtxrun --script plain test.tex is ok (latest context beta) . -- luigi
That would certainly fix the original problem but I still think that it would be nicer to change the engine here. Keeping almost everything in cached font tables except for the parameters table seems counterintuitive.
On 9/26/2018 12:07 PM, Marcel Krüger wrote: the problem is this: when you pass a font to tex it can save the table, in which case you can get it back as-is however, when you ask for a table the parameters will be rewritten now, it is indeed arguable, but then it's better *not* to update the parameters at all because after all, other values are also passed as they are (and they can be different too from the known values inside tex, e.g. rounded dimensions) ... so, i'll just remove the whole update which then also means that you don't get the updated parameters ... it's probably not a problem as no sane user will fetch a table for just that so, "each" and "getfont" will not update parameters any longer when a cached table is used and when one wants to know the ones really used in the engine "getparameters" is to be used; of course that table only has the few official ones, no additional keys known at the lua end (but someone using getfont or getparameters is not interested in those anyway) Hans ----------------------------------------------------------------- Hans Hagen | PRAGMA ADE Ridderstraat 27 | 8061 GH Hasselt | The Netherlands tel: 038 477 53 69 | www.pragma-ade.nl | www.pragma-pod.nl -----------------------------------------------------------------
---- On Wed, 26 Sep 2018 14:22:35 +0200 Hans Hagen
the problem is this:
when you pass a font to tex it can save the table, in which case you can get it back as-is
however, when you ask for a table the parameters will be rewritten
now, it is indeed arguable, but then it's better *not* to update the parameters at all because after all, other values are also passed as they are (and they can be different too from the known values inside tex, e.g. rounded dimensions) ... so, i'll just remove the whole update which then also means that you don't get the updated parameters ... it's probably not a problem as no sane user will fetch a table for just that
Great, that might be the best solution. Best regards Marcel
participants (3)
-
Hans Hagen
-
luigi scarso
-
Marcel Krüger