Discussion:
[Dev-luatex] Preserving unknown keys in cached font table
Marcel Krüger
2018-09-25 19:33:58 UTC
Permalink
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
Hans Hagen
2018-09-25 21:26:10 UTC
Permalink
Post by Marcel Krüger
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
Post by Marcel Krüger
Best regards
Marcel
[bug]: https://github.com/u-fischer/luaotfload/issues/3
---
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) {
--
-----------------------------------------------------------------
Hans Hagen | PRAGMA ADE
Ridderstraat 27 | 8061 GH Hasselt | The Netherlands
tel: 038 477 53 69 | www.pragma-ade.nl | www.pragma-pod.nl
-----------------------------------------------------------------
Marcel Krüger
2018-09-25 22:30:34 UTC
Permalink
Post by Hans Hagen
Post by Marcel Krüger
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.
Post by Hans Hagen
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
Post by Hans Hagen
-----------------------------------------------------------------
Hans Hagen | PRAGMA ADE
Ridderstraat 27 | 8061 GH Hasselt | The Netherlands
tel: 038 477 53 69 | www.pragma-ade.nl | www.pragma-pod.nl
-----------------------------------------------------------------
Hans Hagen
2018-09-26 08:51:57 UTC
Permalink
Post by Marcel Krüger
Post by Hans Hagen
Post by Marcel Krüger
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
Post by Marcel Krüger
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)
Post by Marcel Krüger
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
Post by Marcel Krüger
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
Post by Marcel Krüger
Post by Hans Hagen
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
-----------------------------------------------------------------
Marcel Krüger
2018-09-26 10:07:58 UTC
Permalink
Post by Hans Hagen
Post by Marcel Krüger
[...]
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)
Post by Marcel Krüger
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.
Post by Hans Hagen
Post by Marcel Krüger
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.
Post by Hans Hagen
[...]
- 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.
Post by Hans Hagen
- 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.
Post by Hans Hagen
- 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.
Post by Hans Hagen
- 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
Post by Hans Hagen
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
-----------------------------------------------------------------
luigi scarso
2018-09-26 10:37:56 UTC
Permalink
Post by Marcel Krüger
Post by Hans Hagen
Post by Marcel Krüger
[...]
She proposed that the ConTeXt fontloader might benefit from
overloading
Post by Hans Hagen
Post by Marcel Krüger
`font.each()` similar to `font.getfont` if the engine should not be
changed.
Post by Hans Hagen
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
Hans Hagen
2018-09-26 12:22:35 UTC
Permalink
Post by Marcel Krüger
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.
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
-----------------------------------------------------------------
Marcel Krüger
2018-09-26 14:22:41 UTC
Permalink
Post by Hans Hagen
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

Loading...