Modular Nasal bootstrapping (again)

[Philosopher]

Continuing from viewtopic.php?f=30&t=19487, this isn't "fixed" but rather "dynamic" loading of modules.

I've been thinking that the best way to handle "modules" (loadable and unloadable bits of code that may have inter-module depencies) is to use the property tree. What this really allows for is requesting loading/unloading using the functionality of listeners. Loading is simply loading file(s) into a namespace, unloading potentially triggers listeners set during loading, and reloading - well, I have to decide on that one. There's a tree structure setup in property-tree://nasal/; /nasal/base-dir[0..n] are standard locations to search for Nasal files (currently $FG_ROOT/Nasal and $FG_HOME/Nasal, probably $FG_AIRCRAFT/Nasal, and maybe also $FG_NASAL and such). /nasal/base-dir has "file" and "dir" nodes which have "loaded", "load-requested", "unload-requested" nodes (bool), "path" (relative to parent), "absolute-path", and "namespace" nodes (string). dir can also contain dir and file nodes, recursively. Loading a dir involves loading all of its children; first files then directories (outside-in loading order, because I determined that makes most sense).

One thing this does is allow for circular dependencies that resolve really well. For instance, one can specify a list of symbols for the require() directive that must be present; if they aren't, the loading fails.

It also should allow for runlevels, early Nasal initialization (parsing command line arguments!! ), and C++ code can simply set a property to request "all of the rest of Nasal be loaded".

I have an initial sketch of a "nasal_bootstrap.nas" file (untested, I don't have much access to FG these days):

Code: Select all

# Runs the caller in the desired namespace; returns 1 if it had
# to be called again, 0 if it was already in the namespace.
#
# Defining the namespace:
#   Method 1: names
#     run_in_namespace can accept multiple scalars specifying
#     a path from the global namespace, e.g. "io" (globals.io)
#     or "canvas", "svg" (globals.canvas.svg). If the namespace
#     does not exist, it is created.
#  Method 2: hash
#     It can also accept a direct hash for the namespace.
#
# Common usage:
#   run_this_in_namespace(...) and return;
#
#   if (run_this_in_namespace) return;
var run_this_in_namespace = func(ns...) {
   if (size(ns) == 1 and typeof(ns[0]) == 'hash') {
      ns = ns[0];
   } else {
      var namespace = globals;
      foreach (var n; ns) {
         if (typeof(namespace) == 'hash' and !contains(namespace, n))
            namespace = (namespace[n] = {});
         else namespace = namespace[n];
      }
      ns = namespace;
   }
   var c = caller(1);
   if (c[0] == ns) return 0;
   else {
      call(c[1], c[0]["arg"], c[0]["me"], ns);
      return 1;
   }
}

# Load a sub-module or file if not already loaded.
# @param module_path A sufficiently resolvable path pointing
#                    to a *.nas file or a directory of *.nas
#                    files and/or subdirectories.
# @param symbols A list of symbols, as per _require_symbols(),
#                to require the module to have once loaded or,
#                in the case of a circular dependency, once
#                recursively required again. An empty list just
#                requires that the whole file is loaded before
#                continuing. Nil says "I don't care".
var require = func(module, symbols=nil) {
   resource = find_resource(module_path);
   if (resource.getValue("loaded")) {
      if (symbols == nil) return;
      var namespace = globals[var ns = resource.getValue("namespace")];
      var sym = _require_symbols(namespace, symbol);
      if (sym != nil)
         die(sprintf("symbolic dependency failed: symbol '%s' not"
                     " found in module '%s' once loaded", sym, ns));
      return;
   }
   if (resource.getValue("load-requested")) {
      if (symbols == nil) return;
      var namespace = globals[var ns = resource.getValue("namespace")];
      if (typeof(symbols) == 'scalar')
         symbols = [symbols];
      elsif (!size(symbols))
         die("circular dependency failed: resource wasn't loaded");
      var sym = _require_symbols(namespace, symbol);
      if (sym != nil)
         die(sprintf("circular dependency failed: symbol '%s' not"
                     " found in module '%s'", sym, ns));
      return;
   }
   resource.setValue("load-requested", 1);
}

# Private; for each symbol in symbols, return it if it doesn't
# exist. Each symbol can be a scalar (first-level symbol) or
# a list of scalars (multi-level symbol, e.g. props.Node.getValues).
var _require_symbols = func(namespace, symbols) {
   if (typeof(symbols) == 'scalar')
      symbols = [symbols];
   foreach (var sym; symbols) {
      if (typeof(sym == 'scalar')
         if (!contains(namespace, sym))
            return sym;
      else {
         var current = namespace;
         foreach (var subsym; sym)
            if (!contains(current, subsym))
               return sym~"."~subsym;
            else current = current[subsym];
      )
   }
   return nil;
}

var nasal_dirs = nil;
var make_resources = func() {
   nasal_dirs = [ getprop("/sim/fg-root") ~ "/Nasal",
                  getprop("/sim/fg-home") ~ "/Nasal"
                ] ~ props.globals.getNode("sim").getChildren("fg-nasal");
   var NasalNode = props.globals.getNode("nasal");
   #NasalNode.removeChildren();
   var load_listener = func(n) {
      if (!n.getValue()) return;
      var parent = n.getParent();
      if (parent.getValue("loaded"))
         parent.setValue("unload-requested", 1);
      var name = parent.getName();
      if (name == "file")
         io.load_nasal(parent.getValue("absolute-path"),
                       parent.getValue("namespace"));
      else {
         if (name == "nasal")
            # Load "standard" directories before extra files:
            var resources = parent.getChildren("base-dir")
                           ~parent.getChildren("file");
         else
            # Outside-in loading order otherwise:
            var resources = parent.getChildren("file")
                           ~parent.getChildren("dir");
         foreach (var resource; resources)
            resource.setValue("load-requested", 1);
      }
      parent.setValue("loaded", 1);
      n.setValue(0);
   }
   var unload_listener = func(n) {
      if (!n.getValue()) return;
      var parent = n.getParent();
      if (!parent.getValue("loaded"))
      { n.setValue(0); return }
      parent.setValue("loaded", 0);
      settimer(func n.setValue(0), 0); #next frame, after all listeners have run, FIXME
   }
   var get_abs_path = func(node) {
      node.getParent().getValue("absolute-path")
      ~node.getValue("path");
   }
   var get_module_name = func(node) {
      var current = node.getParent();
      while (current.getParent().getName() != "base-dir")
         current = current.getParent();
      return current.getValue("path");
   )
   var init_module_part = func(node, path) {
      node.setStringValue("path", path);
      node.initNode("loaded", 0, "BOOL");
      setlistener(
         node.initNode("load-requested", 0, "BOOL"),
         load_listener);
      setlistener(
         node.initNode("unload-requested", 0, "BOOL"),
         unload_listener);
      if (node.getName() != "base-dir" and node.getName() != "nasal")
         node.setStringValue("absolute-path", get_abs_path(node));
      if (node.getName() != "nasal")
         node.setStringValue("namespace", get_module_name(node));
      return node;
   }
   var make_basedir = func(path) {
      if (path[0] != `/`)
         die("paths must be absolute");
      path = string.normpath(path);
      var dir = init_module_path(NasalNode.addChild("base-dir"), path);
      var subpaths = directory(path);
      foreach (var subp; subpaths) {
         make_child(dir, subp);
      }
   }
   var make_child = func(node, path) {
      var subpaths = directory(node.getValue("path") ~ path);
      if (subpaths == nil) {
         if (split(".", path)[-1] == "nas")
            init_module_part(node.addChild("file"), path);
      } elsif (size(subpaths)) {
         var dir = init_module_part(node.addChild("dir"), path);
         foreach (var subp; subpaths) {
            make_child(dir, subp);
         }
      }
   }

   init_module_part(NasalNode);
   foreach (var abspath; nasal_dirs)
      make_base_dir(abspath);
}

# A few rules on top of ordinary resolving (see http://wiki.flightgear.org/
# Resolving_Paths). Tries prepending Nasal/ and appending .nas.
#
# It is an error in the filesystem to have both ${resolvable_path}
# and ${resolvable_path}.nas.
var resolvenasal = func(path) {
   var p = path;
   var try = resolvepath(path);
   if (try != "") return try;
   if (split("/", path)[0] != "Nasal") {
      path = "Nasal/"~path;
      var try = resolvepath(path);
      if (try != "") return try;
   }
   if (split(".", path)[-1] != "nas") {
      path = path~".nas";
      var try = resolvepath(path);
      return try;
   }
}

var find_resource = func(path) {
   var p = path;
   path = resolvenasal(path);
   if (path == "") die("nasal path could not be resolved: "~p);
   var NasalNode = props.globals.getNode("nasal");
   var startswith = func(a,b) substr(a,0,size(b)) == b;
   var _find_resource = func(node) {
      foreach (var resource; node.getChildren("dir")~node.getChildren("file")) {
         if ((var abs = resource.getValue("absolute-path")) == path) return resource;
         elsif (startswith(abs, path)) {
            var res = _find_resource(resource);
            if (res != nil) return res;
         }
      }
      nil;
   }
   foreach (var basedir; NasalNode.getChildren("base-dir"))
      if (startswith(basedir.getValue("path"), path)) {
         var res = _find_resource(basedir);
         if (res != nil) return res;
      }
   var res = _find_resource(NasalNode);
   if (res != nil) return res;
   printlog("info", "Creating new nasal resource for path "~p);
   var resource = NasalNode.addChild("file");
   resource.setStringValue("path", p);
   resource.setStringValue("absolute-path", path);
   resource.initNode("loaded", 0, "BOOL");
   setlistener(
      node.initNode("load-requested", 0, "BOOL"),
      func(n) {
         if (!n.getValue()) return;
         var parent = n.getParent();
         if (parent.getValue("loaded"))
            parent.setValue("unload-requested", 1);
         io.load_nasal(parent.getValue("absolute-path"),
                       parent.getValue("namespace"));
         parent.setValue("loaded", 1);
         n.setValue(0);
      });
   node.initNode("unload-requested", 0, "BOOL");
   node.setStringValue("namespace", split(split(path, "/")[0], ".")[0]);
   return resource;
}


[zakalawe]

I would suggest doing this via custom commands (implemented in Nasal, or better in C++ for security) instead of listeners inside /nasal. Command would take a module path, and have load/unload/reload parameters as needed, plus an optional pre-requisite list. With a global wrapper function around the fgcommand() invocation, this could look as nice as:

Code: Select all

import("modulename", depends = ["math", "canvas", ....], require = ["canvas.svg"])

Adding the list of loaded modules under /nasal/modules/ in the property tree is sensible, but using state to trigger dynamic changes (loading / unloading / initing) makes for more complex logic on all sides. This is exactly what I added support for implementing new commands in Nasal, so that going forward we can correctly put custom dynamic things in commands. (If only I'd thought to do this before creates the GPS commands-as-properties interface five years ago)

[Johan G]

If only I'd thought to do this before creates the GPS commands-as-properties interface five years ago

It is still not too late to re-implement them in some (distant?) future release.

[Hooray]

the first step would be re-implementing the current FGNasalSys::init logic (C++) in Nasal, so that ::init() becomes mostly empty and everything is delegated to a bootstrapping script - once that works without regressions, there are several examples to look at - Andy's original import() routine plus listeners sounds reasonable, but I can also see the merits of adding custom fgcommands implemented in Nasal itself, and making them non-mutable afterwards (for security) - I guess it all boils down to making some experiments first, and coming up with a list of desirable features.

[Philosopher]

require(), in this instance, is better than import(), and I've already proved its rationality in my earlier thread: very few modules needed any modification, and that was only a line at most. import() requires unecessary references to namespaces (one for each local namespace that needs it, when we already have a global namespace), and it necessitates doing it in every module that needs another module, versus assuming that a module is already loaded because we have a global namespace that provides a common forum for symbols. require() namely fits the criterion of least-cost because it conforms and adds to the current module handling.

One of my goals for require() is to allow dependencies with sub-modules; for example, that would be like loading gen/gen.nas before gen/gen.bytecode.nas, since the latter depends on the former, and this we would do with «require("gen/gen.nas")» at the top of gen.bytecode.nas. Previously there was virtually no way to acheive this.

James, my view is that the property tree makes it really easy to implement the data storage and module status values and it makes it exceptionally transparent. And it is actually quite simple: register a load listener that, if its a file, loads the file, or if its a directory, it triggers the loading of each file and sub-directory. Loading it uses the io.load_nasal helper (this will have to be copied at some point, I just wrote it as if it were available, along with some other dependencies). With fgcommands, my main concern is how do I store the information, and what I would gain from doing it. I like fgcommands in general, but for the transparency, just using the property tree works well. Do you have any reasons beyond these for using fgcommands? I can always change what I am doing .

[zakalawe]

Regarding require() vs import(), I think you are confused because I did not know there was already an import() in nasal. I simply picked the name I liked, I could have picked require! There's obviously no need to import things in the global or standard namespaces explicitly, that would be crazy.

For the commands vs properties, you said it yourself - you are storing state, and this to me seems un-necessary - can't it be stateless and simply evaluated when the .nas file is loaded? The Nasal subsystem knows what it has loaded already, and can unload things or test for presence of symbols easily enough, so I am unsure why you need additional state in the property tree.

[Philosopher]

Sorry, that was in response to Hooray. Andy's GitHub repo (and derived, i.e. nasal-standalone) has Nasal code for non-embedded uses. This includes an import() function which does... weird things which FG doesn't need. I wrote a lot to, well, express what you said in a sentence. Because... I dunno.

Anyways, the point of this project is to remove legacy C++ code that is better done in Nasal. Some ideas behind the effort are:

1. One of Hooray's more often-repeated points is that what the C++ code does should be done in Nasal. In particular, it will give an update, more flexibility and transparency, and allow for more features, like the rest of these points.
2. I have always believed that the listener method of waiting for dependencies to load is an awful hack; IMO we really need a more flexible approach (~cough~, reqiire()). The nasal-dir-init isn't terrible, but sub-module dependencies (e.g. my gen example above, one sub-module file depends on the other) are really difficult to resolve. With this I hope for more flexibility so we can do this properly.
3. Also, there are just some modules which should just be loaded by default, so other modules are guaranteed they are available, and it's better to do this on the Nasal side instead of C++ (better maintainability for regular Nasal users). E.g. io.nas (securrrity code), which—if we remove the _setlistener—depends on debug, string, and props (all good modules to have around). This will simplify module handling (most of the listeners are for the props module).
4. Another point that Hooray keeps talking about is run-levels and the fact that some modules, if not enabled, should not be loaded (it just wastes time, besides other reasons). Think about headless, or optional features, things like FGCanvas. Moving this to Nasal means the ability to support loading predicates in the future, and having the state in the property tree supports this effort, like if a new feature is enabled we might want to load a module, and when we can check the tree for that information it works well.
5. In short: modernization, simplification, better paradigm support.

I think the file I pasted above fits these well ATM. (I need to do more of developing, testing, etc.) @Hooray: this is replacing FGNasalSys::init().

[Hooray]

In addition, we do have recurring Nasal performance debates on the forums and the devel list - some folks, like ThorstenB for example, have highlighted how the number of symbols and active references does add up at some point -and ultimately- also affects total GC pressure - being able to clearly differentiate between declarative code and code that "runs" would help cleaning up the Nasal runtime environment, instead of having a more-or-less "monolithic" location with Nasal code that may (or may not) be required to run certain stuff - Nasal submodule support was a step in the right direction, but $FG_ROOT/Nasal still is a messy place - especially when it comes to untangling the various dependencies (e.g. to support a proper sim reset/re-init) - some of which are currently satisfied via timer/listener hacks, other handled through corresponding naming conventions, and some through explicit io.load_nasal() calls

So this is going to be remain a valid point until the GC is replaced or at least improved (aka made generational).

As previously mentioned, I once played around with a more basic startup mode for FG where not all subsystems would necessarily need to be available, and while initializing Nasal much earlier would be extremely handy to delegate certain stuff to scripting space (such as unmaintainined spaghetti code in fg_init.cxx or options.cxx), things are not all that straightforward because of our chaotic way of dumping all Nasal code into $FG_ROOT/Nasal and hoping for things to somehow work - without explicitly managing dependencies.

To be fair, the whole "runlevels" idea didn't come from me, IIRC, it dates back to a debate on the devel list long ago - but the recent developments, do prove that incremental re-initialization support would be a desirable feature, and consequently, managing scripting space dependencies would simply make sense, because otherwise all the existing Nasal-space mess would just add up to anything related to fixing reset/re-init and related efforts like FoA, headless/FGCanvas.

Growing/extending FGNasalSys::init beyond its current state would simply be counter-intuitive, and not really solve existing issues, but rather add to them sooner than later - because we'd be having some code that is unconditionally loaded/executed, while others (managed via scripting space) offer more fine-grained control and better accessibility (current submodules are already in many ways superior to stuff in $FG_ROOT/Nasal).

Frankly, Unix/Linux runlevels are also scripted (rc.*) and not managed in a hardcoded fashion - so we don't need to reinvent the wheel here. Having a single bootstrapper script that's called by ::init() would help clean up the C++ code quite a bit while making things much more accessible and maintainable in the long run.

But in its current form, it is much easier to clean up and manage code that is loaded from Nasal space vs. all the stuff that's simply dumped into the globals namespace via ::init(), which is why Philosopher's original thread had to use workarounds to "cripple" ::init().

All that being said, while I am not completely convinced that the property tree is needed for dependency resolution here, I can see the potential merits - especially because everything in the tree is transparently accessible to all other subsystems, so that things can be easily controlled via networking (telnet/http), too - in addition, ThorstenB's existing submodule implementation basically demonstrated that the approach itself does work pretty well.

Obviously, any listener-based approach to require would "require" (pun intended) the property tree as a dependency ...

[Philosopher]

Woohoo! Hooray! Initial work is completed, see my FlightGear & FGData "extended-nasal-bootstrap" branches (SimGear is "extended-nasal" branch, or you could revert the first commit of mine and use standard SimGear) – I apologize for the absolute mess they are, I'll see what I can do about that another day.

P.S. The wiki has syntax highlighting for Nasal now .

[Hooray]

Just skimmed over a handful of your sg/fg/fgdata commits, because I cannot currently test things here - but it looks pretty good to me. I'd just suggest to use #ifdefs instead of commenting stuff, so that testing is easier, i.e. can be done just by adding a #define during build time.

If this is something that should be added for 3.0, I'd suggest to discuss it with Zakalawe, so that it gets sufficient testing time - as in months and not just weeks, because it obviously touches many parts of the simulator, so people should feel confident that the changes work as expected.

As mentioned previously, I'd suggest to extend the bootstrapping script to load a separate directory with unit tests first, prior to loading $FG_ROOT/Nasal - something like Nasal.tests - that is then where we could test individual extension functions and APIs, and only really proceed with $FG_ROOT/Nasal if everything works as expected - that should also provide a safety margin for all the non-default aircraft, outside fgdata.

PS: I took the liberty to add your wiki contribution to the upcoming newsletter
PPS: After looking at your nasal-bootstrap.nas script: where do I have to sign for some advanced Nasal training ?

[zakalawe]

Philosopher, if there are patches you want me to take a look at, especially C++ changes, please submit a merge request and send me an email. You should expect a couple of review iterations for any C++ changes in this area, just to warn you. If they are 'an absolute mess', you need to clean them up to before asking me to look at them or it will be a really boring initial review cycle

Equally, I am happy to do a design review of work-in-progress - there's no point polishing comments / variable names / correct use of const-references is the entire methodology is problematic). For a design review, I can look patches or pull branches, again send me an email saying what/where to look.

[Philosopher]

Hi H, Zakalawe,

As it turns out, the C++ changes are fairly minor. I didn't mess with much currently, but simply by bootstrapping early and triggering different properties, one could configure it to do different things (run levels, initialize some things early, script out command line options, or such). I would prefer to leave these changes to you - you're experienced with the whole FlightGear code base, and unless I gain lots of knowledge, I wouldn't know how to change things. I will take care of Nasal-side changes, if you want. I already have sort of a DSL on paper for run-levels and (approximate) loading order.

Basically my view is that the bootstrap script sets up various signals (currently we have only one) and the C++ can trigger them when it wants to load something. Load predicates on the Nasal side will provide configuration, especially in the case of optional features and run levels. Does this sound good?

[Hooray]

The C++ changes are indeed trivial at the moment - they may be easier to review by using #ifdefs, but otherwise the real stuff is Philosopher's Nasal code, which is fairly sophisticated
Also, coding-wise there's isn't anything really "ugly" per se, especially not in comparison to most fgdata/Nasal code - I think Philosopher was referring to his commits/repo here ?

What I've seen so far is exceptionally clean and lean, and actually maps pretty well to booting up FlightGear in a more restricted mode eventually, where our plethora of existing Nasal code would literally be a huge mess to clean up - frankly, I am thankful that Philosopher is looking into this, because this clearly is not only challenging and requires lots of Nasal internals knowledge, but anybody working on this is also risking their sanity here D

script out command line options, or such

at the moment, we cannot initialize Nasal much earlier due to some hardcoded "PostInit*" stuff in FGNasalSys - so I wouldn't worry too much about it for the time being.

When I scripted a few telnet commands last year, I merely used the globals-> pointer to acquire a handle to the interpreter and passed subsystem specific info to the interpreter as a naHash- and that's what most subsystems will typically do.

We did have some feature requests to add scripting capabilities to various SGSubsystems eventually, notably the generic protocol, the telnet system and the http code.

But none of that is important at the moment, and even the telnet/scripting code that I started predates Tom's cppbind work, which should make most of these things much more straightforward, as can be seen in Tom's NasalPositioned work. Basically, it would be better to start from scratch and directly use cppbind for such things

Being able to do CLI processing in scripting space would seem nice, because 1) it's clearly not performance-critical, 2) the corresponding C++ code is huge and not very actively maintained, 3) adding new options would become much easier, and could be done by fgdata contributors.

Regarding hardcoded postInit* workarounds, I think Tom mentioned a while ago that he's getting along without using any of those - my own suggestion would be to use listeners to start/suspend subsystems accordingly, matching Zakalawe's subsystem-reinitialization fgcommand, so that these things are no longer hardcoded, which should align well with the reset/re-init plans - as long as subsystems can be requested/started and shut down via property signals, it shouldn't matter to FlightGear if it's resetting, re-initializing or just booting.

[zakalawe]

Basically my view is that the bootstrap script sets up various signals (currently we have only one) and the C++ can trigger them when it wants to load something. Load predicates on the Nasal side will provide configuration, especially in the case of optional features and run levels.

It sounds fine, but I would be much happier seeing a proposed diff to get any idea of when things need to happen, so I can guess what might break

To explain, I would prefer to init Nasal earlier in startup, because I would like use Canvas-based UI for pieces such as aircraft-selection - but right now we have to load Nasal in postInit, which is seconds and seconds into startup. Most Nasal things that need FDM state already wait on fdm-initialized, so in theory the only fix required is to skip <nasal> evaluation in 'animation' XML files (apparently there is a long-standing assumption that such elements are skipped when loading an aircraft as the main one, i.e they are only run for AI / MP aircraft - this is a bad situation but changing it now could be a serious compatibility problem)

However, I suspect moving it will not be so simple anyway, because of the number of assumptions in Nasal scripts that everything (especially, the property tree) is already bound and inited. If you have solutions to this, fantastic.

Anyway, to re-iterate, I am delighted to have this cleaned up, and a way to incrementally init bits of Nasal, but I would be much happier to see a patch of the C++ difs, the sooner the better, just in case you're doing something that is not going to work safely.

[Hooray]

I would be much happier seeing a proposed diff to get any idea of when things need to happen
I am delighted to have this cleaned up, and a way to incrementally init bits of Nasal, but I would be much happier to see a patch of the C++ difs, the sooner the better, just in case you're doing something that is not going to work safely.

@Zakalawe: Just check out Philosopher's gitorious account, and you'll see diffs, for all three repos: sg/fg/fgdata

https://gitorious.org/fg/philosophers-f ... 7c7?page=1