Map Enumerators: Proposed change

Monkey Forums/Monkey Bug Reports/Map Enumerators: Proposed change

AdamRedwoods

(Posted 2014) [#1]

Map iteration is slow when using Eachin and Values(). Please consider making NextNode,PrevNode,FirstNode,LastNode, and Value() public instead of private.

Example:

Import monkey.map
Import mojo.app

Class Foo
	Global ids:Int =0
	
	Method New( n$ )
		id=ids
		ids+=1
		name =n
	end
	
	
	Method GetName:String()
		Return name
	End
	
	Field name:String
	Field id:int
	Field data:Float[1024]
	
End

Class FooMap
	Global map:IntMap<Foo> = New IntMap<Foo>
End
	

Function Main()

	For Local i%=0 To 100
		Local f:Foo = New Foo("foo "+i)
		FooMap.map.Set(f.id, f)
	Next
	
	Local ms:Int, k:int
	
	ms=Millisecs()
	
	k=0
	For Local j%=0 To 1000
		For Local i% = 0 To 100
	
			If  (FooMap.map.Get(i).id >0)
				k=k+1
			Endif
			
		Next
	Next
	Print "k="+k
	
	Print (Millisecs()-ms)
	
	
	ms=Millisecs()
	
	k=0
	For Local j%=0 To 1000
		For Local f:Foo = Eachin FooMap.map.Values()
	
			If  (f.id >0)
				k=k+1
			Endif
			
		Next
	Next
	Print "k="+k
	
	Print (Millisecs()-ms)
	
	'' ******** Note the change to public methods below ***********
	ms=Millisecs()
	
	k=0
	Local nn:map.Node<Int, Foo>, f:Foo
	For Local j%=0 To 1000
		nn = FooMap.map.FirstNode()
		While(nn)
			f=nn.Value()
			
			If  (f.id >0)
				k=k+1
			Endif
			
			nn=nn.NextNode
		Wend
	Next
	Print "k="+k
	
	Print (Millisecs()-ms)
	
	
End

the above changes (3rd timer) , when public, is almost 50% faster on Android (v4.1), and uses less class constructor calls.
my output:

I/[Monkey]( 4944): k=100100
I/[Monkey]( 4944): 240
I/[Monkey]( 4944): k=100100
I/[Monkey]( 4944): 131
I/[Monkey]( 4944): k=100100
I/[Monkey]( 4944): 61

marksibly

(Posted 2014) [#2]

Compiles here anyway! Looks like some bug has crept into public/private...

It appears that Value, NextNode and PrevNode are already public, so I don't see any particular reason why FirstNode and LastNode can't also be public.

That said, while using NextNode like this will improve the speed of a virtually empty loop, it'll have much less impact on code that actually does something, although it will make it harder to read.

AdamRedwoods

(Posted 2014) [#3]

it'll have much less impact on code that actually does something,

i noticed this on Android/Java, when you have a loop over the Map loop (lots of constructor calling). that's why i posted the example to show specifically how this is influential. seems to make a difference in collision detection.