Oolite Bulletins • OXP Performance tips - Page 2

Page 2 of 6

Re: OXP Performance tips

Posted: Wed Jun 21, 2017 12:25 am

by UK_Eliter

This is great stuff. I think some of it will do wonders for my code.

I had trouble getting the profiler to work in the past, but I think I can see, without it, which bits of my code are the most time critical.

Happily, I'd started rewriting - mainly just stylistically - already.

If I could get someone to give my newly rewritten Interstellar Tweaks OXP a test run, that would be great. I should be able to have it - the rewritten and ready-for-second-party test - version ready in a few days. There should not be that much to do because, though the OXP allows a great many scenarios, I haven't rewritten that much (and I'll test it myself somewhat).

Re: OXP Performance tips

Posted: Wed Jun 21, 2017 2:43 am

by UK_Eliter

Day, this is throwing me:

Code: Select all

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

What exactly does that do? Suppose that the length of myArray is 3, i.e. myArray has three members, the indices of which are:

Code: Select all

myArray[0]
myArray[1]
MyArray[2]

Does your loop proceed backwards through the members of myArray, starting at myArray[2] and proceeding to myArray[0]? The loop through an error in my code, which makes me think its trying to access myArray[-1] or myArray[3], but probably there's a mistake elsewhere in my code.

EDIT: Here are the relevant bits of my code, i.e. the bits that threw the error.

Code: Select all

this.maxBattleships = 2;

this.$isBattleship = function(e) {
	return e.isShip && e.isThargoid && e.name === "Thargoid Thargorn Battleship"
}
// Note: the battleship lacks a distinct role, so we have to use its name.

if ( this.bugThreat_Installed && Math.random() <= this.chanceLimitBattleshipsInOtherwiseUnmodified ) {
	s = system.filteredEntities(this, this.$isBattleship);
	i = s.length - this.maxBattleships;
	while ( i-- ) {
		s[i].remove(true);
	}
}

The error is:

Code: Select all

JavaScript exception (IST_masterScript 5.38): TypeError: s[i] is undefined

Re: OXP Performance tips

Posted: Wed Jun 21, 2017 6:17 am

by phkb

I think the problem is with this line:

Code: Select all

i = s.length - this.maxBattleships;

You're setting "i" up to be an index of the "s" array, but then you're adjusting the starting point. If "s" ends up having 0 elements (ie because there are no battleships returned through the filteredEntities call), then i is going to start with at index -2. And, therefore, "undefined".

Re: OXP Performance tips

Posted: Wed Jun 21, 2017 8:31 am

by Day

UK_Eliter wrote: ↑Wed Jun 21, 2017 2:43 am
Day, this is throwing me:
Code: Select all
var z = myArray.length;
while (z--) {
    var myElement = myArray[z];
    // my code
}
What exactly does that do? Suppose that the length of myArray is 3, i.e. myArray has three members, the indices of which are:
Code: Select all
myArray[0]
myArray[1]
MyArray[2]
Does your loop proceed backwards through the members of myArray, starting at myArray[2] and proceeding to myArray[0]?

Exactly.

phkb wrote:
I think the problem is with this line:
Code: Select all
i = s.length - this.maxBattleships;
You're setting "i" up to be an index of the "s" array, but then you're adjusting the starting point. If "s" ends up having 0 elements (ie because there are no battleships returned through the filteredEntities call), then i is going to start with at index -2. And, therefore, "undefined".

Exactly. The starting point should be the length of the list you want to iterate through.

Re: OXP Performance tips

Posted: Wed Jun 21, 2017 8:35 am

by Day

UK_Eliter wrote: ↑Wed Jun 21, 2017 12:25 am
This is great stuff. I think some of it will do wonders for my code.

Thank you

Re: OXP Performance tips

Posted: Wed Jun 21, 2017 12:32 pm

by UK_Eliter

phkb, Day

Thank you for your explanations.

About the 'undefined' error: I had been under the impression that if i <= 0, any text within

Code: Select all

while ( i-- ) {
}

would not run at all.

Re: OXP Performance tips

Posted: Wed Jun 21, 2017 4:04 pm

by Day

UK_Eliter wrote: ↑Wed Jun 21, 2017 12:32 pm
phkb, Day

Thank you for your explanations.

About the 'undefined' error: I had been under the impression that if i <= 0, any text within
Code: Select all
while ( i-- ) {
}
would not run at all.

Well, 0 is considered false, and other numbers, including negative ones, are true.
So for i = 0, the code within won't run (the -- is operated after the while comparison).
For i < 0, the code within will an inifinite amount of times.

Re: OXP Performance tips

Posted: Wed Jun 21, 2017 7:21 pm

by UK_Eliter

Ah, right. Sometimes I will have to add some checks for that, resulting in slightly less optimisation and - where I do add a check - a slightly more fragile script. Oh well.

Re: OXP Performance tips

Posted: Thu Jun 22, 2017 1:16 am

by UK_Eliter

I've a question about functions.

Previously, the following was typical in my code.

Code: Select all

this.$timedDamageNonPlayerViaRadiation = function $timedDamageNonPlayerViaRadiation() {
	var i;
	var s;
	var damage;
	var shipsToDamage;

	if ( !system.isInterstellarSpace ) { this.$removeRadiationTimers(); return; }
	if ( !player.ship ) { this.$removeRadiationTimers(); return; }
	s = system.allShips;

	function $toDamage(e) {
		return e.isShip && !e.isPlayer
	}

	shipsToDamage = s.filteredEntities(this, $toDamage);

Now I am getting rid of functions-within-functions. But I am unsure of the syntax for entity filtering and I've been getting an error message. Is the following format right, please?

Code: Select all

this.$toDamage = function(e) {
	return e.isShip && !e.isPlayer
}

this.$timedDamageNonPlayerViaRadiation = function $timedDamageNonPlayerViaRadiation() {
	var i;
	var s;
	var damage;
	var shipsToDamage;

	if ( !system.isInterstellarSpace ) { this.$removeRadiationTimers(); return; }
	if ( !player.ship ) { this.$removeRadiationTimers(); return; }
	s = system.allShips;
	shipsToDamage = s.filteredEntities(this, this.$toDamage); // DOES THIS WORK?

I am unsure about the 'this' in 'this.$toDamage'. On the other hand, perhaps the problem is with variable declaration . .

Re: OXP Performance tips

Posted: Thu Jun 22, 2017 2:10 am

by cag

your error is coming from

Code: Select all

s = system.allShips;
...
shipsToDamage = s.filteredEntities(this, $toDamage);

allShips is just an array i.e., a property of system, and has no methods.
filteredEntities is a method of system.
So, instead, just use

Code: Select all

shipsToDamage = system.filteredEntities(this, $toDamage);

Re: OXP Performance tips

Posted: Thu Jun 22, 2017 2:28 am

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 ) ...

I replaced all (29) of the indexOf used on arrays in telescope.js and got 3 more frames/sec!

Re: OXP Performance tips

Posted: Thu Jun 22, 2017 2:55 am

by UK_Eliter

cag: thanks a lot. About your suggestion that I use:

Code: Select all

shipsToDamage = system.filteredEntities(this, $toDamage);

Is the '(this, $toDamage)' right? I don't need '(this, this.$toDamage)'?

(As you can see, I don't really understand this stuff, though somehow I manage to have written a 1, 000 line OXP that works! Well, it worked before I started the latest bit of fiddling with it, anyway . .)

Re: OXP Performance tips

Posted: Thu Jun 22, 2017 3:03 am

by cag

Yes, you do need that this!

Code: Select all

shipsToDamage = system.filteredEntities(this, this.$toDamage);

The 1st (lone) this tells filteredEntities where to operate in, in what 'scope' or enviroment to use (ie. this tells it to run using your script file's variables).
You absolutely need the 2nd one on $toDamage, or you'll get a reference error (the interpreter won't be able to find the function)

Re: OXP Performance tips

Posted: Thu Jun 22, 2017 3:13 am

by UK_Eliter

OK, great. Thank you. I did not need the second 'this' when the function in question was defined within the function that I was in already. (See an earlier post of mine if you need explanation.) But, as per one of the performance tips earlier in this thread. I was trying to get rid of such nested (if that's the word I want) functions.

Re: OXP Performance tips

Posted: Thu Jun 22, 2017 9:16 am

by Day

cag wrote: ↑Thu Jun 22, 2017 2:28 am
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 ) ...
I replaced all (29) of the indexOf used on arrays in telescope.js and got 3 more frames/sec!

Wow!
I would never have thought that the native method would be slower than the reimplementation

ToBeInserted => Done