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

Light C functions #512

Closed
petrihakkinen opened this issue May 30, 2022 · 11 comments
Closed

Light C functions #512

petrihakkinen opened this issue May 30, 2022 · 11 comments
Labels
enhancement New feature or request

Comments

@petrihakkinen
Copy link
Contributor

Luau currently does not support light C functions. A typical use case is returning stateless iterators from C code to Lua. Without light C functions, lua_pushcfunction creates a new closure which has garbage collection overhead. While it is possible to workaround the overhead by precreating the iterator function in registry, it is easily forgotten, especially when porting existing code from Lua to Luau. Also fetching the iterators from registry has some overhead. If possible, light C functions would be a great addition to Luau.

Light C functions are documented at the end of lua_pushcclosure in the Lua 5.4 manual:
https://www.lua.org/manual/5.4/manual.html#lua_pushcclosure

@petrihakkinen petrihakkinen added the enhancement New feature or request label May 30, 2022
@petrihakkinen
Copy link
Contributor Author

Not entirely sure if lack of light C functions is technically a compatibility issue or not, but I noticed the difference is not documented here: https://luau-lang.org/compatibility

@petrihakkinen
Copy link
Contributor Author

petrihakkinen commented May 31, 2022

I've been thinking how this could be implemented efficiently. Couple of ideas:

  1. Type variants

For example,

typedef struct lua_TValue
{
    Value value;
    int extra[LUA_EXTRA_SIZE];
    uint16_t tt;
    uint16_t tvariant; // e.g. 0 = Lua closure, 1 = C closure, 2 = light C function
} TValue;
  1. Alternatively add new primary type LUA_TLIGHTCFUNCTION
#define ttisfunction(o) (ttype(o) == LUA_TFUNCTION || ttype(o) == LUA_TLIGHTCFUNCTION)
#define ttislightcfunction(o) (ttype(o) == LUA_TLIGHTCFUNCTION)

btw. this sort of code in lvmexecute.cpp could be slightly faster, because 'isC' wouldn't be needed:

// fast-path: user data with C __index TM
const TValue* fn = 0;
if (ttisuserdata(rb) && (fn = fasttm(L, uvalue(rb)->metatable, TM_INDEX)) && ttisfunction(fn) && clvalue(fn)->isC)

->

// fast-path: user data with C __index TM
const TValue* fn = 0;
if (ttisuserdata(rb) && (fn = fasttm(L, uvalue(rb)->metatable, TM_INDEX)) && ttislightcfunction(fn))

(This would rule out C closures on the fast path but that sounds sensible to me.)

Downside is that lua_precall would need a branch / conditional move:
lua_CFunction f = ttislightfunction(func) ? (lua_CFunction)func->p : ccl->c.f;

That branch would be very coherent though (and therefore maybe better implemented using LUAU_LIKELY?).

Another downside is losing 'debugname' for light C functions which are used for error messages. (Maybe that could be implemented using bytecode backtracing though, which could make sense anyway for better error messages in general.)

Of course all places using clvalue() / ttisfunction() would need to be carefully inspected. I believe in many cases we could avoid adding new branches, because the code assumes it's working with Lua closures.

Anyway, quite a lot of work and not sure if it would be worth it considering the added complexity. Although it's appealing that GC wouldn't need to sweep the light C functions in the heap at all, which would be very common since most C functions do not have upvalues.

@zeux
Copy link
Collaborator

zeux commented May 31, 2022

This somewhat summarizes thoughts on the matter (this was considered at some point):

Anyway, quite a lot of work and not sure if it would be worth it considering the added complexity

To be more specific, the gain here is the ability to avoid some allocations, although it's more likely that for performance critical areas the extra allocations would be accidental. As you note, you can store the closure in the registry; another, sometimes simpler, trick is to store the closure as an upvalue of the outer function, which for example the builtins pairs/ipairs do.

The loss is that what constitutes a "C" function becomes harder to check and access, requiring divergent paths in the interpreter. Because light C functions don't have space for debug names (and bytecode tracing is more fragile and complex, we specifically avoided this in the interpreter even though PUC Lua uses it), a lot of commonly called functions won't be able to use a light function pointer, so the branches around the function type may not be very predictable, even if you ignore the expanding code size. This may lead to increase of complexity or decrease of performance on some paths inside the interpreter, and some of these become less "obvious" than the [hopefully rare and easily avoidable] cost of allocation that is incurred right now.

In PUC Lua the original motivation behind this change was to reduce the allocation count / heap cost of a new VM instance, since all the standard libraries can mostly stop allocating function objects. However, because of debugname, this wouldn't really happen in Luau, and light C functions would be mostly useful for "temporary"/"internal" functions that are less prevalent.

So overall the balance here doesn't seem to be in favor of this.

@zeux
Copy link
Collaborator

zeux commented May 31, 2022

Oh, btw, the reason why this isn't mentioned on compatibility page is twofold:

  • The compatibility page lists user-visible changes as far as the language/library is concerned, and this is purely internal;
  • The compatibility page was originally made from official Lua release notes, so this probably was never mentioned in whatever version these got added in.

@petrihakkinen
Copy link
Contributor Author

I agree that it's not worth it, unless majority of C functions could be light C functions. Only in that case branching would be coherent and the savings in the form of reduced GC pressure would be (potentially) more significant. We currently have over a thousand C functions exposed to Lua scripts (not counting the built-in Luau functions). Do you think it could make a difference for a full GC pass, if these functions would be light C functions?

It seems that the lack of debugname is a decisive factor here. If you don't mind me asking, is there anything else than bytecode tracing that we could do to improve error messages in general? Things like "attempt to index a nil value" and "attempt to call a nil value". The errors messages would be more useful if they were "attempt to index 'some_table' (a nil value)" and "attempt to call 'some_func' (a nil value)' etc. Mistakes happen, and some bugs can slip through even with type checking... Descriptive error messages, not only during programming, but especially in the form of detailed bug reports can be really useful tool.

@zeux
Copy link
Collaborator

zeux commented Jun 1, 2022

Do you think it could make a difference for a full GC pass, if these functions would be light C functions?

The key question is, how many C functions are there in relation to all other object types. I'd be surprised if there's a tangible difference in sizeable programs - the only case I can imagine where this becomes meaningful is "micro-VMs", eg applications that load each individual script into a tiny VM that still has a host interface exposed. These are somewhat fundamentally inefficient though for other reasons as well, for example even if functions were light, they still need to be present as table entries, which means string keys need to still be allocated.

In more concrete terms, a thousand C functions ~= 48 KB of heap space; we normally expect that a 10 MB heap is relatively light (with 1 MB being "small" and 100 MB being something that we optimize for), and even in a 1 MB heap this would be 5% of heap size.

On an unrelated note, when looking through the code to compute the memory size, it looks like C functions have their own environment table that can be set with lua_setfenv (thankfully it can't be used from user scripts). Unsure why this exists - it's a 5.1 artifact - and since we don't allow using it from user space we can probably remove it for C functions, but it's something to keep in mind. C functions can probably be reduced from 48 bytes to 32 bytes in general, it just hasn't been a focus because per above it's rare for them to be a sizeable portion of the heap.

we could do to improve error messages in general

There's certain cases where errors can be more precise. For example, #255. In general the default bytecode configuration does not embed local name information for size/memory efficiency reasons, so it's not really possible to emit errors with information that isn't present. While everything else being equal better error messages are, well, better, all of this is a delicate balance of complexity / performance / usability, so it's hard to say "X is worthwhile" definitively.

@petrihakkinen
Copy link
Contributor Author

the only case I can imagine where this becomes meaningful is "micro-VMs"

Yeah, this is actually an use case we have. Without going into too many details here, we are running certain, highly domain specific Lua scripts in a multithreaded environment. We have currently maybe 50 of these VM instances running in parallel, completely separated from each other. Luckily these scripts hardly allocate any memory at runtime (after initial setup) so GC cost should still be manageable.

That C function env optimization you mentioned could indeed be a nice optimization in our case, but I completely understand it would not be a big benefit for Roblox and should therefore have very low priority.

I'd suggest adding a brief mention to the compatibility page about light C functions. While it's technically not a breaking change, that page is a good checklist when transitioning from Lua/LuaJIT to Luau.

Many thanks for the detail answers and interesting discussion!

@petrihakkinen
Copy link
Contributor Author

I'm closing this issue now, since it's increasingly clear that light C function is not the way to go. I've made workarounds (a la pairs, thanks for the suggestion!) and the allocation overhead is now gone when creating the iterators.

@petrihakkinen petrihakkinen closed this as not planned Won't fix, can't repro, duplicate, stale Jun 1, 2022
@zeux
Copy link
Collaborator

zeux commented Jun 1, 2022

I'd suggest adding a brief mention to the compatibility page about light C functions. While it's technically not a breaking change, that page is a good checklist when transitioning from Lua/LuaJIT to Luau.

Note that LuaJIT doesn't support light C functions either; I think this is new as of Lua 5.2, partially enabled by the removal of function environments.

We can definitely take a look at reducing memory impact of C function objects; in general the issue you raise here is a more general "should we support more than one internal representation of a given type". Later versions of Lua went down the path of having many subtypes of basic types, e.g. there's multiple types of strings, functions, numbers. So far we've tried to preserve the single type to the extent possible, as it often helps reduce the branching in various places where we need to work with the objects, but there's sometimes changes we can make that don't fork the type and yet make it faster.

And yeah we can mention the lack of support for light C functions on compatibility page.

zeux added a commit that referenced this issue Jun 1, 2022
Add a note on light C functions that Lua 5.2 added; suggested by #512.
@petrihakkinen
Copy link
Contributor Author

btw. if you go down the road of type variants in the future, short strings could fit nicely into 'extra' space in tagged value :) I believe Mike Pall wrote a patch for PUC Lua a long time ago... And yeah, another complexity vs. efficiency balancing thing.

@petrihakkinen
Copy link
Contributor Author

Here, in case you've missed it: http://lua-users.org/wiki/FastStringPatch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants