Howto:Vectorizing the Nasal setprop() API

From FlightGear wiki
Jump to navigation Jump to search
Property Tree
This article is a stub. You can help the wiki by expanding it.

Objective

Making f_setprop()[1] faster by allowing a Nasal vector to be passed in as the 2nd argument which automatically "maps" the vector to the property path specified using the vector's index as the property index without having to use a loop to switch between Nasal/C.

Motivation

Under some circumstances, property access via props.nas may be too slow, so that setprop() may be preferable. Additional performance gains can be accomplished by reducing the overhead from switching between C/C++ and the Nasal engine (VM).

This means that repeated function calls operating on a vector (think in a for/forindex/foreach loop) can be expressed using a single function call that operates on a whole vector of data, analogous to map/reduce in functional programming.

Background

Let's consider the following snippet of code [2]: 

    var path = my_graph._node.getPath();

    for (var i = 0; i< size(cmd); i=i+1)
    {
    index1 = 2 * i;
    index2 = index1+1;

    setprop(path~"/cmd["~i~"]", cmd[i]);
    setprop(path~"/coord["~index1~"]", coord[index1]);
    setprop(path~"/coord["~index2~"]", coord[index2]);
    }

Or this version, which is getting away without using string concatenation in the loop body [3]: 

var cmd_string = path~"/cmd";
var coord_string = path~"/coord";

for (var i=0; i< n_max; i=i+1)
	{
	index = 2.0 * i;
	index1 = index + 1;
	if (i < n_cur)
		{
		setprop(cmd_string,i, cmd[i]);
		setprop(coord_string, index, draw[index]);
		setprop(coord_string, index1, draw[index1]);
		}
	else
		{
		setprop(cmd_string,i, canvas.Path.VG_MOVE_TO);
		setprop(coord_string, index, 0);
		setprop(coord_string, index1, 0);
		}

	}

group.max_pts = n_cur;
}


This contains a single for-loop that ends up calling one Nasal extension function (setprop) 3 times per loop iteration. However, the overhead from switching between C and Nasal once per call can obviously add up, depending on the size of the cmd vector.

Thus, we could greatly reduce this overhead by coming up with a modified setprop() equivalent that directly operates on a property path and a Nasal vector accordingly, without switching between Nasal/C once per vector element: 

    var path = my_graph._node.getPath();
    # cmd being a Nasal vector, the for-loop can be executed in native C code instead of requiring context switches: 
    setprop(path~"/cmd", cmd);

Understanding setprop()

One quick workaround would be providing a setprop() equivalent that directly works using Nasal vectors (arrays), i.e. which directly operate on vectors (think GLSL-like): setprop_vector("/foo/x", vector);

This would be the equivalent of the Nasal code shown above, just in C/C++ space - i.e. it would be a subset of the f_setprop() API:

However, it's pretty much copy & paste actually - i.e. a new f_setpropv() function and adapting it to directly get the size of the passed vector and use that in a for-loop to do indexed property updates directly - which should translate into N fewer C<->Nasal context switches, because it'd be all done in native C++ code at that point. We once did that to help speed up diff'ing two Nasal vectors in a fast way, and it caused a massive speed-up - I think it was James or Stuart who helped get that commited back then. [4]

WIP.png Work in progress
This article or section will be worked on in the upcoming hours or days.
See history for the latest developments. 
// setprop() extension function.  Concatenates its string arguments as
// property names and sets the value of the specified property to the
// final argument.
static naRef f_setprop(naContext c, naRef me, int argc, naRef* args)
{
    // this checks if we have two arguments, and bails out if we don't
    if (argc < 2) {
        naRuntimeError(c, "setprop() expects at least 2 arguments");
    }

    // next, this will get the 2nd argument and store it as val
    naRef val = args[argc - 1];

    // this will look up the property matching the property path specified and store it as p
    // TODO: this can be further simplified for the vector use case
    SGPropertyNode* p = findnode(c, args, argc-1, true);

    bool result = false;
    try {
        // if the value is a Nasal string, set the property to be a string
        if(naIsString(val)) result = p->setStringValue(naStr_data(val));

        // check if we have a vector
        else if (naIsVector(val) ) {
          SG_LOG(SG_GENERAL, SG_ALERT, "2nd argument to setprop() is a vector !");
          for (int i=0;i<naVec_size(val);i++) {
          SG_LOG(SG_GENERAL, SG_ALERT, "Updating property via vector index:" << i);
          //TODO: recall and adapted version of findnode() here

          }
        } // vectorized
        
        else {
            // it is neither a string, nor a value, so it bails out here:
            if(!naIsNum(val))
                naRuntimeError(c, "setprop() value is not string or number");
                
            if (SGMisc<double>::isNaN(val.num)) {
                naRuntimeError(c, "setprop() passed a NaN");
            }
            // finally, treat the whole thing a s a number and update that
            result = p->setDoubleValue(val.num);
        }
    } catch (const string& err) {
        naRuntimeError(c, (char *)err.c_str());
    }
    // return the number to the caller
    return naNum(result);
}


Thus, to fix up setprop() to also work on a vector of properties, we need to validate its arguments - by requiring the first argument to be a string (and a valid property path) and the 2nd argument should be a vector.

To do this, we can use the APIs discussed at Howto:Extend_Nasal#Argument_processing

Considerations

  • Instead of touching setprop() directly, it would make more sense to introduce a new dedicated API, even if only to make the review process much less of a hassle, because there's less chance to break things if a new API is introduced rather than touching an existing one that is used all over the place.
  • We can do away with the findnode() stuff to do things even faster
  • It may make sense to allow an optional offset to be specified to begin indexing using the offset instead of using the vector index
  • equally, it may make sense to allow an optional callback to be specified to handle pruning (analogous to a simple GC scheme)
  • modify/extend api.nas to provide an updateCoords() method that can directly use the new API (as per sanhozay's comments) [5]
  • it may also make sense to introduce support for setprop(property_path, format_string_for_values, values_vector) [6], e.g.:

setprop(property: "/foo/value", format_string:"%s = %dft (FL%d FL)", values:["altitude",12000, 120]);

Patch

References

References
Retrieved from "https://wiki.flightgear.org/w/index.php?title=Howto:Vectorizing_the_Nasal_setprop()_API&oldid=108842"