OXP Performance tips

[Day]

Hi everybody, I create this topic to have a place to share our performance tips.

To put this into context, I had to improve drastically my code performance-wise for my Diplomacy OXP, where I (begin to) simulate wars between 2048 planetary systems. So... 2048 x 2048 = 4.000.000 relations.
(Yes, I know, more along 8 x 256 x 256; and still more along 8 x 256 x 7 (average systems in range). It's still 14000 operations for each action in the universe )

So without any more ado, here is what I learnt:

- Used time in oxps functions is limited. So, the first limit is a hard one: your functions will crash if they take too much time.

- Oxp scripts impact framerate. Rather than trying to optimize everything, you can profile to identify which parts need the most improvement. The goal should be to impact the framerate the least possible.

- Closures are extremely costly. Minimize cost of closures.
A closure is when you access something which is outside your function.
No closure when possible. Save the closure result in a variable to avoid doing it again.
In oxps, save particularly the native obj-c oolite objects (ie javascript calls provided directly by oolite; oolite is developed in the obj-c language, and provide special javascript objects to use in oxps, like SystemInfo for example, but using them is costly).
Functions inside functions (technically closures) generate a new function each time the enclosing function is called: memory leak. Use a prototype if necessary, or declare the inner function outside.
Src: https://developers.google.com/speed/art ... javascript

- Dereferences are costly. Minimize cost of dereferences.
A dereference is (well, at least it's what I call them) when you access a property of an object in this fashion:

Code: Select all

thingie.myProperty or thingie['myProperty']

Save the result in a variable to avoid doing it again.
A dereference on this is costly too, especially if it isn't set, as it checks all the prototype chain.
So save your this.something in a variable if you're using it more than once.
In oxps, save particularly the native obj-c oolite sub-objects.

Edit 1.1:
- Function calls are costly. Do not split your functions depending on their meaning. Split them depending on what calls them and when. Use comments and correctly named functions and variables to convey meaning.

- No need to use singletons, as the Script is one. (A Singleton is an object that you only have once. For example, a script in your OXP is a singleton: whatever the location you access it, it will always be the same object. As the script is a singleton, everything you put into it is never created twice, so you don't need a dedicated piece of code to ensure it is unique (another singleton). So... there is no need to implement singletons in Oolite. This advice might be useful only to pro dev willing to code oxps.)

- Use compiled regexps, initialized only once, rather than regexps created each time.

- Don't use non-native Objects, if you'll have to (de)serialize them. It will slow as hell your (de)serialization.

- No foreach loops, no for loops. Use this way:

Code: Select all

var z = myArray.length;
while (z--) {
    var myElement = myArray[z];
    // my code
}

This is the quickest way as it caches the array length and compares to zero only.


Volunteered by 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;
}

so,

Code: Select all

if( targets.indexOf( ship ) ...

becomes

Code: Select all

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

- to speed your functions, rather than filtering your data at the execution, you might store it prefiltered in this way:

Code: Select all

{dataType: [data,...], ...}

To speed it more, store separately the filter and the data:

Code: Select all

{dataType: [dataId,...], ...}
{dataId: data,...}

This way, you avoid the for...in loop, the hasOwnProperty() check, and of course you avoid iterating on the prototypes' properties.

And of course:

- save everything used twice in a variable,

- put everything that can be calculated outside of loops, well, outside of loops,

- put everything that might be useless because only needed after a return, well, after the return.

Doing this, I have sped my code by at least a factor of 40.
(Which is not always enough...)

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();

Edit 1.1:
Not yet tested (but I plan to):
- use tco to avoid recursion: http://www.integralist.co.uk/posts/js-recursion.html
- minify to improve performance: https://jscompress.com/, https://github.com/mishoo/UglifyJS2
- use lazy regexps

You may consider as an alternative to addFrameCallback() this pattern.
Disclaimer: not personnally tested, I opted for addFrameCallback() to have it running synchronously with the other js scripts. So I do not know if it would work inside Oolite.

Here is a example of a complex oxp, fully optimized (and if you find something not optimized, just tell me and I'll do it :p ):
http://pradier.info/owncloud/index.php/ ... t/download

So, what do you think?
Have you got some tips to share?

[Astrobe]

That's good to know. That I asked myself that sort of question because the few OXP I made use the OnFrameCallback() (or something like that), but I was too happy that my stuff finally worked (working with dynamically typed languages can be a pain sometimes), and I didn't see any slowdown, so I didn't investigate the performance side. But the truth is that if one piles up bits of suboptimal code here and there one ends up with a slow game and no obvious culprit. A "Do and Don't" page on the wiki could help with avoiding this, I think.

I'm not sure about minification, because I believe Oolite precompiles the scripts and keep the result in a cache. I believe at worst it increases startup times but not execution times.

[Day]

That's good to know. That I asked myself that sort of question because the few OXP I made use the OnFrameCallback() (or something like that), but I was too happy that my stuff finally worked (working with dynamically typed languages can be a pain sometimes), and I didn't see any slowdown, so I didn't investigate the performance side. But the truth is that if one piles up bits of suboptimal code here and there one ends up with a slow game and no obvious culprit. A "Do and Don't" page on the wiki could help with avoiding this, I think.

I'm not sure about minification, because I believe Oolite precompiles the scripts and keep the result in a cache. I believe at worst it increases startup times but not execution times.

Hi Astrobe, thank you for the kind words.

Well, I use addFrameCallback() too. I wondered if it would be less wasteful to use something directly implemented in javascript, and finally decided against it because at least this way the execution would be guaranteed synchronous by oolite itself.
Now, if there is a less wasteful way to do lots of small tasks, I take it

Concerning minification, I don't know. I'll measure the improvement when I implement it, but I'm not convinced it will be noticeable, too.

If the tips are deemed worthy enough, I'll put them into the wiki

Ps: edited for function name correctness.

[cag]

Good idea for a topic but it will need to a) be made sticky and b) moderated (ie. edit/modify/delete posts). This has been tried before but after a few pages of off topic discussion, I usually give up on it. You've got some good ideas but we need across the board adoption to really impact frame rate. I believe much of our frame rate deficit can be restored if we all can avoid these pitfalls.

Remember who your audience is, mostly, self-taught neophytes and expand a bit on some of your points:
- what's a singleton?
- compile a regexp? cool, but how?
- native obj-c oolite objects? in english, please, with maybe an example
- is there really a OnFrameCallback function? where's it documented?

You get the idea. The last thing we want to do is send a newbie on a wild goose chase; (s)he may not come back.

We also need a small collection of model scripts for authors to emulate. Documentation is boring (and in oolite's case, quite terse), so often people look at other scripts to see how something is done. So these problems are copied in the belief they are correct (the oxp is popular, the author has published many oxp's, etc)

Finally, we're going to have to deal with the orphaned oxp's, whose authors are no longer around. Compile a list, recruit people to re-code, proof and test. See what you started!
-----

- never use 'delete' - it's not doing what you'd expect. In shipWillDockWithStation, I see a lot of

Code: Select all

if( isValidFrameCallback( this.x) ) {
	removeFrameCallback( this.x);
	delete this.x;
}

where the last line just needs to be

Code: Select all

this.x= null;

If that's the only reference, garbage collection will do the rest. The delete command removes the property 'x' from the script's object, is an expensive op and not necessary. (esp. if you do 'x= addFrameCallback(...)' in shipWillLaunchFromStation)
-----

I have a problem with your use of the term 'closure'. I've just realized serious frame rate gains by using closures, but of course, we're talking about 2 different things. As I understand the term, a closure is a persistent local environment that JavaScript creates only when you return an inner function. Consider the following:

Code: Select all

this.$uservar = 5;

this._func = function() {
	if( this.$uservar < 0 ) 
		this._do_this();
	else
		this.$uservar = this._do_that();
}

This is costly because of fetching this.$uservar at least once/call. Many oxp's have user setting like this, which only change when the script is editted. By using a closure, you only need fetch this.$uservar once/game, like this:

Code: Select all

this.$uservar = 5;

this.startUp = function() {
	this._func = this._setup();
	...
}

this._setup = function() {
	var dothis = this._do_this;
	var dothat = this._do_that;
	var user_var = this.$uservar;
	func = function() {
		if( user_var < 0 ) 
			dothis();
		else
			user_var = dothat();
	}
	return func;
}

So, the propertyGet for this.$uservar is only done once, in startUp, and stored locally in user_var for the rest of the game. As you can see, this also works with functions as well as any data that passed by reference, like arrays. BUT great care is warranted, lest screwy bugs appear, when dealing with global data! Avoid global data until you have a good understanding of this. However, debugging is more difficult as user_var is not available via the debug console, so I'd advise leaving this step until after you've got the code well developed.

I've also been able to combine this with your 'deference' advice by grouping functions that use the same properties into a single closure.
-----

Functions calls aren't as costly as some of those other things but your advise valuable for a different reason, what I like to call (read scream) the GDERM bug (feature?).
It roughly stands for 'Gosh Darn Event(handler) Return Maybe' bug. Unlike interrupt handlers, event handler sometimes don't return to where they so rudely elbowed their way into your code's execution. In figuring this out (don't ask), it appears oolite respect function boundaries (ie. it'll wait until your return statement to butt in). My point is, if you have many short functions vs. a few larger ones, you're more apt to encounter this little gem. You'd think this would only occur amidst a flurry of events but it can happen in a relatively tame environment.
-----

"use tco to avoid recursion" should read "avoid recursion". Seriously.
-----

I'm with Astrobe wrt minification, even if there was a small performance gain. The minified code would be unreadable, so you'd have a source folder to your oxp. Remember, under our licences, we're allowed to steal share code.
-----

re: alternative to onFrameCallback()
It's an interesting solution, but according to our wiki, Timer's interval "will be rounded up to 0.25". Is that no longer true?
You can dynamically allocate callbacks but it's of limited use as it can take a handful of frames to take effect; if your list is static and you have, say, 20+ items and want to do 4/frame, it works fine. But if your list is changing and you want to scale the processing rate to measured performance, not so much.
Another way I've done it is a generalized version of the 'chunk' function in your link: functions needing items processed push their request onto a stack while a frame callback checks the stack and executes a set number of them. I still haven't figured out how to vary the number to match the hardware. If I observe a slow frame rate, is it a slow machine or a fast machine over-tasked by oxp's ...
-----

- put everything that can be calculated at load time outside of functions

eg. this.$FORTYFIVE_DEGREES = 45 * Math.PI / 180;
but not: this.$CORRECT_FOR_WIDESCR = oolite.gameSettings.gameWindow.height / oolite.gameSettings.gameWindow.width;
-----

On a lighter note, anything you scoop will turn hostile and target you, including cargo pods and splinters!
Also, a spawned entity has isVisible set to true, regardless of distance. It will change to false but I don't know how long that takes.

[another_commander]

Interesting and useful topic stickified. Keep the suggestions coming guys.

[Day]

Thank you cag, this is quite an interesting post!

Good idea for a topic

Thank you

but it will need to a) be made sticky

Done. Thank you another_commander

and b) moderated (ie. edit/modify/delete posts). This has been tried before but after a few pages of off topic discussion, I usually give up on it.

Ok, I'll edit the first post to include all incoming information, up until the time this post is too big. Then (or sooner) I'll move it to the wiki.
I won't moderate the posts of others, because:
a) I'm no moderator,
b) shame would kill me

You've got some good ideas but we need across the board adoption to really impact frame rate. I believe much of our frame rate deficit can be restored if we all can avoid these pitfalls.

Interesting. I thought about improving the most costly oxps, but you're right, the way to go is to promote the existence of an ideal framerate goal.

This means we need to profile, in terms of framerate. Done.

Remember who your audience is, mostly, self-taught neophytes and expand a bit on some of your points:
- what's a singleton?
- compile a regexp? cool, but how?
- native obj-c oolite objects? in english, please, with maybe an example
- is there really a OnFrameCallback function? where's it documented?

You get the idea. The last thing we want to do is send a newbie on a wild goose chase; (s)he may not come back.

Right. I'll edit these points.
- Singleton: ToBeDefined => Done
- compiling a regexp: ToBeDefined
- native obj-c oolite objects: ToBeDefined => Done
- onFrameCallback function: ToBeDefined => Done

We also need a small collection of model scripts for authors to emulate. Documentation is boring (and in oolite's case, quite terse), so often people look at other scripts to see how something is done. So these problems are copied in the belief they are correct (the oxp is popular, the author has published many oxp's, etc)

As a first one, I propose my Diplomacy OXP. Done.
It's already optimized for performance, and is written by a pro dev (ok, not a pro javascript dev, you'll have to be happy with a java dev).
If it is found imperfect in some ways (about which I have no doubt, it is perfect), I'll be happy to improve it.

Finally, we're going to have to deal with the orphaned oxp's, whose authors are no longer around. Compile a list, recruit people to re-code, proof and test. See what you started!

I've got no shame, and I've got you !

- never use 'delete' - it's not doing what you'd expect. In shipWillDockWithStation, I see a lot of

Code: Select all

if( isValidFrameCallback( this.x) ) {
	removeFrameCallback( this.x);
	delete this.x;
}

where the last line just needs to be

Code: Select all

this.x= null;

If that's the only reference, garbage collection will do the rest. The delete command removes the property 'x' from the script's object, is an expensive op and not necessary. (esp. if you do 'x= addFrameCallback(...)' in shipWillLaunchFromStation)

ToBeInserted

I have a problem with your use of the term 'closure'. I've just realized serious frame rate gains by using closures, but of course, we're talking about 2 different things. As I understand the term, a closure is a persistent local environment that JavaScript creates only when you return an inner function. Consider the following:

Code: Select all

this.$uservar = 5;

this._func = function() {
	if( this.$uservar < 0 ) 
		this._do_this();
	else
		this.$uservar = this._do_that();
}

This is costly because of fetching this.$uservar at least once/call. Many oxp's have user setting like this, which only change when the script is editted. By using a closure, you only need fetch this.$uservar once/game, like this:

Code: Select all

this.$uservar = 5;

this.startUp = function() {
	this._func = this._setup();
	...
}

this._setup = function() {
	var dothis = this._do_this;
	var dothat = this._do_that;
	var user_var = this.$uservar;
	func = function() {
		if( user_var < 0 ) 
			dothis();
		else
			user_var = dothat();
	}
	return func;
}

So, the propertyGet for this.$uservar is only done once, in startUp, and stored locally in user_var for the rest of the game. As you can see, this also works with functions as well as any data that passed by reference, like arrays. BUT great care is warranted, lest screwy bugs appear, when dealing with global data! Avoid global data until you have a good understanding of this. However, debugging is more difficult as user_var is not available via the debug console, so I'd advise leaving this step until after you've got the code well developed.

I've also been able to combine this with your 'deference' advice by grouping functions that use the same properties into a single closure.

I may not have used the closure word in exactly the correct way.
ToBeChecked

Having grokked for this ( https://stackoverflow.com/questions/180 ... javascript ), I'll now use it as meaning (a function + an external environment).

Closures are necessary, and better than some other alternatives. I only say they must be avoided when possible.

I don't use the debug console, so...

Functions calls aren't as costly as some of those other things but your advise valuable for a different reason, what I like to call (read scream) the GDERM bug (feature?).
It roughly stands for 'Gosh Darn Event(handler) Return Maybe' bug. Unlike interrupt handlers, event handler sometimes don't return to where they so rudely elbowed their way into your code's execution. In figuring this out (don't ask), it appears oolite respect function boundaries (ie. it'll wait until your return statement to butt in). My point is, if you have many short functions vs. a few larger ones, you're more apt to encounter this little gem. You'd think this would only occur amidst a flurry of events but it can happen in a relatively tame environment.
-----

"use tco to avoid recursion" should read "avoid recursion". Seriously.

Well, yes.
The reason I didn't implement tco was I got rid of recursion.
ToBeIncluded

I'm with Astrobe wrt minification, even if there was a small performance gain. The minified code would be unreadable, so you'd have a source folder to your oxp. Remember, under our licences, we're allowed to steal share code.

This is what I have in mind: a source folder in the oxp.

re: alternative to onFrameCallback()
It's an interesting solution, but according to our wiki, Timer's interval "will be rounded up to 0.25". Is that no longer true?
You can dynamically allocate callbacks but it's of limited use as it can take a handful of frames to take effect; if your list is static and you have, say, 20+ items and want to do 4/frame, it works fine. But if your list is changing and you want to scale the processing rate to measured performance, not so much.
Another way I've done it is a generalized version of the 'chunk' function in your link: functions needing items processed push their request onto a stack while a frame callback checks the stack and executes a set number of them. I still haven't figured out how to vary the number to match the hardware. If I observe a slow frame rate, is it a slow machine or a fast machine over-tasked by oxp's ...

Yes, I've seen that, too. Yet I wanted to document this solution in case it went useful, probably because it would be less costly than a call each frame.

I currently implemented a counter in the frame callback, so as to act only each 10 frames. I thought to make this number variable depending on the time needed to execute tasks. Not done anything yet, as it would be premature optimization.

- put everything that can be calculated at load time outside of functions

eg. this.$FORTYFIVE_DEGREES = 45 * Math.PI / 180;
but not: this.$CORRECT_FOR_WIDESCR = oolite.gameSettings.gameWindow.height / oolite.gameSettings.gameWindow.width;

ToBeIncluded

On a lighter note, anything you scoop will turn hostile and target you, including cargo pods and splinters!
Also, a spawned entity has isVisible set to true, regardless of distance. It will change to false but I don't know how long that takes.

I beg your pardon ?

[another_commander]

Just pointing out the obvious here, but I think it needs to be noted that all these optimization techniquess explained here are not themselves the objective, but the means to get performance. These techniques are not there to jump in and start writing complicated code. They should be used when there is need and where they are needed. And to find this out, the biggest weapon we have is profiling. I fully recommend that before you change anything in your code, you profile it and find out where it really needs attention. Cag already did that and this is the right way to go about it.

Oolite Test Release writes something in the log header that may have escaped the attention of many, but if you look more carefully, you'll see it: It says: Build options: [...] JavaScript profiling. So you can use Oolite itself to see where bottlenecks in your OXP might be and then you can apply all these techniques exactly where there is a gain to be obtained.

To use the built-in profiler, you need the Debug Console. You just need to run something like :time worldScripts["snoopers"].calcNewDate() or console.profile(function() { [...] }); to get profiling information about the function you want to examine. The output would look something like this

Code: Select all

:time system.sun.goNova()
> console.profile(eval("(function codeToBeProfiled() { (" + "system.sun.goNova()" + ") })"), this)
Total time: 0.073 ms
JavaScript: 0.046 ms, native: 0.021 ms
Counted towards limit: 0.0590004 ms, excluded: 0.0139996 ms
Profiler overhead: 0.041 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                          (<console input>) codeToBeProfiled  J      1     0.07     0.05    91.8    63.0     0.05
                                                   SunGoNova  N      1     0.02     0.01    21.9    19.2     0.01
                                           SystemGetProperty  N      1     0.00     0.00     6.8     6.8     0.00
                                           JSSunGetSunEntity  N      1     0.00     0.00     2.7     2.7     0.00
                                           
                                           
                                           
> :time worldScripts["snoopers"].calcNewDate()
> console.profile(eval("(function codeToBeProfiled() { (" + "worldScripts[\"snoopers\"].calcNewDate()" + ") })"), this)
Total time: 0.446 ms
JavaScript: 0.403 ms, native: 0.037 ms
Counted towards limit: 0.428001 ms, excluded: 0.0179987 ms
Profiler overhead: 0.08 ms
                                                        NAME  T  COUNT    TOTAL     SELF  TOTAL%   SELF%  SELFMAX
                                (snoopers.js:93) <anonymous>  J      1     0.30     0.30    67.0    67.0     0.30
                               (snoopers.js:111) <anonymous>  J      1     0.36     0.06    81.8    14.3     0.06
                          (<console input>) codeToBeProfiled  J      1     0.44     0.04    98.7     9.0     0.04
                                     WorldScriptsGetProperty  N      1     0.03     0.03     7.8     7.2     0.03
                                        OOStringFromJSString  N      1     0.00     0.00     0.7     0.7     0.00
                                            ClockGetProperty  N      1     0.00     0.00     0.4     0.4     0.00

There is lots of information to digest, but the important items are at the top, especially total time. As you can see, the tables contain also the trace of the calls, with separate metrics for each one of them, as well as their type (J for Javascript, N for Native).

Once the bottlenecks have been identified, then you know where you need to turn to and apply all the tips contained here. If you want performance, always profile.

[phkb]

<various code-related things>

*gulps*

[Later]
Hmm. Better get stuck in then, eh? Station Dock Control is probably the worst offender. So, let's throw the profiler at the main generation function and see what we get...

Code: Select all

> :time worldScripts.StationDockControl.$runPopulation()
> console.profile(eval("(function codeToBeProfiled() { (" + "worldScripts.StationDockControl.$runPopulation()" + ") })"), this)
Connection closed with no message.

Oh. "oolite.exe has stopped working". Immediately.

This might take me a bit of time....

It might be time for a colourful metaphor, though.

[another_commander]

@phkb: Is this with trunk 7eae3c8? I fixed a profiler crash with that commit. If you are using a previous version, try updating.

[phkb]

Will do. Thanks! And here I was just thinking it was my bad programming!

[another_commander]

OK cool. In the meantime, I managed to dig this 10-year old post by Jens, describing the presented profiler information in full detail. Note that a lot of things have changed in these 10 years and it might be that there are new native functions that need to be excluded from the profiler metrics. We'll just have to wait for incoming reports on them in order to identify them and subsequently exclude them, if necessary.

[phkb]

...and, yes! The crash is fixed! I can now profile SDC!

<does some profiling>

Oh dear. There are some really big numbers in there!

<dons hard hat>

[Day]

Slowly inserting all the propositions into the first post...

We need an example oxp which is at the same time simple, easy to understand for beginners, and small.
Some idea? Some volunteer?

[Day]

Finally, we're going to have to deal with the orphaned oxp's, whose authors are no longer around. Compile a list, recruit people to re-code, proof and test. See what you started!

Hmm, there should be no orphaned oxp in the manager, am I right?
So... As a lazy dev (the best kind, imo), I'll only aknowledge the existence of managed oxps.
There, a whole lot of work has just disappeared

[phkb]

there should be no orphaned oxp in the manager, am I right?

Um, actually I think there are quite a number of orphaned OXP's in the manager. When the manager was first created, a lot of older OXP's were given a perfunctory OXZ reformat and plonked on the manager, and not necessarily by their original author. For instance, I think randomshipnames is one of these. It's probably one of the most popular OXP's, but I don't think the original author is around any more, and I don't think the author was the one to load it. As another example, Ramirez hasn't been around since 2015, but a good number of his OXP's are in the manager.

I think any work on older OXP's should start in the manager, and in particular, with the larger mission OXP's. These are the ones that will not have aged so well with changes in Oolite, or phrased more accurately, these OXP's are more likely to have subtle bugs that are hard to track down. Resistance Commander, Trident down, Deposed, Military Fiasco - I'd be more inclined to take these ones, dust off any cobwebs, clean up any code that could do with a refresh because of new Oolite features, and re-release them (if the license allows for it).