"Eval Once" for engine script calls: optimal, sub-optimal, dangerous, or a reinvented wheel?

[TheBeardyMan]

Conditional branches are one example of where the engine can call Javascript code provided by the user. They're processed by the function Game_Interpreter.prototype.command111 at lines 9183 to 9318 in rpg_objects.js, and lines 9306 to 9308 of that function handle the case where the condition must evaluate Javascript code. They read:

        case 12:  // Script
            result = !!eval(this._params[1]);
            break;

But that code calls eval on a string containing Javascript code every time the conditional branch is executed, and eval in Javascript is noted for being slow. It appears that it could be replaced with the following code, which will cause the eval call to be made only once per map visit:

        case 12:  // Script
            if (typeof(this._params[1]) === 'string') {
                eval('this._params[1] = () => ' + this._params[1]);
            }
            result = !!this._params[1]();
            break;

I've tested this with a simple condition, and it seems to work, but are any performance improvements likely? Or is this likely to break horribly in a way that I've overlooked?

 

[gstv87]

eval(expr) can return anything, from nil, to an integer, to true/false.
!expr assumes "expr" to be considerable as true/false, meaning, it'll turn "nil" into "false" and negate it, returning "true", which prevents some crashes related to comparing "nil" to basically anything else.
!!expr flips that on itself again, and returns "false" which is the translated equivalent to "nil", but with "false" being able to be compared logically.
any other cases, would return "true", as expected.

that expression is there to catch the odd case where the condition doesn't return anything that can be translated to boolean.
you don't need to catch the case where that expression is a string, because the original instruction doesn't care about it being a string, it just passes it as is and only works with the result, whichever it might be.

as a designer, you wouldn't even evaluate a function in that conditional branch to begin with.... you would try to simplify the comparison to a case of booleans as much as you can, so that conversion shouldn't be even considered.
*BUT*, it is there in the odd case that you have no other choice than to send an actual string with a function that returns a comparable result.

 

[TheBeardyMan]

eval(expr) can return anything, from nil, to an integer, to true/false.
!expr assumes "expr" to be considerable as true/false, meaning, it'll turn "nil" into "false" and negate it, returning "true", which prevents some crashes related to comparing "nil" to basically anything else.
!!expr flips that on itself again, and returns "false" which is the translated equivalent to "nil", but with "false" being able to be compared logically.
any other cases, would return "true", as expected.

that expression is there to catch the odd case where the condition doesn't return anything that can be translated to boolean.

Yes, converting non-boolean results of the expression to boolean is why I kept the !! in the new code - except that it's now applied to the result of calling a function referenced by this._params[1], instead of the result of an eval call on a string referenced by this._params[1].

you don't need to catch the case where that expression is a string, because the original instruction doesn't care about it being a string, it just passes it as is and only works with the result, whichever it might be.

The line:

            if (typeof(this._params[1]) === 'string') {

isn't checking the type of the result of evaluating the expression, it's checking whether the object referenced by this._params[1] is still a string representing the expression that would need an eval call to get the expression's result. In the original code, the type of the object referenced by this._params[1] is always string, because it never gets changed. In the new code, it's string only on the first pass, because this line:

                eval('this._params[1] = () => ' + this._params[1]);

evals an expression that overwrites this._params[1] with a reference to an anonymous function that returns the result of the original expression without needing to make any eval calls itself. The result of the expression is then obtained by calling that anonymous function. Later passes of the code for the same conditional branch don't need to make any eval calls because they find that this._params[1] isn't a string - it's the function that was created in the first pass - and they just need to call it to get the result.

as a designer, you wouldn't even evaluate a function in that conditional branch to begin with.... you would try to simplify the comparison to a case of booleans as much as you can, so that conversion shouldn't be even considered.
*BUT*, it is there in the odd case that you have no other choice than to send an actual string with a function that returns a comparable result.

Yes, conditional branches that can be simplified to the point where they don't need to use the "script" setting from tab 4 of the "Conditional Branch" dialog won't ever cause case 12 of the top level switch statement in Game_Interpreter.prototype.command111 to be hit. This possible optimization is for conditional branches that do need to use the "script" setting.

 

[Aloe Guvner]

In theory, yeah, caching it as a function and then just calling that function will be much much faster on subsequent executions. You could also use the Function constructor for the same purpose.

I'm not sure about this though, but I haven't been able to test it yet. I thought `this._params` gets reset on every event command?

Another idea is that you could use the string value of this._params[1] as key to some sort of object, where the value of that key is the function to execute - in a memoization-type pattern. I'm not sure where the best place to store such an object would be, but here's some (pseudocode), just to brainstorm ideas.

// If the function hasn't been constructed yet, create it
if (!obj[this._params[1]]) {
    obj[this._params[1]] = new Function(this._params[1]);
}
// Execute the function
obj[this._params[1]]();

eval is slow, so I appreciate this idea you have to make the engine run more efficiently. Let us know what you find if you end up testing any more.

 

[Shaz]

which will cause the eval call to be made only once per map visit

Maybe I'm misunderstanding what you're saying, but what if the result of the eval changes while you're on the map?

What if I did something really basic, like put $gameSwitches.value(15) in the Script of my Conditional Branch, and I have another event on the map that turns that switch on? I would want it to return a different value before I interact with that event, to what it returns after I interact with it.

I thought `this._params` gets reset on every event command?

Yes, it does.


I am not sure how replacing one command with a !! and eval, with a number of commands that have a !!, an eval, and a condition, is going to be more efficient?

 

[LTN Games]

Micro optimising is a waste of time in my opinion and you're better off spending time on developing rather than discussing and worrying about such a minor issue. Unless of course it is a major issue and you are calling the eval a lot every frame where there actually is an issue with performance. But then it's not really about optimising the eval but optimizing the logic as of calling the eval every frame.

 

[TheBeardyMan]

Maybe I'm misunderstanding what you're saying, but what if the result of the eval changes while you're on the map?

What if I did something really basic, like put $gameSwitches.value(15) in the Script of my Conditional Branch, and I have another event on the map that turns that switch on? I would want it to return a different value before I interact with that event, to what it returns after I interact with it.

Immediately before this line of code is executed:

                eval('this._params[1] = () => ' + this._params[1]);

this._params[1] references a string containing the expression - in your example it would be equal to "$gameSwitches.value(15)". But that line of code doesn't evaluate the expression in the string referenced by this._params[1] - it creates a new string equal to the concatenation of 'this._params[1] = () => ' and the string containing the expression and evals that. In your example, the string that gets eval'ed by that line of code would be:

                'this._params[1] = () => $gameSwitches.value(15)'

which doesn't evaluate the expression $gameSwitches.value(15). It creates an anonymous function that would evaluate $gameSwitches.value(15) when called, but it doesn't actually call that function at this point. Instead, it modifies this._params[1] to reference that function. The anonymous function isn't called until we reach this line:

            result = !!this._params[1]();

and at that point, we're not necessarily calling an anonymous function that we created on this pass - we could be calling a function that we created on an earlier pass, having detected that this._params[1] isn't a string, thus avoiding an eval call on every pass after the first. And regardless of whether we're calling a new function or an old one, the function returns the current value $gameSwitches.value(15) - we don't "bake" an old value of $gameSwitches.value(15) into the function when we create it.

Yes, it does.

The value assigned to this._params at the start of an event command is a reference to the array of parameters in the event data - the array of parameters isn't copied by value. So when the code for executing an event command modifies a parameter, the modified value will persist for the remainder of the player's current visit to the map.

 

[Another Fen]

First, I generally agree with LTN Games here.
There is nothing wrong with optimizing your code, but just replacing a command with a slightly faster one will hardly fix your performance issues. At least from my experience whenever you have a lot of continuously executed script commands, it's often easier to make that event system a plugin in the first place.

Aside from that, using eval also allows you to create script commands with multiple instructions like
var v = $gameVariables.value; (v(3) + v(4)) > (v(5) + v(6))
which right now would break in your example above.

Also, your event command lists are also saved in your games savefiles, and while I haven't tested it I don't think lambda expressions or functions can be serialized using the Json format. So you probably would have to exempt them or find another place to store your code.

Edit: The context your expression will be executed in might change during gameplay, as the same event command list can be executed by different interpreter objects. If your script commands contain "this", you would probably have to bind the expression to the current context every time.

 

[Aloe Guvner]

@LTN Games I hear you man, but this is posted in Learning Javascript, so it could be a purely academic pursuit (which is totally fine in my opinion). However, there are cases where eval's in popular plugins really cause a hit to performance (I've done several commissions to improve plugins out there which causes a noticeable increase in FPS). I think it would be good for the community in general to understand the possible impact of eval's, especially when people add 50 plugins to their game, fill their database with eval notetags, and wonder why it lags.

The value assigned to this._params at the start of an event command is a reference to the array of parameters in the event data - the array of parameters isn't copied by value. So when the code for executing an event command modifies a parameter, the modified value will persist for the remainder of the player's current visit to the map.

I see now, you're right. Modifying `this._params` will modify the actual command that exists in `this._list`, where the list exists for the lifetime of that Game_Interpreter instance.

Spoiler

Game_Interpreter.prototype.executeCommand = function() {
    var command = this.currentCommand();
    if (command) {
        this._params = command.parameters;
        this._indent = command.indent;
        var methodName = 'command' + command.code;
        if (typeof this[methodName] === 'function') {
            if (!this[methodName]()) {
                return false;
            }
        }
        this._index++;
    } else {
        this.terminate();
    }
    return true;
};

Game_Interpreter.prototype.currentCommand = function() {
    return this._list[this._index];
};

@TheBeardyMan Have you tested how it would work in a Common Event?

 

[peq42_]

I once tried fo find out if there were ways to speed up the Eval()

All that I could find that wouldn't(as far as I know) limit the possibilities of Eval is placing the string inside a function before using eval on it. Example:

Eval("(function(){\n" + thestring + "\n})()")


For some reason, that way the V8 engine optimises the code at least sometimes, when it isn't something really simple such as a "for" loop to add numbers or something, making it up to 4x faster(but still, tends to be 10x slower than if you were to execute the code on startup as a normal part of your script)

edit: o shoot wait, seems you already knew that

 

[LTN Games]

If we really want to discuss how to speed up the use of eval then someone should create a small interpreter that allows the plugin users to still input javascript code that is evaluated using it. A small interpreter would also open up options of adding easier to understand syntax for plugin users and it would definitely improve performance compared to eval.
I was not trying to say don't waste your time discussing ways to improve performance I was just trying to move the discussion to something that is actually worthwhile of improving(sorry if it sounded that way), as others have mentioned improving the speed of eval is a micro-optimization amd technically a dead end and trying to squeeze as much juice from it as you can is just unrealistic in my opinion. What should be discussed here is why evals are being used and where they're being used, because this is the true cause of the performance issues, it's not the eval it's the logic. You already know my suggestion, a custom interpreter wold make a world of a difference, unfortunately that won't fix any older plugins using eval unless they're updated. Just my two cents.

 

[Aloe Guvner]

Custom interpreter would be quite interesting, but sounds like too much work

I think this technique of caching the eval as a function could have a noticeable improvement if, say, somebody was running a complex Eval condition in a parallel event (every frame). But as you said, the better approach would be to figure out why we need to run the condition every frame, could we instead only run it on-demand when it's needed.