OXP Performance tips

[Ramen]

ToBeInserted

ビビビビビビビビビビビビビビビビ

(This is supposed to be ship lasers or something.)

[cag]

I think it's because we're bypassing JS type checking, prototype & boundary tests. For example, if list is null, we'll get a reference error. But I'd rather know that case exists sooner rather than later.

[Day]

I think it's because we're bypassing JS type checking, prototype & boundary tests. For example, if list is null, we'll get a reference error. But I'd rather know that case exists sooner rather than later.

Yup

[Day]

ToBeInserted

ビビビビビビビビビビビビビビビビ

(This is supposed to be ship lasers or something.)

ToBeInserted => Done
KabooM!!

(This is supposed to be the q-bomb ignited by the laser...)

[Day]

For your consideration, two more improvement propositions:

First, some micro-optimizations:
- Replace

Code: Select all

return $this.myfunction() ? true : false;

by

Code: Select all

return $this.myfunction();

- Replace

Code: Select all

return $this.myfunction() === true;

by

Code: Select all

return $this.myfunction();

- Replace

Code: Select all

return $this.myfunction() === false;

by

Code: Select all

return !$this.myfunction();

This should speed, but well, I guess it could even slow the code if the number of uses of the result is very greater than the number of calculations. What do you think?

A better one:
variables can be used for data structures, like this:

Code: Select all

var myvar = {"someValue"};

but they can be used for functions too:

Code: Select all

var myfunc = function() {
	var myinnerfunc = function() {};
	return myinnerfunc; // this returns the function itself
}
// Here I get the func, and use it
var myresult = myfunc(); // The parenthesis executes the function. myresult contains now the inner function.
myresult(); // The parenthesis executes the inner function :-)

So we can store functions into variables.
For example:

Code: Select all

var r = Math.random();
var z = 1000;
while (z--) {
	r();
}

If we need to call Math.random() in a loop, it will be quicker to store the function in a variable, and then execute the function from the variable rather than calling from inside the loop Math (which is external to the englobing function, so expensive), and then Math.random, which is a dereference, which is expensive.

The only thing to check is to be sure of what $this means inside the stored function.
When you execute a function stored into a variable, this in the function means the current this from where the function is called, not the this at the time of the function storage into the variable.
If you want to use the this at the time of the storage, use:

Code: Select all

var myfunc = (function() {}).bind(this);

The current this is then associated to the function, and will always be used when the function is executed.

So, what do you think?

[hoqllnq]

For example:

Code: Select all

var r = Math.random();
var z = 1000;
while (z--) {
	r();
}

Should that be:

Code: Select all

var r = Math.random;

without the parentheses,
so that the value of r becomes (a reference to) the function itself, not the result of calling the function?

[cag]

First, some micro-optimizations:

To test your assertion, I profiled the following:

Code: Select all

(function (){
	function truth() {
		var x = true;
		return x ? true : false;
	}
	function plain() {
		var x = true;
		return x;
	}
	log(console.profile( function() {var i=10000; var result =  false; while(i--) { result = result && truth() } } ) );
	log(console.profile( function() {var i=10000; var result =  false; while(i--) { result = result && plain() } } ) );
})()

I ran it a few times (the 1st profile run has some extra over-head) and got:

Code: Select all

truth: Total time: 0.467 ms
JavaScript: 0.458 ms, native: 0 ms
Counted towards limit: 0.467 ms, excluded: 0 ms
Profiler overhead: 0.018 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                               (<console input>) <anonymous>  J      1     0.46     0.46    98.1    98.1     0.46

plain: Total time: 0.283 ms
JavaScript: 0.275 ms, native: 0 ms
Counted towards limit: 0.283 ms, excluded: 0 ms
Profiler overhead: 0.014 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                               (<console input>) <anonymous>  J      1     0.27     0.27    97.2    97.2     0.27

but a few more runs gave me:

Code: Select all

truth: Total time: 0.28 ms
JavaScript: 0.271 ms, native: 0 ms
Counted towards limit: 0.28 ms, excluded: 0 ms
Profiler overhead: 0.016 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                               (<console input>) <anonymous>  J      1     0.27     0.27    96.8    96.8     0.27

plain: Total time: 0.311 ms
JavaScript: 0.298 ms, native: 0 ms
Counted towards limit: 0.311 ms, excluded: 0 ms
Profiler overhead: 0.019 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                               (<console input>) <anonymous>  J      1     0.30     0.30    95.8    95.8     0.30

At best we gain a fraction of a millisecond (averaged over 10k iterations, 100s of nano-seconds), so not much at all, at the expense of readability & explicit typing.

A better one:
variables can be used for data structures, like this:

Code: Select all

var myvar = {"someValue"};

but they can be used for functions too:

Code: Select all

var myfunc = function() {
	var myinnerfunc = function() {};
	return myinnerfunc; // this returns the function itself
}
// Here I get the func, and use it
var myresult = myfunc(); // The parenthesis executes the function. myresult contains now the inner function.
myresult(); // The parenthesis executes the inner function :-)

So we can store functions into variables.
For example:

Code: Select all

var r = Math.random();
var z = 1000;
while (z--) {
	r();
}

By returning myinnerfunc, you're telling JS that you'll be back (ie. will call the function later) and so it preserves its environment or scope. This is a 'closure': any variables you define will be preserved over each call (but the code itself won't be duplicated if you create many copies). Consider the following:

Code: Select all

(function (){
	function sum( start ) {
		var total = 0;
		if( start !== undefined ) total = start;
		function add( x ){
			total += x;
			return total;
		}
		return add;
	}
	var i = 10;
	var func1 = sum();
	while( i-- ) func1( i );
	log( 'result1: ' + func1( 0 ) );
	var func2 = sum( 100 );
	log( 'result2: ' + func2( 1 ) );
	log( 'result1: ' + func1( 0 ) );
	log( 'result2: ' + func2( 1 ) );
})()

Its output:

Code: Select all

result1: 45
result2: 101
result1: 45
result2: 102

The value of 'total' is preserved from one call to the next (call of add via func1), or we'd have result: 0 (or rather, a ReferenceError); And func2 has its own scope, independent of func1. From everything I've read, there is only one copy of the function 'add'.
Here's an example of how I sped up an FCB:

Code: Select all

this._VModelAndRing_closure = function() {
	// 'constant' variables
	var ws = worldScripts.telescope;
	var w_shiplib = worldScripts.shiplib;
	var dataKeys77 = ws.$DataKeys77;
	var autoLock = ws.$AutoLock;
[editted for brevity]
	// function references
	var system_addVisualEffect = system.addVisualEffect;
	var oolite_compareVersion = oolite.compareVersion;
	var _Scan = ws._Scan;
[editted for brevity]
	// local variables
	var vRing = null; //a ring around the visual effect target
	var vShip = null; //visual effect to show the selected target
	var vDataKey = null; //key of the visual effect
	var ps;
[editted for brevity]
	
	function getVShip() { return vShip; }
	function setVShip( ship, up, delay, length ) {
		vShip = ship;
[editted for brevity]
		return ship;
	}
	function clearVShip() { //Clear Visual Effect Ship Model and large visual ring
		if( vShip ) {
			vShip.remove();
			vShip = null;
		}
		if( vRing ) {//remove large visual ring
			vRing.remove();
			vRing = null;
		}
	}
	function showVShip() { //Show Visual Effect
		ps = player.ship && player.ship;
		if( !ws.$ShowVisualTarget ) return;
		if( !autoLock && !ps.target ) return;
[editted for brevity]
			_Scan();
[editted for brevity]
			vShip = system_addVisualEffect( dk, p );
[editted for brevity]

	}
	return { clear: clearVShip,
			   get: getVShip,
			  show: showVShip }
}

And in startUp()

Code: Select all

	var vc = ws._VModelAndRing_closure();
	ws._Clear_VModel_Ring = vc.clear;
	ws._GetVModel = vc.get;
	ws._ShowVModel = vc.show;

So, the pointers to the VisualEffects are stored inside _VModelAndRing_closure and the speed gain comes from having to only evaluate all the var statements (with their expensive worldscript property gets) before the line 'function getVShip() ...' once per game instead of once per frame!

You don't necessarily have to put a closure in an IIFE (immediately invoked fucntion expression - who comes up with this stuff?!) as you see in most online discussion of closures. In oolite (vs. a web page) it makes more sense to delay initiation of the closure, as some of your references won't exist right away, esp. if you reference another oxp; you may have to wait until startUpComplete.

For example:

Code: Select all

var r = Math.random();
...

Should that be:

Code: Select all

var r = Math.random;

without the parentheses,

Yup, the first 'r' has a random number, the second a function reference.

[Day]

At best we gain a fraction of a millisecond (averaged over 10k iterations, 100s of nano-seconds), so not much at all, at the expense of readability & explicit typing.

Ok, so this part is useless.

Thanks a lot for the profiling

[Day]

For example:

Code: Select all

var r = Math.random();
var z = 1000;
while (z--) {
	r();
}

Should that be:

Code: Select all

var r = Math.random;

without the parentheses,
so that the value of r becomes (a reference to) the function itself, not the result of calling the function?

Exactly.

[Day]

By returning myinnerfunc, you're telling JS that you'll be back (ie. will call the function later) and so it preserves its environment or scope. This is a 'closure': any variables you define will be preserved over each call (but the code itself won't be duplicated if you create many copies).
[...]
The value of 'total' is preserved from one call to the next (call of add via func1), or we'd have result: 0 (or rather, a ReferenceError); And func2 has its own scope, independent of func1. From everything I've read, there is only one copy of the function 'add'.
[...]
So, the pointers to the VisualEffects are stored inside _VModelAndRing_closure and the speed gain comes from having to only evaluate all the var statements (with their expensive worldscript property gets) before the line 'function getVShip() ...' once per game instead of once per frame!

You don't necessarily have to put a closure in an IIFE (immediately invoked fucntion expression - who comes up with this stuff?!) as you see in most online discussion of closures. In oolite (vs. a web page) it makes more sense to delay initiation of the closure, as some of your references won't exist right away, esp. if you reference another oxp; you may have to wait until startUpComplete.

I've carefully read all of that, yup, I'm totally ok with everything you wrote
I need a way to include this into the first post...

For the delay, I've developed a way;

In the master script:

Code: Select all

this.startUpComplete = function () {
    var s = this._subscribers.sort();
    var z = s.length, y = z - 1;
    while (z--) {
        var startDate = new Date();
        worldScripts[s[y - z]]._startUp();
        log(s[y - z], "startUp in ms: " + (new Date().getTime() - startDate.getTime()));
    }
    delete this.startUpComplete; // No need to startup twice
};
this._subscribers = []; // [ scriptName ]
this.$subscribe = function (aScriptName) {
    this._subscribers.push(aScriptName);
};

In the other scripts:

Code: Select all

this.startUp = function() {
    worldScripts.DayDiplomacy_000_Engine.$subscribe(this.name);
    delete this.startUp; // No need to startup twice
};

You don't necessarily have to put a closure in an IIFE (immediately invoked fucntion expression - who comes up with this stuff?!)

[cag]

In the other scripts:

And the subscribers will put their startUp code in:

Code: Select all

this._startUp = function() {
    // move your startUp code here
};

Submit to you, compel everyone, how will you, 'Master', hmm?

Granted, your solution is easier to grasp than
http://wiki.alioth.net/index.php/Handli ... JavaScript
but herding cats would be less daunting.

As for the closure optimization, many of the references will get defined when the script loads. And if people put as much initialization as possible in startUp (vs. startUpComplete, as the wiki suggests), then initializing closures in startUpComplete should work, for the most part. Esp. if we follow the 'use delete sparingly' rule. But there will always be some that require the full property lookup, like the oxp that does

Code: Select all

this.$myArrayThatEveryoneLovesToReference = [];

vs

Code: Select all

while( i-- ) this.$myArrayThatEveryoneLovesToReference.pop();

<sigh>
------------------------------------------------------------------------------------------------------------------------------------

I need a way to include this into the first post...

I'll try and get you started. Feel free to butcher what follows, it only sounds good in my head.

You should stress that this is something you do after the code has matured and only when there is a significant performance gain to be had. I'm learning that it's easy to introduce a bug if you're not careful... and debugging a closure is harder, mainly because the variables inside are 'private', in that they are only visible from functions inside the closure and not accessible from debug console. What I've had to do was put everything in the return object until I get the bugs fixed - I wish I had waited!

I'd suggest a slow & deliberate, step-by-step approach, at least until you get a feel for it. (If you editor has versioning, turn it on or get a better editor.)

First, just 'enclose' a single function:

Code: Select all

this._myFirst_closure = function() {
    function _my_orig_fn() {
        ...
    }
    return _my_orig_fn;
}
// and in startUpComplete() add:
    this._my_orig_fn = this._myFirst_closure();

and test it. Note you don't have to alter any other code, as the function calls remain the same. I'd also recommend that you preserve the 1st char. in your function name, so later, if you add some closure-only functions, they are easy to distinguish.

Second, cache the function calls made by _my_orig_fn(), as they are the least likely to change

Code: Select all

this._myFirst_closure = function() {
    var wm = worldScripts.my_fabulous_oxp;
    var _do_something = wm._do_something;
    function _my_orig_fn() {
        ...
        // wm._do_something(); gets changed to:
        _do_something();
        ...
    }
    return _my_orig_fn;
}

and test it. Simple enough, 1 property get per game vs. per call but they add up. When I started profiling functions, they all started like this:

Code: Select all

Total time: 6.936 ms
JavaScript: 2.821 ms, native: 4.106 ms
Counted towards limit: 5.63274 ms, excluded: 1.30326 ms
Profiler overhead: 2.472 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                                     WorldScriptsGetProperty  N     24     1.50     1.38    21.6    19.9     0.12
                                     ...

And 21.6% was average; I've seen as high as 60%, when all the variables were (script) globals, 'this.$myvar...'.

And if you're anyway near as skilled a typist as I am, never make the code changes manually! Use the editor's replace utility! That way, if you make a typo, it'll be glaring obvious (nothing works) vs subtle (single line typo only shows up a week later in some outlying circumstance).

Next, you can move in 'static' variables. Many oxp's have properties the user can play with by editing the .js file that are NOT available via the 'mode' & 'activated' events. Or real constants, like, this.$RADIANS_to_ANGLE = 180 / Math.PI;. The procedure is the same as with function references above. And test it.

Next, there are less 'static' variable, like values dealing with the player, his ship or the game environment. For these, we need a function to reset these whenever the ship launches.

Code: Select all

this._myFirst_closure = function() {
  // 'constant' variables
    var wm = worldScripts.my_fabulous_oxp;
    var gameWindow = oolite.gameSettings.gameWindow;
    var $RADIANS_to_ANGLE = wm.$RADIANS_to_ANGLE;
    var ps, scannerRange;
  // function references   
    var _do_something = wm._do_something;
    var addShips = system.addShips;
  // local variables
    var weaps = true;     //the previous state of the player weapons
    var vShip = null;     //visual effect to show the selected target
    
    function _reset_vars() { //            <============== call in shipLaunchedFromStation()
        ps = player.ship;
        scannerRange = ps.scannerRange;
        gameWindow = oolite.gameSettings.gameWindow;
    }
    function _my_orig_fn() {
        ...
       if( weaps && !ps.weaponsOnline ) {
           weaps = false;
            vShip = _addShips( ... );
        }
        ...
    }
    return { _my_orig_fn: _my_orig_fn,
             _reset_vars: _reset_vars };
}
// and startUpComplete() is now changed to this:
    var mfc = this._myFirst_closure();
    this._my_orig_fn = mfc._my_orig_fn;
    this._reset_vars = mfc._reset_vars;
    ...
// and in shipLaunchedFromStation() add:
    this._reset_vars();

And test it! I sound like a broken record (boy, does that date me!) but it'll probably be faster (and saner) if you test each step, at least the first time doing this.

A few things to note:
* closure now returns an object. By naming the properties the same as their values, we eliminate one typo vector (I admit, it looks weird) vs
return { my_fn: _my_orig_fn, ... and this._my_orig_fn = mfc.my_fn;
* choose your semi-static variables with care. Above, I cache scannerRange but not weaponsOnline (d'uh) but it may not always be that obvious
* by storing a value inside the closure (e.g. vShip) it's protected/hidden from getting clobbered by outside code but you must add a function, called from shipWillDockWithStation(), say, that will vShip.remove(); vShip = null; to clean up; you cannot do so directly from shipWillDockWithStation()
* support for the var gameWindow is incomplete, as the player can change some game settings when in space. In this case, we may have to load it on each call but if it's used more that once, we're ahead, esp. since any sub-routine calls inside the closure also benefits
* sharing a closure's variable with outside functions is messy. You need either 'set' & 'get' functions or remember to always do wm.$myvar = myvar = .... when you alter its value. I've avoided that so far, instead moving the other function into the closure!

You can see by the last 3 points that closures have a tendency to grow, another good reason to delay implementation. But the speed gains, esp. in timer and frame callback functions, are substantial.

[cag]

- the following is faster than indexOf when dealing with arrays:

Code: Select all

this._index_in_list = function( item, list ) { // for arrays only
    var k = list.length;
    while( k-- ) {
        if( list[ k ] === item ) return k;
    }
    return -1;
}

I've decided to add if( !list ) return -1; as the 1st line, so that calling it is cleaner. I'll get an occasional extra function call but the calling statement goes from

Code: Select all

if( my_list && _index_in_list( x, my_list ) )

to

Code: Select all

if( _index_in_list( x, my_list ) )

Shifting the existence check into the function makes very little difference. I profiled both in 10k loops and they're very close. At one end, it was 3.5 ms slower, while the other extreme saw it 1.4 ms faster!?? On average, about 1 ms slower or 0.0001 ms (0.1 micro-second) / call.

for your 'wiki':

- the following is faster than indexOf when dealing with arrays:

Code: Select all

this._index_in_list = function( item, list ) { // for arrays only
    if( !list ) return -1;
    var k = list.length;
    while( k-- ) {
        if( list[ k ] === item ) return k;
    }
    return -1;
}

so,

Code: Select all

if( targets && targets.indexOf( ship ) ...

becomes

Code: Select all

if( ws._index_in_list( ship, targets ) ...

[Day]

In the other scripts:

And the subscribers will put their startUp code in:

Code: Select all

this._startUp = function() {
    // move your startUp code here
};

Submit to you, compel everyone, how will you, 'Master', hmm?

Granted, your solution is easier to grasp than
http://wiki.alioth.net/index.php/Handli ... JavaScript
but herding cats would be less daunting.

Sorry and thank you, I forgot to include this part.

I didn't mean anything by that. It's only a subscriber/listener pattern, but if I begin to use these terms, I'm sure I'll confuse beginners even more

This solution is interesting because it allows to develop the dependencies piece by piece.
One "master" per functionality, not one per package.

And if you're anyway near as skilled a typist as I am, never make the code changes manually! Use the editor's replace utility! That way, if you make a typo, it'll be glaring obvious (nothing works) vs subtle (single line typo only shows up a week later in some outlying circumstance).

Exactly! I use Intellij Idea.

I need time to process all of this
Maybe it's time to put the wiki page into place

[Day]

The tip I gave phkb, and I realized then that although it is implicit in what was already exposed, it is not obvious:

From:

Code: Select all

this.$rand = function(max) {
	return Math.floor((Math.random() * max) + 1);
}

To:

Code: Select all

this.$rand = function $rand(max) {
	var that = $rand;
	var floor = (that.floor = that.floor || Math.floor);
	var random = (that.random = that.random || Math.random);
	return floor((random() * max) + 1);
}

This way, rather than using 2 accesses outside the function (and quite far away, in a library), after the first time, you only use 2 accesses to an immediate property of the function.
Seems it was enough to make a big difference in phkb code.

I always do this, each time I access an external library or script.

[cag]

Cool! And certainly 'not obvious' (to me, at least). Remember most of us are self-taught wrt JScript - at first I thought that was another keyword, like self in Python

So you're basically storing a (far) reference as a function's property. Would it be safe to say, for clarity, that

Code: Select all

var floor = (that.floor = that.floor || Math.floor);

is logically equivalent to

Code: Select all

if( that.floor === undefined ) that.floor = Math.floor;
var floor = that.floor;

You get some of the speed gain of a closure (where these references are instead, stored in persistent local variables), without all the overhead ( & drama!), with WorldScriptsGetProperty being a most expensive one. This should definitely appear before any discussion of closures in your wiki.

As an example, I put the above change on a single call to _dump_map(), which is in a different .js file and went from:

Code: Select all

Total time: 6.541 ms
JavaScript: 3.824 ms, native: 2.711 ms
Counted towards limit: 5.06183 ms, excluded: 1.47917 ms
Profiler overhead: 1.719 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                                 (cagsdebug.js:28) _dump_map  J      1     5.85     2.55    89.5    39.0     2.55
                                                   GlobalLog  N      2     0.81     0.74    12.4    11.4     0.39
                                (cagsdebug.js:36) number_str  J     51     0.44     0.44     6.7     6.7     0.20
                       (telescope.js:796) _RelativeDirection  J     17     0.96     0.30    14.6     4.6     0.06
                                           EntityGetProperty  N    166     0.54     0.29     8.2     4.4     0.01
                                      (cagsdebug.js:29) dist  J     17     0.90     0.23    13.7     3.5     0.06
                                     WorldScriptsGetProperty  N      3     0.25     0.23     3.8     3.5     0.12
-[NSObject(OOJavaScriptConversion) oo:jsDescriptionWithClassName:]  N     17     0.22     0.22     3.4     3.4     0.02
                      (telescope.js:1269) _detect_distanceTo  J     17     0.56     0.21     8.5     3.3     0.04
                                                          ...

to:

Code: Select all

Total time: 4.622 ms
JavaScript: 1.981 ms, native: 2.634 ms
Counted towards limit: 3.11484 ms, excluded: 1.50716 ms
Profiler overhead: 1.411 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                                 (cagsdebug.js:28) _dump_map  J      1     4.08     0.99    88.2    21.5     0.99
                                                   GlobalLog  N      2     0.86     0.79    18.6    17.2     0.48
                       (telescope.js:796) _RelativeDirection  J     17     0.94     0.31    20.3     6.7     0.08
                                           EntityGetProperty  N    166     0.52     0.27    11.3     5.9     0.01
-[NSObject(OOJavaScriptConversion) oo:jsDescriptionWithClassName:]  N     17     0.24     0.24     5.1     5.1     0.03
                      (telescope.js:1269) _detect_distanceTo  J     17     0.55     0.21    11.8     4.6     0.04
                                     WorldScriptsGetProperty  N      2     0.20     0.19     4.4     4.2     0.11
                                      (cagsdebug.js:29) dist  J     17     0.83     0.19    18.0     4.1     0.05
                                (cagsdebug.js:36) number_str  J     51     0.19     0.19     4.0     4.0     0.03
                                                            ...

Not only did I lose an expensive WorldScriptsGetProperty but the functions that _dumpmap() itself called (in that same .js file) also run faster (due to interpreter optimizations).

Anyone who has multiple .js files in their oxp should try this as a first step; the speed gain is quite high vs the effort it takes!