ImageEngine/gaffer — pull request #909 by johnhaddon (2014-07-15 18:29:41+00:00)
Title: Error tolerant loading

I'm putting this up for discussion rather than to necessarily have it merged. It implements error tolerance for loading of Gaffer scripts, as requested in #746.

I experimented with several approaches :

1. Execute a single line of python at a time, simply moving on to the next one on errors. This is dead simple, and pretty much identical in speed to the original. The problem with a line-at-a-time approach is that it can't support things like loops and conditionals, or anything else that spans multiple lines. Currently all our `.gfr` files should fit the line-at-a-time approach, but since `ScriptNode::execute()` could in theory be used to execute arbitrary Python, this introduces a potential issue, and reduces future flexibility in serialising into `.gfr` files.
2. Use the Python `ast` module to parse the code, then execute it a statement at a time rather than a line at a time. Because we're using a proper Python parser, we can support compound statements properly, moving on to the next one on error. This is much much slower than the original - taking more than 2x longer.
3. As above, but using the C API for the ast. This can cope with everything that 2) can, and is about 1.2x slower than the original. But it uses pretty low level APIs from a messy python header which looks rather subject to change.

At the moment I've gone with 3), but I'm not sure it's the best way to go in the long run. Our file loading isn't particularly fast, and currently about 50% of it is on the Python side, and the other 50% is on our side, constructing nodes and networks in C++. There are some opportunities for speeding up our side, but we'd quickly get to the point where to make progress we'd need to speed up the Python side. I kindof have the feeling that the only way to do that might be to make a very slimmed down parser that copes only with what we put in `.gfr` files, and doesn't support the whole Python language. If we did end up doing that, we might as well go with 1) now.

Any thoughts?