Support for multiple global records in `TypeWalker`

[makspll]

Correct me if I am wrong, but it looks like as of now the only way to generate multiple global records like these:

global record MyPointType
   x: number
   y: number
end

global record MyCompositeType
   center: MyPointType
end

is to manually merge the output of multiple generate_global calls in TypeWalker, since the generate call wraps everything under a single global record.

TypeWalker could perhaps use a TypeRecord structure which essentially encapsulates the current TypeWalker behaviour, but storing the global/local type of each record. TypeWalker could then have a generate call which does not need the is_global or outer_name arguments.

If mixing global and local records is not allowed by teal, then perhaps TypeWalker could be made into an Enum, with local and global variants.

[lenscas]

you probably don't want multiple global records and instead want 1 global one that contains the local ones.

tealr uses the top global one kind of as a namespace to prevent polluting the global namespace. Especially important as you most likely want to do a global autoload when embedding lua/teal.

I do want to eventually allow people to make records that contain other records, not being able to do this is one of the reasons why the FromToLua macro doesn't support structure variants in enums. However, I'm not sure how to best display this in the sidebar so everything stays findable yet organized.

[makspll]

Oh so there's currently no way to document modules containing userdata etc ?

how about global functions like this one?:

        ctx.globals().set("print",ctx.create_function(|_ctx, msg: String| {
            info!("{}", msg);
            Ok(())
        })?)?;

[lenscas]

hmm... global functions are indeed not really documentable.... guess that that should go to the backlog.

Oh so there's currently no way to document modules containing userdata etc ?

what exactly do you mean? When creating a userdata there should be a document_type method while adding the methods/fields

[makspll]

what exactly do you mean? When creating a userdata there should be a document_type method while adding the methods/fields

Essentially things like:

let my_module = ctx.create_table()?;
my_module.set("my_userdata",...);
my_mosule.set("my_function",...);

ctx.globals().set("my_module",my_module);

[lenscas]

what I would do is make a new struct that has the methods/fields that you want and share that with lua. Then you can use the "process_inline" method on the type walker to get the result that you want. That is how I did https://lenscas.github.io/tealsql/

https://github.com/lenscas/tealsql/blob/a1673fe9584eb6dec1d19d3bb3e42b6423a2a725/pgteal/src/lib.rs#L24

https://github.com/lenscas/tealsql/blob/a1673fe9584eb6dec1d19d3bb3e42b6423a2a725/pgteal/src/base.rs#L49

[lenscas]

Actually, I think I have a better solution. Will do some experiments after I am done with dinner.

[lenscas]

I made #49 which allows you to more easily expose instances as a global and have it documented. Right now the api only allows it to expose values, documenting them will come later. Just wanted to get this set up so you could use it :)

[makspll]

Fantastic work! I believe this should be all functionality necessary to build my documentation!

I might leave one last issue but that's performance related 😂

[lenscas]

I believe this should be all functionality necessary to build my documentation!

I would hope so, and if not then just leave a new issue :)

I might leave one last issue but that's performance related 😂

Oh? As far as I know, Mlua (and presumably rlua) only get the methods/fields that a UserData type has once. So, I don't expect the overhead that tealr introduces there to be that bad? Am I wrong in that assumption?

When generating the files/documentation there are probably a few places that could be optimized better, but I figured that it wouldn't be that bad as it only needs to run a couple of times per release at most.

So... yea.. though I knew the code wasn't the most optimized I am still quite surprised if you run into actual performance issues....

[makspll]

Oh? As far as I know, Mlua (and presumably rlua) only get the methods/fields that a UserData type has once. So, I don't expect the overhead that tealr introduces there to be that bad? Am I wrong in that assumption?

I believe that add_methods is called via: UserData::to_lua Lua::create_userdata Lua::make_userdata Lua::push_userdata_meta

which all happen every time you create a UserData, so this would mean that all documentation methods run every time then as well (sorry if this is what you meant).

This might not be bad, however for Copy things like bevy::Vec3 which might be created a lot via scripts there could be noticeable slow downs, especially since a lot of the documentation machinery is based on hashmaps, string manipulation etc.

One thing I wanted to suggest is using optional compilation to turn all documentation machinery into no-ops when it's not being generated. Ideally something based around an env variable (say DOC_GEN) or something not dependent on the build profile. The compiler could then completely optimise those away when documentation is not being built.

An ideal option to completely remove runtime costs and shift to static documentation generation. This would require separating documentation (data) from the native types completely (logic). The construction of docs could be abstracted behind macros which operate over markers and docstrings, so something like this:

mlua_doc_generator!{
/// Type level documentation
pub struct MyType {}

impl UserData for MyType {
    fn add_methods<'lua, M: UserDataMethods<'lua, Self>>(methods: &mut M) {

        /// Documentation to be sent to lua for `add`
        methods.add_method_mut("add", |_, this, value: i32| {
            this.0 += value;
            Ok(())
        });

        /// Some documentation for Meta Add
        methods.add_meta_method(MetaMethod::Add, |_, this, value: i32| {
            Ok(this.0 + value)
        });
    }
}
};

Could then add code like this:

impl DocGen{
        fn get_type_level_doc() {"Type level documentation"};
        fn get_methods() -> Vec<Methods>;
        ...
}

which could be entirely static! and the TypeWalker could use this single trait to build the types, any additional options needed to resolve a type could be added via meta/macro attributes. Globals/modules/functions would likely need special treatment, but could be dealt with by allowing things like:

TypeWalker::new()
    .process_type::<MyType>
    .global_fn::<Fn(String,int) -> String>("my_function","Documentation for the function")
    .module("my_module", TypeWalker::new().global_fn::<Fn>(...)) 

In general I think separating the data and the logic parts would be useful, like with globals in example above here.

Not sure how feasible this would be and it would require a lot of re-writing :rofl: . An added bonus would be this would not require wrapping the rlua/mlua libraries, and could reduce code duplication.

Just my 2 cents!

[lenscas]

which all happen every time you create a UserData, so this would mean that all documentation methods run every time then as well (sorry if this is what you meant).

hmm... when talking about the guy who worked on hv_lua (which is a fork of mlua) it sounded like Mlua only goes over the AddMethods/AddFields methods once per type and from that point on caches the result for said type. I was actually thinking of adding a way to disable the doc gen unti this was mentioned, so a bit of manual digging is required to find out what is actually the case.

I'm also not a fan to having users write code while in a macro as autocompletion tends to be rather poor at best of times. I also like to force people to use the same api to register things to lua as they use to register things to be documented. That way the amount of places that need to be edited stays low and thus the amount of mistakes should stay small.

Current way to register globals should already be fairly cheap, though I haven't looked if llvm properly inlines everything to make it 0 cost. Documentation methods on this will be NOOP when sharing with Lua, as there is not much that they can do (there is no .help() method that can be generated there). So, that part at least shouldn't suffer. Which is at least a plus :)

[makspll]

hmm... when talking about the guy who worked on hv_lua (which is a fork of mlua) it sounded like Mlua only goes over the AddMethods/AddFields methods once per type and from that point on caches the result for said type. I was actually thinking of adding a way to disable the doc gen unti this was mentioned, so a bit of manual digging is required to find out what is actually the case.

Not sure whre the caching would take place, as on the rust side it seems everything gets called on every instantiation, I've benchmarked this for rlua before and the cost of creating a new UserData with a medium amount of methods seemed pretty constant at around 300ns

I've left a question in mlua in case I am wrong: https://github.com/khvzak/mlua/issues/175

I'm also not a fan to having users write code while in a macro as autocompletion tends to be rather poor at best of times. I also like to force people to use the same api to register things to lua as they use to register things to be documented. That way the amount of places that need to be edited stays low and thus the amount of mistakes should stay small.

Fair enough, that's some good points against macros, I think the current way works perfectly fine but the performance definitely needs to be considered for the case where UserData are created a lot.

[lenscas]

Fair enough, that's some good points against macros, I think the current way works perfectly fine but the performance definitely needs to be considered for the case where UserData are created a lot.

yea, if mlua doesn't cache it then I should need to worry A LOT more about it.

hmm... maybe I can add a cache myself if it isn't done by mlua?

Also, a simple way to check it is to add a println!() statement in the add_methods part and then share a few objects of this type. See how often it prints stuff. If it prints only once then it is cached :)

[makspll]

I was wrong. mlua does indeed cache userdata!

However I think removing runtime costs is still worth it since a lot of applications might use "one-shot" scripts where this cost would appear every time a new user data is used in each new script. An example might be UI handler sort of scripts:

<UIElem, on_click="<Lua code>">
<UIElem>

So I'd still think no-opping everything is a worthwhile effort!

[lenscas]

If someone constantly runs a new luavm for every script then I think they have bigger performance problems than me adding the documentation. There also is already a workaround for them, as they can just make their own UserDataMethods wrapper that replaces the documentation adding methods with NOOPs.

[lenscas]

So.... I believe that #49 added support for what you actually needed in this issue? If so, feel free to close it.

The performance concerns can be moved to their own issue if it is really important for you, but I'm not sure if I will tackle it as there are already ways for users to deal with it by making their own wrappers.

[makspll]

Yes #49 does exactly what I wanted!

The performance issue is definitely much less of a problem in my eyes after finding out about the caching, I will do some benchmarks and if it turns out to be a problem I'll open an issue.