Bug - Fixed Maximizer slow after r17231

Darzil

Developer
Reported at http://forums.kingdomofloathing.com/vb/showpost.php?p=4874169&postcount=4085, with great detail given by Ryo_Sangnoir here http://forums.kingdomofloathing.com/vb/showpost.php?p=4874241&postcount=4093.

Tested to replicate Ryo's findings, on a fairly slow computer:

Code:
r17230 maximize mp
Maximizing (1st time may take a while)...
13542 combinations checked, best score 4,042.00
30450 combinations checked, best score 4,042.00
46726 combinations checked, best score 4,042.00
63873 combinations checked, best score 4,042.00
80945 combinations checked, best score 4,042.00
98084 combinations checked, best score 4,042.00
109824 combinations checked, best score 4,042.00

r17231 maximize mp
Maximizing (1st time may take a while)...
1 combinations checked, best score 3,660.00
1197 combinations checked, best score 4,042.00
2431 combinations checked, best score 4,042.00
3583 combinations checked, best score 4,042.00
4871 combinations checked, best score 4,042.00
6133 combinations checked, best score 4,042.00
7363 combinations checked, best score 4,042.00
8463 combinations checked, best score 4,042.00
9569 combinations checked, best score 4,042.00
10780 combinations checked, best score 4,042.00
12040 combinations checked, best score 4,042.00
13284 combinations checked, best score 4,042.00
14558 combinations checked, best score 4,042.00
15772 combinations checked, best score 4,042.00
16977 combinations checked, best score 4,042.00
18153 combinations checked, best score 4,042.00
19296 combinations checked, best score 4,042.00
20359 combinations checked, best score 4,042.00
21538 combinations checked, best score 4,042.00
22670 combinations checked, best score 4,042.00
23853 combinations checked, best score 4,042.00
25019 combinations checked, best score 4,042.00
26315 combinations checked, best score 4,042.00
27556 combinations checked, best score 4,042.00
28845 combinations checked, best score 4,042.00
30100 combinations checked, best score 4,042.00
and so on . . .

So, 10x slower in the MaximizerSpeculation code, which wasn't directly changed. I'm struggling to see which of the changes could have this effect, but will investigate.
 
17231 added "Variable" to the twitch belt's modifiers. That modifier doesn't show up anywhere else in Modifiers.txt

Is that doing something weird?
 

Darzil

Developer
No, first thing I tried.
I think the issue is that the extra code in override did an getItemId on name before name was defined in the function, so would have used the name of the Modifier or something instead, which it will fail to find, but it may take a while. Am testing that now.
 

Darzil

Developer
Yeah, that helps a bit. The bigger issue is that by that code going before the expression check makes things much slower. Am going to think about it.
 

Veracity

Developer
Staff member
Sheesh.

The override method of a Modifier object calculates modifiers that can vary. That includes expressions and a handful of items which need special code. To wit:

Tuesday's ruby
Uncle Hobo's beard
Operation Patriot Shield
time-twitching toolbelt

That last one is the first item that ALSO has expressions. And so, the original code which returned after calculating expressions was inadequate.

I saw Variable and misunderstood what it was for. The boolean VARIABLE is set after modifiers are overridden or if the modifier is a "Loc" or a "Zone".

(We only have one Zone. From modifiers.txt:

Code:
Zone	Zone:The Sea	Initiative Penalty: -100, Item Drop Penalty: -100, Meat Drop Penalty: -100

Why is there a "Zone:" in the name? Surely it should simply be "The Sea"?)

Who looks at VARIABLE? Looks like DebugModifiers.allModifiers, which is called only by the "modifies" command, which will append a "v" to the printout of a variable modifier.

So, as coded, putting Variable in modifiers.txt does nothing - especially since there is no tag pattern for it in the boolean modifiers array. I suppose we could add the tag pattern and let the override method use !this.getBoolean( Modifiers.VARIABLE ) rather than "!lookup.equals( "time-twitching toolbelt" )" to continue after expressions.

I'll experiment a bit.
 

Darzil

Developer
Yeah, main changes I made were just kicking lookups as late as possible, as it's relatively slow, so the fewer you hit the better, especially for something like Maximization, which hits them so often. I'm sure there other other finesses which could speed it up further. So far Maximization tweaks I've done have looked at reducing the number of items to run combinations for, rather than reducing the time each loop takes to run.
 

Darzil

Developer
It looks cleaner, but I'm afraid is about 10x slower on my PC :

r17249

Maximizing (1st time may take a while)...
5907 combinations checked, best score 3,501.00
13919 combinations checked, best score 3,501.00
21971 combinations checked, best score 3,501.00
29988 combinations checked, best score 3,501.00
37976 combinations checked, best score 3,501.00
45891 combinations checked, best score 3,501.00
53680 combinations checked, best score 3,501.00
61415 combinations checked, best score 3,501.00
68984 combinations checked, best score 3,501.00
76763 combinations checked, best score 3,501.00
84685 combinations checked, best score 3,501.00
92646 combinations checked, best score 3,501.00
100627 combinations checked, best score 3,501.00
108663 combinations checked, best score 3,501.00
109824 combinations checked, best score 3,501.00

r17248

Maximizing (1st time may take a while)...
99716 combinations checked, best score 3,501.00
109824 combinations checked, best score 3,501.00
 
Last edited:

Darzil

Developer
I'm trying to work out what this bit of code is for, in Modifiers.getModifiers():

Code:
		newMods.variable = newMods.override( lookup );
		if ( newMods.variable || type.equals( "Loc" ) || type.equals( "Zone" ) )
		{
			newMods.bitmaps[ 0 ] |= 1 << Modifiers.VARIABLE;
		}
I think it is setting variable for any items with Loc, Zone or Expressions (as they return true from override()), which will mean many will still hit the slow lookup section.
 

Veracity

Developer
Staff member
It is setting that variable AFTER it calls override(). However, if you call getModifiers again, if will have a Modifiers object with that variable set.

Since it sets newMods.variable - which is NOT the same as the Modifiers.VARIABLE modifier - why can't the caller just look at that?

So:

Code:
		newMods.variable = newMods.override( lookup ) || type.equals( "Loc" ) || type.equals( "Zone" );
And in DebugModifiers.allModifiers:

Code:
				if ( value != 0.0 )
				{
					list.add( new Change( type, name, value,
						mods.getBoolean( Modifiers.VARIABLE ) ) );
				}
becomes:

Code:
				if ( value != 0.0 )
				{
					list.add( new Change( type, name, value, mods.variable ) );
				}
 

Veracity

Developer
Staff member
Which is sort of like reusing the existing boolean but using the "variable" field of Modifiers better.
 

Darzil

Developer
Makes sense to me. I tried adding an extra boolean, which worked, for a marginal improvement (my example maximisation did 106k rather than 99k combinations in the first text), but was starting to think about using that variable instead.
 

Darzil

Developer
Yes, that does it. Elegance and performance.

With r17252 :

Maximizing (1st time may take a while)...
100080 combinations checked, best score 3,501.00
109824 combinations checked, best score 3,501.00
 
Top